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.

Sign up for GitHub

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

Add a public API to determine if a switch expression was valid in C# 6. #13498

Closed
wants to merge 1 commit into from
Closed

Add a public API to determine if a switch expression was valid in C# 6. #13498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,22 @@ internal static SeparatedSyntaxList<TOther> AsSeparatedList<TOther>(this SyntaxN
return builder.ToList();
}

#region ITypeSymbol

/// <summary>
/// Returns true iff the supplied type is sbyte, byte, short, ushort, int, uint,
/// long, ulong, bool, char, string, or an enum-type, or if it is the nullable type
/// corresponding to one of those types. These types were permitted as the governing
/// type of a switch statement in C# 6.
/// </summary>
public static bool IsValidV6SwitchGoverningType(this ITypeSymbol type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be an API that will appear in the completion list of all users of ITypeSymbol? This seems like a well known fact that could be in a private helper that the edit-and-continue logic uses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't "need" to be in any particular place, but this is a language question implemented in the compiler layer. Generally speaking, when language questions need to be asked by other layers, they should do so via public APIs. I think you're suggesting that perhaps this language question is not generally useful enough to expose publicly?

In reply to: 77065023 [](ancestors = 77065023)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be more appropriate to expose on some more specialized type, perhaps on SemanticModel? Also it would be nice to avoid V6 in the name. Perhaps IsPrimitiveSwitchType?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a simpler name. For example, it includes enum types.

The implementation is simple enough that I think it can just be coped into the E&C code. I'll place the implementation in the issue for this, and close this PR.

In reply to: 77084316 [](ancestors = 77084316)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but it looks like the EnC project does not have access to compiler internals (and trying to give it access causes many conflicts). I'm assuming there is a reason for that.
If so, having this public compiler API will help.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnC doesn't need access to compiler internals to copy the logic that is inside that API.

{
var typeSymbol = type as TypeSymbol;
return (typeSymbol == null) ? false :
TypeSymbolExtensions.IsValidV6SwitchGoverningType(typeSymbol);
}
#endregion ITypeSymbol

#region SyntaxNode
internal static IList<DirectiveTriviaSyntax> GetDirectives(this SyntaxNode node, Func<DirectiveTriviaSyntax, bool> filter = null)
{
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ override Microsoft.CodeAnalysis.CSharp.Syntax.WhenClauseSyntax.Accept<TResult>(M
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetDeclaredSymbol(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.DeclarationPatternSyntax declarationSyntax, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.ILocalSymbol
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetDeclaredSymbol(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.SingleVariableDesignationSyntax designationSyntax, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.ISymbol
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetForEachStatementInfo(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.CommonForEachStatementSyntax forEachStatement) -> Microsoft.CodeAnalysis.CSharp.ForEachStatementInfo
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.IsValidV6SwitchGoverningType(this Microsoft.CodeAnalysis.ITypeSymbol type) -> bool
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.CasePatternSwitchLabel(Microsoft.CodeAnalysis.CSharp.Syntax.PatternSyntax pattern, Microsoft.CodeAnalysis.CSharp.Syntax.WhenClauseSyntax whenClause, Microsoft.CodeAnalysis.SyntaxToken colonToken) -> Microsoft.CodeAnalysis.CSharp.Syntax.CasePatternSwitchLabelSyntax
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.CasePatternSwitchLabel(Microsoft.CodeAnalysis.CSharp.Syntax.PatternSyntax pattern, Microsoft.CodeAnalysis.SyntaxToken colonToken) -> Microsoft.CodeAnalysis.CSharp.Syntax.CasePatternSwitchLabelSyntax
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.CasePatternSwitchLabel(Microsoft.CodeAnalysis.SyntaxToken keyword, Microsoft.CodeAnalysis.CSharp.Syntax.PatternSyntax pattern, Microsoft.CodeAnalysis.CSharp.Syntax.WhenClauseSyntax whenClause, Microsoft.CodeAnalysis.SyntaxToken colonToken) -> Microsoft.CodeAnalysis.CSharp.Syntax.CasePatternSwitchLabelSyntax
Expand Down
67 changes: 57 additions & 10 deletions src/Compilers/CSharp/Test/Semantic/Semantics/SwitchTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Roslyn.Test.Utilities;
using Xunit;
using System.Collections.Generic;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
Expand Down Expand Up @@ -1304,7 +1305,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "t").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1346,7 +1353,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "C").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1397,7 +1410,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "C").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1441,7 +1460,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "D").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1480,7 +1505,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "C").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1524,7 +1555,13 @@ public static int Main()
";
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6).VerifyDiagnostics();
CreateCompilationWithMscorlib(text, parseOptions: TestOptions.Regular6WithV7SwitchBinder).VerifyDiagnostics();
CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "C").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.True(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[Fact]
Expand Down Expand Up @@ -1575,8 +1612,13 @@ static void M<T>(B2<T> b2) where T : A2
// (20,17): error CS0151: A switch expression or case label must be a bool, char, string, integral, enum, or corresponding nullable type
Diagnostic(ErrorCode.ERR_V6SwitchGoverningTypeValueExpected, "b1.F()").WithLocation(20, 17)
);
CreateCompilationWithMscorlib(text).VerifyDiagnostics(
);
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<ExpressionSyntax>().Where(s => s.ToString() == "b1.F()").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.False(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[WorkItem(543673, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/543673")]
Expand Down Expand Up @@ -1631,8 +1673,13 @@ static void Main()
// switch(a)
Diagnostic(ErrorCode.ERR_V6SwitchGoverningTypeValueExpected, "a").WithLocation(28, 20)
);
CreateCompilationWithMscorlib(text).VerifyDiagnostics(
);
var compilation = CreateCompilationWithMscorlib(text).VerifyDiagnostics();
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var switchExpression = tree.GetRoot().DescendantNodes().OfType<SimpleNameSyntax>().Where(s => s.ToString() == "a").Single();
var typeInfo = model.GetTypeInfo(switchExpression);
Assert.False(typeInfo.Type.IsValidV6SwitchGoverningType());
Assert.False(typeInfo.ConvertedType.IsValidV6SwitchGoverningType());
}

[WorkItem(543673, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/543673")]
Expand Down