dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.96k stars 4.02k forks source link

VisualBasicRemoveUnusedValuesCodeFixProvider crashes #36682

Open Paxxi opened 5 years ago

Paxxi commented 5 years ago

Version Used: VS 2019 16.1.3 but also seeing issue with git version 99f88ce03be3f8a524d9a855258cd9f8cdbe2c08

Steps to Reproduce:

  1. Create a VB classlib targeting netfx 472
  2. Paste this into Class1.vb
    Public Class Class1
    Sub Method()
        For Each a In New Integer() {1, 2, 3}
            a = 1
            Console.WriteLine(a)
        Next
    End Sub
    End Class
  3. place character on a after For Each For Each |a and trigger light bulb, I used ctrl+. but clicking it seems to do the same thing.

Expected Behavior: Get a code fix suggestion to remove unused variable (IDE hints about this by fading the variable). I don't believe it should be offered at all for the foreach variable as removing it will break the code. Actual Behavior: image

System.InvalidOperationException : The item specified is not the element of a list.
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitForEachBlock(ForEachBlockSyntax node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.ForEachBlockSyntax.Accept[TResult](VisualBasicSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.BaseListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitMethodBlock(MethodBlockSyntax node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.MethodBlockSyntax.Accept[TResult](VisualBasicSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.BaseListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitClassBlock(ClassBlockSyntax node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.ClassBlockSyntax.Accept[TResult](VisualBasicSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.BaseListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.VisitCompilationUnit(CompilationUnitSyntax node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.CompilationUnitSyntax.Accept[TResult](VisualBasicSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.BaseListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.NodeListEditor.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.VisualBasic.Syntax.SyntaxReplacer.InsertNodeInList(SyntaxNode root,SyntaxNode nodeInList,IEnumerable`1 nodesToInsert,Boolean insertBefore)
   at Microsoft.CodeAnalysis.VisualBasic.VisualBasicSyntaxNode.InsertNodesInListCore(SyntaxNode nodeInList,IEnumerable`1 nodesToInsert,Boolean insertBefore)
   at Microsoft.CodeAnalysis.SyntaxNodeExtensions.InsertNodesBefore[TRoot](TRoot root,SyntaxNode nodeInList,IEnumerable`1 newNodes)
   at Microsoft.CodeAnalysis.VisualBasic.CodeGeneration.VisualBasicSyntaxGenerator.InsertDeclarationsBeforeInternal(SyntaxNode root,SyntaxNode declaration,IEnumerable`1 newDeclarations)
   at Microsoft.CodeAnalysis.VisualBasic.CodeGeneration.VisualBasicSyntaxGenerator._Closure$__309-0._Lambda$__0(SyntaxNode r)
   at Microsoft.CodeAnalysis.Editing.SyntaxGenerator.PreserveTrivia[TNode](TNode node,Func`2 nodeChanger)
   at Microsoft.CodeAnalysis.VisualBasic.CodeGeneration.VisualBasicSyntaxGenerator.InsertNodesBefore(SyntaxNode root,SyntaxNode declaration,IEnumerable`1 newDeclarations)
   at Microsoft.CodeAnalysis.Editing.SyntaxEditor.InsertChange.Apply(SyntaxNode root,SyntaxGenerator generator)
   at Microsoft.CodeAnalysis.Editing.SyntaxEditor.GetChangedRoot()
   at async Microsoft.CodeAnalysis.RemoveUnusedParametersAndValues.AbstractRemoveUnusedValuesCodeFixProvider`11.FixAllAsync[TExpressionSyntax,TStatementSyntax,TBlockSyntax,TExpressionStatementSyntax,TLocalDeclarationStatementSyntax,TVariableDeclaratorSyntax,TForEachStatementSyntax,TSwitchCaseBlockSyntax,TSwitchCaseLabelOrClauseSyntax,TCatchStatementSyntax,TCatchBlockSyntax](<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeFixes.SyntaxEditorBasedCodeFixProvider.FixAllWithEditorAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeActions.CodeAction.GetChangedSolutionAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeActions.CodeAction.ComputeOperationsAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeActions.CodeAction.GetPreviewOperationsAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.GetPreviewResultAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedActionWithNestedFlavors.<>c__DisplayClass11_0.<GetPreviewAsync>b__0(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.PerformFunctionAsync[T](<Unknown Parameters>)
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
mavasani commented 4 years ago

@lameox Do you think this is fixed by your recent change https://github.com/dotnet/roslyn/pull/40412?

mavasani commented 4 years ago

Ah possibly not. I think this line needs to be moved before the switch statement, as VB does not support discards: https://github.com/dotnet/roslyn/blob/f6759acf9db639b4045a545a4a35419c53ee8951/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedValuesCodeFixProvider.cs#L125

paul1956 commented 4 years ago

I thought VB support discards with _1, 2 vs. just in C#. image image

lameox commented 4 years ago

@mavasani This reproduces in the current master with my merged changes. I can try to tackle this if you want.

mavasani commented 4 years ago

@lameox Go for it if you'd like.