From b3cbe7abce7633e45d7dd468bde96bfe24ccde47 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Fri, 21 Feb 2020 17:36:39 -0800 Subject: [PATCH] Bind expression variables appearing in a goto case statement. (#41709) * Bind expression variables appearing in a goto case statement. Fixes #40714 --- .../Binder/ExpressionVariableFinder.cs | 7 ++ .../Portable/Binder/LocalScopeBinder.cs | 1 + .../Symbols/Source/SourceLocalSymbol.cs | 5 + .../IOperationTests_InvalidStatement.cs | 56 +++++++++ .../Semantics/PatternMatchingTestBase.cs | 14 +++ .../Semantics/PatternMatchingTests3.cs | 117 ++++++++++++++++++ 6 files changed, 200 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Binder/ExpressionVariableFinder.cs b/src/Compilers/CSharp/Portable/Binder/ExpressionVariableFinder.cs index 1d268ccee79f9..b46912b639f5e 100644 --- a/src/Compilers/CSharp/Portable/Binder/ExpressionVariableFinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/ExpressionVariableFinder.cs @@ -46,6 +46,7 @@ protected void FindExpressionVariables( case SyntaxKind.VariableDeclarator: case SyntaxKind.ConstructorDeclaration: case SyntaxKind.SwitchExpressionArm: + case SyntaxKind.GotoCaseStatement: break; case SyntaxKind.ArgumentList: Debug.Assert(node.Parent is ConstructorInitializerSyntax); @@ -99,6 +100,12 @@ public override void VisitVariableDeclarator(VariableDeclaratorSyntax node) VisitNodeToBind(node.Initializer); } + public override void VisitGotoStatement(GotoStatementSyntax node) + { + if (node.Kind() == SyntaxKind.GotoCaseStatement) + Visit(node.Expression); + } + private void VisitNodeToBind(CSharpSyntaxNode node) { SyntaxNode previousNodeToBind = _nodeToBind; diff --git a/src/Compilers/CSharp/Portable/Binder/LocalScopeBinder.cs b/src/Compilers/CSharp/Portable/Binder/LocalScopeBinder.cs index 94f7c797a4447..34a19d69a0e8e 100644 --- a/src/Compilers/CSharp/Portable/Binder/LocalScopeBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/LocalScopeBinder.cs @@ -247,6 +247,7 @@ protected ImmutableArray BuildLocals(SyntaxList st case SyntaxKind.YieldReturnStatement: case SyntaxKind.ReturnStatement: case SyntaxKind.ThrowStatement: + case SyntaxKind.GotoCaseStatement: ExpressionVariableFinder.FindExpressionVariables(this, locals, innerStatement, enclosingBinder.GetBinder(innerStatement) ?? enclosingBinder); break; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceLocalSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceLocalSymbol.cs index 982e950087ba2..f4826acfc68aa 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceLocalSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceLocalSymbol.cs @@ -171,6 +171,7 @@ internal static LocalSymbol MakeLocalSymbolWithEnclosingContext( nodeToBind.Kind() == SyntaxKind.BaseConstructorInitializer || nodeToBind.Kind() == SyntaxKind.SwitchExpressionArm || nodeToBind.Kind() == SyntaxKind.ArgumentList && nodeToBind.Parent is ConstructorInitializerSyntax || + nodeToBind.Kind() == SyntaxKind.GotoCaseStatement || // for error recovery nodeToBind.Kind() == SyntaxKind.VariableDeclarator && new[] { SyntaxKind.LocalDeclarationStatement, SyntaxKind.ForStatement, SyntaxKind.UsingStatement, SyntaxKind.FixedStatement }. Contains(nodeToBind.Ancestors().OfType().First().Kind()) || @@ -742,6 +743,7 @@ public LocalSymbolWithEnclosingContext( nodeToBind.Kind() == SyntaxKind.ArgumentList && nodeToBind.Parent is ConstructorInitializerSyntax || nodeToBind.Kind() == SyntaxKind.VariableDeclarator || nodeToBind.Kind() == SyntaxKind.SwitchExpressionArm || + nodeToBind.Kind() == SyntaxKind.GotoCaseStatement || nodeToBind is ExpressionSyntax); Debug.Assert(!(nodeToBind.Kind() == SyntaxKind.SwitchExpressionArm) || nodeBinder is SwitchExpressionArmBinder); this._nodeBinder = nodeBinder; @@ -783,6 +785,9 @@ protected override TypeWithAnnotations InferTypeOfVarVariable(DiagnosticBag diag var armBinder = (SwitchExpressionArmBinder)_nodeBinder; armBinder.BindSwitchExpressionArm(arm, diagnostics); break; + case SyntaxKind.GotoCaseStatement: + _nodeBinder.BindStatement((GotoStatementSyntax)_nodeToBind, diagnostics); + break; default: _nodeBinder.BindExpression((ExpressionSyntax)_nodeToBind, diagnostics); break; diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_InvalidStatement.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_InvalidStatement.cs index 2899b0686880e..80be00a3ba156 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_InvalidStatement.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_InvalidStatement.cs @@ -357,6 +357,62 @@ static void Main(string[] args) VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); } + [CompilerTrait(CompilerFeature.IOperation)] + [Fact, WorkItem(40714, "https://github.com/dotnet/roslyn/issues/40714")] + public void InvalidGotoCaseStatement_BadLabel() + { + string source = @" +using System; + +class Program +{ + static void Main(string[] args) + { + switch (args[0], args[1]) + { + case (string s1, string s2) _: + /**/goto case args is (var x1, var x2);/**/ + x1 = x2; + case (string str, null) _: + break; + } + } +} +"; + string expectedOperationTree = @" +IInvalidOperation (OperationKind.Invalid, Type: null, IsInvalid) (Syntax: 'goto case a ... 1, var x2);') + Children(1): + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: (System.String, System.String), IsInvalid, IsImplicit) (Syntax: 'args is (var x1, var x2)') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsInvalid) (Syntax: 'args is (var x1, var x2)') + Value: + IParameterReferenceOperation: args (OperationKind.ParameterReference, Type: System.String[], IsInvalid) (Syntax: 'args') + Pattern: + IRecursivePatternOperation (OperationKind.RecursivePattern, Type: null, IsInvalid) (Syntax: '(var x1, var x2)') (InputType: System.String[], DeclaredSymbol: null, MatchedType: System.String[], DeconstructSymbol: null) + DeconstructionSubpatterns (2): + IDeclarationPatternOperation (OperationKind.DeclarationPattern, Type: null, IsInvalid) (Syntax: 'var x1') (InputType: ?, DeclaredSymbol: ?? x1, MatchesNull: True) + IDeclarationPatternOperation (OperationKind.DeclarationPattern, Type: null, IsInvalid) (Syntax: 'var x2') (InputType: ?, DeclaredSymbol: ?? x2, MatchesNull: True) + PropertySubpatterns (0) +"; + var expectedDiagnostics = new DiagnosticDescription[] { + // file.cs(10,18): error CS0163: Control cannot fall through from one case label ('(string s1, string s2) _') to another + // case (string s1, string s2) _: + Diagnostic(ErrorCode.ERR_SwitchFallThrough, "(string s1, string s2) _").WithArguments("(string s1, string s2) _").WithLocation(10, 18), + // file.cs(11,27): error CS0029: Cannot implicitly convert type 'bool' to '(string, string)' + // /**/goto case args is (var x1, var x2);/**/ + Diagnostic(ErrorCode.ERR_NoImplicitConv, "goto case args is (var x1, var x2);").WithArguments("bool", "(string, string)").WithLocation(11, 27), + // file.cs(11,45): error CS1061: 'string[]' does not contain a definition for 'Deconstruct' and no accessible extension method 'Deconstruct' accepting a first argument of type 'string[]' could be found (are you missing a using directive or an assembly reference?) + // /**/goto case args is (var x1, var x2);/**/ + Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "(var x1, var x2)").WithArguments("string[]", "Deconstruct").WithLocation(11, 45), + // file.cs(11,45): error CS8129: No suitable 'Deconstruct' instance or extension method was found for type 'string[]', with 2 out parameters and a void return type. + // /**/goto case args is (var x1, var x2);/**/ + Diagnostic(ErrorCode.ERR_MissingDeconstruct, "(var x1, var x2)").WithArguments("string[]", "2").WithLocation(11, 45) + }; + + VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); + } + [CompilerTrait(CompilerFeature.IOperation)] [Fact, WorkItem(17607, "https://github.com/dotnet/roslyn/issues/17607")] public void InvalidGotoCaseStatement_OutsideSwitchStatement() diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs index 7aea810b8357c..c1256ed500fea 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs @@ -170,6 +170,20 @@ protected static void VerifyModelForDuplicateVariableDeclarationInSameScope(Sema Assert.True(model.LookupNames(declarator.SpanStart).Contains(declarator.Identifier.ValueText)); } + internal static void VerifyModelForDuplicateVariableDeclarationInSameScope( + SemanticModel model, + SingleVariableDesignationSyntax designation, + LocalDeclarationKind kind) + { + var symbol = model.GetDeclaredSymbol(designation); + Assert.Equal(designation.Identifier.ValueText, symbol.Name); + Assert.Equal(designation, symbol.DeclaringSyntaxReferences.Single().GetSyntax()); + Assert.Equal(kind, symbol.GetSymbol().DeclarationKind); + Assert.Same(symbol, model.GetDeclaredSymbol((SyntaxNode)designation)); + Assert.NotEqual(symbol, model.LookupSymbols(designation.SpanStart, name: designation.Identifier.ValueText).Single()); + Assert.True(model.LookupNames(designation.SpanStart).Contains(designation.Identifier.ValueText)); + } + protected static void VerifyNotAPatternField(SemanticModel model, IdentifierNameSyntax reference) { var symbol = model.GetSymbolInfo(reference).Symbol; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests3.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests3.cs index eac4fe72c3523..8024f8080780f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests3.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests3.cs @@ -1771,5 +1771,122 @@ static void M(int i) Diagnostic(ErrorCode.ERR_AmbigUDConv, "new A()").WithArguments("A.implicit operator B(A)", "B.implicit operator B(A)", "A", "B").WithLocation(16, 43) ); } + + [WorkItem(40714, "https://github.com/dotnet/roslyn/issues/40714")] + [Fact] + public void BadGotoCase_01() + { + var source = @" +class C +{ + static void Example(object a, object b) + { + switch ((a, b)) + { + case (string str, int[] arr) _: + goto case (string str, decimal[] arr); + case (string str, decimal[] arr) _: + break; + } + } +} +"; + var compilation = CreateCompilation(source); + compilation.VerifyDiagnostics( + // (8,18): error CS0163: Control cannot fall through from one case label ('(string str, int[] arr) _') to another + // case (string str, int[] arr) _: + Diagnostic(ErrorCode.ERR_SwitchFallThrough, "(string str, int[] arr) _").WithArguments("(string str, int[] arr) _").WithLocation(8, 18), + // (8,26): error CS0136: A local or parameter named 'str' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case (string str, int[] arr) _: + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "str").WithArguments("str").WithLocation(8, 26), + // (8,37): error CS0136: A local or parameter named 'arr' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case (string str, int[] arr) _: + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "arr").WithArguments("arr").WithLocation(8, 37), + // (9,17): error CS0150: A constant value is expected + // goto case (string str, decimal[] arr); + Diagnostic(ErrorCode.ERR_ConstantExpected, "goto case (string str, decimal[] arr);").WithLocation(9, 17), + // (9,28): error CS8185: A declaration is not allowed in this context. + // goto case (string str, decimal[] arr); + Diagnostic(ErrorCode.ERR_DeclarationExpressionNotPermitted, "string str").WithLocation(9, 28), + // (9,28): error CS0165: Use of unassigned local variable 'str' + // goto case (string str, decimal[] arr); + Diagnostic(ErrorCode.ERR_UseDefViolation, "string str").WithArguments("str").WithLocation(9, 28), + // (9,40): error CS8185: A declaration is not allowed in this context. + // goto case (string str, decimal[] arr); + Diagnostic(ErrorCode.ERR_DeclarationExpressionNotPermitted, "decimal[] arr").WithLocation(9, 40), + // (9,40): error CS0165: Use of unassigned local variable 'arr' + // goto case (string str, decimal[] arr); + Diagnostic(ErrorCode.ERR_UseDefViolation, "decimal[] arr").WithArguments("arr").WithLocation(9, 40), + // (10,26): error CS0136: A local or parameter named 'str' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case (string str, decimal[] arr) _: + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "str").WithArguments("str").WithLocation(10, 26), + // (10,41): error CS0136: A local or parameter named 'arr' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case (string str, decimal[] arr) _: + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "arr").WithArguments("arr").WithLocation(10, 41) + ); + + var tree = compilation.SyntaxTrees.Single(); + var model = compilation.GetSemanticModel(tree); + + var strDecl = tree.GetRoot().DescendantNodes().OfType().Where(s => s.Identifier.ValueText == "str").ToArray(); + Assert.Equal(3, strDecl.Length); + VerifyModelForDuplicateVariableDeclarationInSameScope(model, strDecl[1], LocalDeclarationKind.DeclarationExpressionVariable); + + var arrDecl = tree.GetRoot().DescendantNodes().OfType().Where(s => s.Identifier.ValueText == "arr").ToArray(); + Assert.Equal(3, arrDecl.Length); + VerifyModelForDuplicateVariableDeclarationInSameScope(model, arrDecl[1], LocalDeclarationKind.DeclarationExpressionVariable); + } + + [WorkItem(40714, "https://github.com/dotnet/roslyn/issues/40714")] + [Fact] + public void BadGotoCase_02() + { + var source = @" +class C +{ + static void Example(object a, object b) + { + switch ((a, b)) + { + case (string str, int[] arr) _: + goto case a is (var x1, var x2); + x1 = x2; + case (string str, decimal[] arr) _: + break; + } + } +} +"; + var compilation = CreateCompilation(source); + compilation.VerifyDiagnostics( + // (8,18): error CS0163: Control cannot fall through from one case label ('(string str, int[] arr) _') to another + // case (string str, int[] arr) _: + Diagnostic(ErrorCode.ERR_SwitchFallThrough, "(string str, int[] arr) _").WithArguments("(string str, int[] arr) _").WithLocation(8, 18), + // (9,17): error CS0029: Cannot implicitly convert type 'bool' to '(object a, object b)' + // goto case a is (var x1, var x2); + Diagnostic(ErrorCode.ERR_NoImplicitConv, "goto case a is (var x1, var x2);").WithArguments("bool", "(object a, object b)").WithLocation(9, 17), + // (9,32): error CS1061: 'object' does not contain a definition for 'Deconstruct' and no accessible extension method 'Deconstruct' accepting a first argument of type 'object' could be found (are you missing a using directive or an assembly reference?) + // goto case a is (var x1, var x2); + Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "(var x1, var x2)").WithArguments("object", "Deconstruct").WithLocation(9, 32), + // (9,32): error CS8129: No suitable 'Deconstruct' instance or extension method was found for type 'object', with 2 out parameters and a void return type. + // goto case a is (var x1, var x2); + Diagnostic(ErrorCode.ERR_MissingDeconstruct, "(var x1, var x2)").WithArguments("object", "2").WithLocation(9, 32) + ); + + var tree = compilation.SyntaxTrees.Single(); + var model = compilation.GetSemanticModel(tree); + + var x1Decl = GetPatternDeclarations(tree, "x1").ToArray(); + var x1Ref = GetReferences(tree, "x1").ToArray(); + Assert.Equal(1, x1Decl.Length); + Assert.Equal(1, x1Ref.Length); + VerifyModelForDeclarationOrVarSimplePattern(model, x1Decl[0], x1Ref); + + var x2Decl = GetPatternDeclarations(tree, "x2").ToArray(); + var x2Ref = GetReferences(tree, "x2").ToArray(); + Assert.Equal(1, x2Decl.Length); + Assert.Equal(1, x2Ref.Length); + VerifyModelForDeclarationOrVarSimplePattern(model, x2Decl[0], x2Ref); + } } }