Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IntroduceVariableCodeRefactoring Crash #38708

Closed
333fred opened this issue Sep 16, 2019 · 10 comments
Closed

IntroduceVariableCodeRefactoring Crash #38708

333fred opened this issue Sep 16, 2019 · 10 comments
Assignees
Labels
Area-IDE Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com IDE-CodeStyle Built-in analyzers, fixes, and refactorings Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Urgency-Soon
Milestone

Comments

@333fred
Copy link
Member

333fred commented Sep 16, 2019

System.ArgumentOutOfRangeException : Specified argument was out of the range of valid values.
Parameter name: position
   at Microsoft.CodeAnalysis.SyntaxNode.FindTokenCore(Int32 position,Boolean findInsideTrivia)
   at Microsoft.CodeAnalysis.CodeRefactoringHelpers.IsNodeUnderselected(SyntaxNode node,TextSpan selection)
   at async Microsoft.CodeAnalysis.IntroduceVariable.AbstractIntroduceVariableService`6.State.TryInitializeAsync[TService,TExpressionSyntax,TTypeSyntax,TTypeDeclarationSyntax,TQueryExpressionSyntax,TNameSyntax](<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.IntroduceVariable.AbstractIntroduceVariableService`6.State.GenerateAsync[TService,TExpressionSyntax,TTypeSyntax,TTypeDeclarationSyntax,TQueryExpressionSyntax,TNameSyntax](<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.IntroduceVariable.AbstractIntroduceVariableService`6.IntroduceVariableAsync[TService,TExpressionSyntax,TTypeSyntax,TTypeDeclarationSyntax,TQueryExpressionSyntax,TNameSyntax](<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.IntroduceVariable.IntroduceVariableCodeRefactoringProvider.ComputeRefactoringsAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeRefactorings.CodeRefactoringService.GetRefactoringFromProviderAsync(<Unknown Parameters>)

The selection here appears to be a little finicky, it need to be in the middle of some kind of nested expression, I think? Here's a screenshot of my selection:
image
As far as I can tell, the offer to rename the method is unrelated, it's just showing that as I was simplifying my repro.

Raw code as well:

using System;
class C {
    public void M()
    {
        Console.WriteLine("Hello world");
        Console.WriteLine(new C());
    }
}

VS Version 29312.127 Int Preview

@333fred 333fred added Bug Area-IDE IDE-CodeStyle Built-in analyzers, fixes, and refactorings labels Sep 16, 2019
@CyrusNajmabadi
Copy link
Member

tagging @petrroll . Note: this may have been fixed already.

@yyjdelete
Copy link

Hit the same issue in VS16.4 Preview1.0

Select all text between > and <.

        public static void Method()
        {
            //>
            var str = "<aaa";
        }

@mavasani
Copy link
Contributor

Hit the same issue on latest dogfood build. I will investigate.

@mavasani mavasani self-assigned this Sep 24, 2019
@mavasani mavasani added this to the 16.4 milestone Sep 24, 2019
@petrroll
Copy link
Contributor

petrroll commented Sep 24, 2019

I assume the issue is that the position is outside of the Token passed into the IsNodeUnderselected which is also used as root for finding leftmost token.

I thought I eliminated all instances where I used non-root Token as Token but this one must've slipped by

I can take a look at it on Friday if it proves to be more problematic to solve.

@petrroll
Copy link
Contributor

petrroll commented Sep 24, 2019

Hmm, that doesn't seem to be the issue: http://source.roslyn.io/#Microsoft.CodeAnalysis.Features/CodeRefactoringHelpers.cs,34b38eefcd6d9c43

Can't immediately see how that can fail. The node's span isn't empty so .End - 1 location is definitely within, so is .Start.

That said, IIRC there should be an easier way (and faster) to get the first and last token of the Node (and then their respective parents). And switching to that solution might fix the issue as byproduct.

@miloush
Copy link

miloush commented Oct 6, 2019

Just hit this one with VisualStudio.16.Preview/16.4.0-pre.1.0+29319.158

@miloush
Copy link

miloush commented Oct 6, 2019

And here is a repro:

class Program
{
    static void Main()
    {
        Main();
    }
}

Select the semicolon only, either by double-clicking it or using keyboard.

@petrroll
Copy link
Contributor

petrroll commented Oct 6, 2019

I have found the issue and have a fix ready, PR in an hour or so.

petrroll added a commit to petrroll/roslyn that referenced this issue Oct 6, 2019
petrroll added a commit to petrroll/roslyn that referenced this issue Oct 6, 2019
@petrroll
Copy link
Contributor

petrroll commented Oct 6, 2019

Fix in #39091

cc: @mavasani

@jinujoseph jinujoseph added the Developer Community The issue was originally reported on https://developercommunity.visualstudio.com label Oct 9, 2019
@jinujoseph
Copy link
Contributor

reported in DC , DC

@sharwell sharwell added Urgency-Soon Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented labels Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com IDE-CodeStyle Built-in analyzers, fixes, and refactorings Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Urgency-Soon
Projects
None yet
Development

No branches or pull requests

8 participants