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

SyntaxEditorBasedFixAllProvider crashed for IDE0066 #37907

Closed
stephentoub opened this issue Aug 12, 2019 · 6 comments · Fixed by #38007
Closed

SyntaxEditorBasedFixAllProvider crashed for IDE0066 #37907

stephentoub opened this issue Aug 12, 2019 · 6 comments · Fixed by #38007
Assignees
Labels
Area-IDE Bug IDE-CodeStyle Built-in analyzers, fixes, and refactorings Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@stephentoub
Copy link
Member

C# Tools 3.3.0-beta2-19401-05+8ed56b826700926009731ca4d3de0a4e4e652969

I tried running IDE0066 (use 'switch' expression) on a .sln for all of corefx. At some point it failed with the message "'SyntaxEditorBasedFixAllProvider' encountered an error and has been disabled." with the stack:

System.InvalidOperationException : Unexpected value 'BreakStatement' of type 'Microsoft.CodeAnalysis.CSharp.SyntaxKind'
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.DefaultVisit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor`1.VisitBreakStatement(BreakStatementSyntax node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.BreakStatementSyntax.Accept[TResult](CSharpSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.RewriteSwitchStatement(SwitchStatementSyntax node,Boolean allowMoveNextStatementToSwitchExpression)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.VisitSwitchStatement(SwitchStatementSyntax node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SwitchStatementSyntax.Accept[TResult](CSharpSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.GetSwitchExpressionArm(SwitchSectionSyntax node)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.<RewriteSwitchStatement>b__13_1(SwitchSectionSyntax s)
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.RewriteSwitchStatement(SwitchStatementSyntax node,Boolean allowMoveNextStatementToSwitchExpression)
   at Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.Rewriter.Rewrite(SwitchStatementSyntax switchStatement,SemanticModel semanticModel,SyntaxEditor editor,SyntaxKind nodeToGenerate,Boolean shouldMoveNextStatementToSwitchExpression)
   at async Microsoft.CodeAnalysis.CSharp.ConvertSwitchStatementToExpression.ConvertSwitchStatementToExpressionCodeFixProvider.FixAllAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeFixes.SyntaxEditorBasedCodeFixProvider.FixAllWithEditorAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeFixes.SyntaxEditorBasedCodeFixProvider.SyntaxEditorBasedFixAllProvider.FixDocumentAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeFixes.SyntaxEditorBasedCodeFixProvider.SyntaxEditorBasedFixAllProvider.GetFixAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeFixes.SyntaxEditorBasedCodeFixProvider.SyntaxEditorBasedFixAllProvider.GetFixAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.FixAllGetFixesService.GetFixAllCodeActionAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.FixAllGetFixesService.GetFixAllOperationsAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeActions.CodeAction.GetOperationsCoreAsync(<Unknown Parameters>)
   at Roslyn.Utilities.TaskExtensions.WaitAndGetResult_CanCallOnBackground[T](Task`1 task,CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.InvokeWorker(Func`1 getFromDocument,IProgressTracker progressTracker,CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.<>c__DisplayClass20_0.<InvokeCore>b__0()
   at Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.PerformAction(IExtensionManager extensionManager,Object extension,Action action)
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)

I don't have a simple repro.

@CyrusNajmabadi
Copy link
Member

tagging @alrz

@alrz
Copy link
Contributor

alrz commented Aug 12, 2019

from the stacktrace, this looks like a nested switch followed by a break which probably belongs to the outer switch. a few regex lookups didn't find any interesting case in corefx so far. I'll see if I can come up with a repro.

@alrz
Copy link
Contributor

alrz commented Aug 12, 2019

@CyrusNajmabadi would it be possible to record the diagnostic location for which the CodeFixProvider throws? so that we can spot the offending code in a fix-all operation.

@CyrusNajmabadi
Copy link
Member

@mavasani ?

@vatsalyaagrawal vatsalyaagrawal added Area-IDE Bug IDE-CodeStyle Built-in analyzers, fixes, and refactorings labels Aug 12, 2019
@vatsalyaagrawal vatsalyaagrawal modified the milestones: 15.9, 16.3 Aug 12, 2019
@stephentoub
Copy link
Member Author

I attached a debugger and found the problematic case. Here's a repro:

using System;

class Program
{
    public static void Main() { }
    public DayOfWeek StatusValue() => DayOfWeek.Monday;
    public short Value => 0;
    public bool ValueBoolean()
    {
        bool value;
        switch (StatusValue())
        {
            case DayOfWeek.Monday:
                switch (Value)
                {
                    case 0:
                        value = false;
                        break;
                    case 1:
                        value = true;
                        break;
                    default:
                        throw new Exception();
                }
                break;
            default:
                throw new Exception();
        }
        return value;
    }
}

@alrz
Copy link
Contributor

alrz commented Aug 14, 2019

I have a fix for this and a few others, will open a pr soon.

@mavasani mavasani added the 4 - In Review A fix for the issue is submitted for review. label Aug 16, 2019
@mavasani mavasani assigned alrz and unassigned mavasani Aug 16, 2019
ryzngard added a commit that referenced this issue Oct 3, 2019
Bugfixes for ConvertSwitchStatementToExpression

Fixes #37949 (leave open for offering the fix instead of bail out)
Fixes #37950 (will be resolved on the compiler side)
Fixes #36876
Fixes #37873
Fixes #37872
Fixes #37907
Fixes #37947
Closes #36877 (duplicate of #37873)
@jinujoseph jinujoseph added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 4 - In Review A fix for the issue is submitted for review. labels Oct 3, 2019
@jinujoseph jinujoseph modified the milestones: 16.3, 16.4.P3 Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug IDE-CodeStyle Built-in analyzers, fixes, and refactorings Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants