From 74b2ea285f4299cff80dab6c997ec706ef8c2944 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 6 Feb 2019 15:48:45 -0800 Subject: [PATCH] Classify constructor/destructor as containing type (#32912) * Classify Constructors and Destructors as their containing type * Updated classifier test * Added tests for symbol display parts * Added tests for static constructor and destructor --- .../SymbolDisplayVisitor.Members.cs | 197 ++++++++++-------- .../SymbolDisplay/SymbolDisplayTests.cs | 158 +++++++++++++- .../Classification/SemanticClassifierTests.cs | 37 +++- .../SyntacticClassifierTests.cs | 8 +- .../Classification/TotalClassifierTests.cs | 6 +- .../TotalClassifierTests_Dynamic.cs | 2 +- .../Classification/ClassificationHelpers.cs | 8 +- .../NameSyntaxClassifier.cs | 8 + 8 files changed, 326 insertions(+), 98 deletions(-) diff --git a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs index e4aaae016f8..1fe5adc9b72 100644 --- a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs +++ b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs @@ -351,122 +351,142 @@ public override void VisitMethod(IMethodSymbol symbol) case MethodKind.Ordinary: case MethodKind.DelegateInvoke: case MethodKind.LocalFunction: - //containing type will be the delegate type, name will be Invoke - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.Name)); - break; + { + //containing type will be the delegate type, name will be Invoke + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.Name)); + break; + } case MethodKind.ReducedExtension: - // Note: Extension methods invoked off of their static class will be tagged as methods. - // This behavior matches the semantic classification done in NameSyntaxClassifier. - builder.Add(CreatePart(SymbolDisplayPartKind.ExtensionMethodName, symbol, symbol.Name)); - break; + { + // Note: Extension methods invoked off of their static class will be tagged as methods. + // This behavior matches the semantic classification done in NameSyntaxClassifier. + builder.Add(CreatePart(SymbolDisplayPartKind.ExtensionMethodName, symbol, symbol.Name)); + break; + } case MethodKind.PropertyGet: case MethodKind.PropertySet: - isAccessor = true; - var associatedProperty = (IPropertySymbol)symbol.AssociatedSymbol; - if (associatedProperty == null) { - goto case MethodKind.Ordinary; + isAccessor = true; + var associatedProperty = (IPropertySymbol)symbol.AssociatedSymbol; + if (associatedProperty == null) + { + goto case MethodKind.Ordinary; + } + AddPropertyNameAndParameters(associatedProperty); + AddPunctuation(SyntaxKind.DotToken); + AddKeyword(symbol.MethodKind == MethodKind.PropertyGet ? SyntaxKind.GetKeyword : SyntaxKind.SetKeyword); + break; } - AddPropertyNameAndParameters(associatedProperty); - AddPunctuation(SyntaxKind.DotToken); - AddKeyword(symbol.MethodKind == MethodKind.PropertyGet ? SyntaxKind.GetKeyword : SyntaxKind.SetKeyword); - break; case MethodKind.EventAdd: case MethodKind.EventRemove: - isAccessor = true; - var associatedEvent = (IEventSymbol)symbol.AssociatedSymbol; - if (associatedEvent == null) { - goto case MethodKind.Ordinary; + isAccessor = true; + var associatedEvent = (IEventSymbol)symbol.AssociatedSymbol; + if (associatedEvent == null) + { + goto case MethodKind.Ordinary; + } + AddEventName(associatedEvent); + AddPunctuation(SyntaxKind.DotToken); + AddKeyword(symbol.MethodKind == MethodKind.EventAdd ? SyntaxKind.AddKeyword : SyntaxKind.RemoveKeyword); + break; } - AddEventName(associatedEvent); - AddPunctuation(SyntaxKind.DotToken); - AddKeyword(symbol.MethodKind == MethodKind.EventAdd ? SyntaxKind.AddKeyword : SyntaxKind.RemoveKeyword); - break; case MethodKind.Constructor: case MethodKind.StaticConstructor: - // Note: we are using the metadata name also in the case that - // symbol.containingType is null (which should never be the case here) or is an - // anonymous type (which 'does not have a name'). - var name = format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames) || symbol.ContainingType == null || symbol.ContainingType.IsAnonymousType - ? symbol.Name - : symbol.ContainingType.Name; - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, name)); - break; - case MethodKind.Destructor: - // Note: we are using the metadata name also in the case that symbol.containingType is null, which should never be the case here. - if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames) || symbol.ContainingType == null) { - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.Name)); + // Note: we are using the metadata name also in the case that + // symbol.containingType is null (which should never be the case here) or is an + // anonymous type (which 'does not have a name'). + var name = format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames) || symbol.ContainingType == null || symbol.ContainingType.IsAnonymousType + ? symbol.Name + : symbol.ContainingType.Name; + + var partKind = GetPartKindForConstructorOrDestructor(symbol); + + builder.Add(CreatePart(partKind, symbol, name)); + break; } - else + case MethodKind.Destructor: { - AddPunctuation(SyntaxKind.TildeToken); - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.ContainingType.Name)); + var partKind = GetPartKindForConstructorOrDestructor(symbol); + + // Note: we are using the metadata name also in the case that symbol.containingType is null, which should never be the case here. + if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames) || symbol.ContainingType == null) + { + builder.Add(CreatePart(partKind, symbol, symbol.Name)); + } + else + { + AddPunctuation(SyntaxKind.TildeToken); + builder.Add(CreatePart(partKind, symbol, symbol.ContainingType.Name)); + } + break; } - break; case MethodKind.ExplicitInterfaceImplementation: - AddExplicitInterfaceIfRequired(symbol.ExplicitInterfaceImplementations); - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, - ExplicitInterfaceHelpers.GetMemberNameWithoutInterfaceName(symbol.Name))); - break; - - case MethodKind.UserDefinedOperator: - case MethodKind.BuiltinOperator: - if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames)) { - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.MetadataName)); + AddExplicitInterfaceIfRequired(symbol.ExplicitInterfaceImplementations); + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, + ExplicitInterfaceHelpers.GetMemberNameWithoutInterfaceName(symbol.Name))); + break; } - else + case MethodKind.UserDefinedOperator: + case MethodKind.BuiltinOperator: { - AddKeyword(SyntaxKind.OperatorKeyword); - AddSpace(); - if (symbol.MetadataName == WellKnownMemberNames.TrueOperatorName) + if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames)) { - AddKeyword(SyntaxKind.TrueKeyword); - } - else if (symbol.MetadataName == WellKnownMemberNames.FalseOperatorName) - { - AddKeyword(SyntaxKind.FalseKeyword); + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.MetadataName)); } else { - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, - SyntaxFacts.GetText(SyntaxFacts.GetOperatorKind(symbol.MetadataName)))); + AddKeyword(SyntaxKind.OperatorKeyword); + AddSpace(); + if (symbol.MetadataName == WellKnownMemberNames.TrueOperatorName) + { + AddKeyword(SyntaxKind.TrueKeyword); + } + else if (symbol.MetadataName == WellKnownMemberNames.FalseOperatorName) + { + AddKeyword(SyntaxKind.FalseKeyword); + } + else + { + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, + SyntaxFacts.GetText(SyntaxFacts.GetOperatorKind(symbol.MetadataName)))); + } } + break; } - break; - case MethodKind.Conversion: - if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames)) { - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.MetadataName)); - } - else - { - // "System.IntPtr.explicit operator System.IntPtr(int)" - - if (symbol.MetadataName == WellKnownMemberNames.ExplicitConversionName) + if (format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.UseMetadataMethodNames)) { - AddKeyword(SyntaxKind.ExplicitKeyword); - } - else if (symbol.MetadataName == WellKnownMemberNames.ImplicitConversionName) - { - AddKeyword(SyntaxKind.ImplicitKeyword); + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, symbol.MetadataName)); } else { - builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, - SyntaxFacts.GetText(SyntaxFacts.GetOperatorKind(symbol.MetadataName)))); - } + // "System.IntPtr.explicit operator System.IntPtr(int)" - AddSpace(); - AddKeyword(SyntaxKind.OperatorKeyword); - AddSpace(); - AddReturnType(symbol); - } - break; + if (symbol.MetadataName == WellKnownMemberNames.ExplicitConversionName) + { + AddKeyword(SyntaxKind.ExplicitKeyword); + } + else if (symbol.MetadataName == WellKnownMemberNames.ImplicitConversionName) + { + AddKeyword(SyntaxKind.ImplicitKeyword); + } + else + { + builder.Add(CreatePart(SymbolDisplayPartKind.MethodName, symbol, + SyntaxFacts.GetText(SyntaxFacts.GetOperatorKind(symbol.MetadataName)))); + } + AddSpace(); + AddKeyword(SyntaxKind.OperatorKeyword); + AddSpace(); + AddReturnType(symbol); + } + break; + } default: throw ExceptionUtilities.UnexpectedValue(symbol.MethodKind); } @@ -479,6 +499,17 @@ public override void VisitMethod(IMethodSymbol symbol) } } + private static SymbolDisplayPartKind GetPartKindForConstructorOrDestructor(IMethodSymbol symbol) + { + // In the case that symbol.containingType is null (which should never be the case here) we will fallback to the MethodName symbol part + if (symbol.ContainingType is null) + { + return SymbolDisplayPartKind.MethodName; + } + + return GetPartKind(symbol.ContainingType); + } + private void AddReturnType(IMethodSymbol symbol) { var methodSymbol = symbol as MethodSymbol; diff --git a/src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs b/src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs index d7f56b20564..3dc95d0d6fb 100644 --- a/src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs @@ -1298,7 +1298,7 @@ class C { findSymbol, format, ".ctor", - SymbolDisplayPartKind.MethodName); + SymbolDisplayPartKind.ClassName); } [Fact] @@ -6930,5 +6930,161 @@ void M() SymbolDisplayPartKind.Space, SymbolDisplayPartKind.Punctuation); // _ } + + [Fact] + public void ClassConstructorDeclaration() + { + TestSymbolDescription( +@"class C +{ + C() { } +}", + global => global.GetTypeMember("C").Constructors[0], + new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType), + "C.C", + SymbolDisplayPartKind.ClassName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.ClassName); + } + + [Fact] + public void ClassDestructorDeclaration() + { + TestSymbolDescription( +@"class C +{ + ~C() { } +}", + global => global.GetTypeMember("C").GetMember("Finalize"), + new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType), + "C.~C", + SymbolDisplayPartKind.ClassName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.ClassName); + } + + [Fact] + public void ClassStaticConstructorDeclaration() + { + TestSymbolDescription( +@"class C +{ + static C() { } +}", + global => global.GetTypeMember("C").Constructors[0], + new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType), + "C.C", + SymbolDisplayPartKind.ClassName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.ClassName); + } + + [Fact] + public void ClassStaticDestructorDeclaration() + { + TestSymbolDescription( +@"class C +{ + static ~C() { } +}", + global => global.GetTypeMember("C").GetMember("Finalize"), + new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType), + "C.~C", + SymbolDisplayPartKind.ClassName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.ClassName); + } + + [Fact] + public void ClassConstructorInvocation() + { + var format = new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType); + + var source = +@"class C +{ + C() + { + var c = new C(); + } +}"; + + var compilation = CreateCompilation(source); + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + + var constructor = tree.GetRoot().DescendantNodes().OfType().Single(); + var symbol = model.GetSymbolInfo(constructor).Symbol; + + Verify( + symbol.ToMinimalDisplayParts(model, constructor.SpanStart, format), + "C.C", + SymbolDisplayPartKind.ClassName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.ClassName); + } + + + [Fact] + public void StructConstructorDeclaration() + { + TestSymbolDescription( +@"struct S +{ + int i; + + S(int i) + { + this.i = i; + } +}", + global => global.GetTypeMember("S").Constructors[0], + new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType), + "S.S", + SymbolDisplayPartKind.StructName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.StructName); + } + + [Fact] + public void StructConstructorInvocation() + { + var format = new SymbolDisplayFormat(memberOptions: SymbolDisplayMemberOptions.IncludeContainingType); + + var source = +@"struct S +{ + int i; + + public S(int i) + { + this.i = i; + } +} + +class C +{ + C() + { + var s = new S(1); + } +}"; + + var compilation = CreateCompilation(source); + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + + var constructor = tree.GetRoot().DescendantNodes().OfType().Single(); + var symbol = model.GetSymbolInfo(constructor).Symbol; + + Verify( + symbol.ToMinimalDisplayParts(model, constructor.SpanStart, format), + "S.S", + SymbolDisplayPartKind.StructName, + SymbolDisplayPartKind.Punctuation, + SymbolDisplayPartKind.StructName); + } } } diff --git a/src/EditorFeatures/CSharpTest/Classification/SemanticClassifierTests.cs b/src/EditorFeatures/CSharpTest/Classification/SemanticClassifierTests.cs index 680217a03c0..ca4e7f5b491 100644 --- a/src/EditorFeatures/CSharpTest/Classification/SemanticClassifierTests.cs +++ b/src/EditorFeatures/CSharpTest/Classification/SemanticClassifierTests.cs @@ -581,10 +581,8 @@ class Derived : Base Namespace("System"), Class("Base"), Class("My"), - Method("My"), Class("Derived"), Class("My"), - Method("My"), Class("Attribute"), Class("Base")); } @@ -1157,6 +1155,37 @@ class Nested { } Class("String")); } + [Fact, Trait(Traits.Feature, Traits.Features.Classification)] + public async Task Constructors() + { + await TestAsync( +@"struct S +{ + public int i; + + public S(int i) + { + this.i = i; + } +} + +class C +{ + public C() + { + var s = new S(1); + var c = new C(); + } +}", + Field("i"), + Parameter("i"), + Keyword("var"), + Struct("S"), + Keyword("var"), + Class("C")); + } + + [Fact, Trait(Traits.Feature, Traits.Features.Classification)] public async Task TypesOfClassMembers() { @@ -2408,7 +2437,7 @@ class MyClass public MyClass(int x) { } -}", Method("MyClass")); +}", Class("MyClass")); } [WorkItem(633, "https://github.com/dotnet/roslyn/issues/633")] @@ -2426,7 +2455,7 @@ public MyClass(int x) } }", Class("MyClass"), - Method("MyClass")); + Class("MyClass")); } [WorkItem(13174, "https://github.com/dotnet/roslyn/issues/13174")] diff --git a/src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests.cs b/src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests.cs index f0dfd7094dc..a3c9d1d9914 100644 --- a/src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests.cs +++ b/src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests.cs @@ -2104,7 +2104,7 @@ public async Task AttributeTargetSpecifiersOnCtor() Punctuation.Colon, Identifier("A"), Punctuation.CloseBracket, - Method("C"), + Class("C"), Punctuation.OpenParen, Punctuation.CloseParen, Punctuation.OpenCurly, @@ -2132,7 +2132,7 @@ public async Task AttributeTargetSpecifiersOnDtor() Identifier("A"), Punctuation.CloseBracket, Operators.Tilde, - Method("C"), + Class("C"), Punctuation.OpenParen, Punctuation.CloseParen, Punctuation.OpenCurly, @@ -2679,7 +2679,7 @@ interface Bar Punctuation.OpenCurly, Punctuation.CloseCurly, Keyword("public"), - Method("Goo"), + Class("Goo"), Punctuation.OpenParen, Keyword("int"), Parameter("i"), @@ -2956,7 +2956,7 @@ interface Bar Field("field"), Punctuation.Semicolon, Keyword("public"), - Method("Baz"), + Class("Baz"), Punctuation.OpenParen, Keyword("int"), Parameter("i"), diff --git a/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs b/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs index 06626d2a61d..60cc4301773 100644 --- a/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs +++ b/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs @@ -148,7 +148,7 @@ public async Task VarAsConstructorName() Keyword("class"), Class("var"), Punctuation.OpenCurly, - Method("var"), + Class("var"), Punctuation.OpenParen, Punctuation.CloseParen, Punctuation.OpenCurly, @@ -741,7 +741,7 @@ public MyClass(int x) XmlDoc.AttributeQuotes("\""), Class("MyClass"), Operators.Dot, - Method("MyClass"), + Class("MyClass"), Punctuation.OpenParen, Keyword("int"), Punctuation.CloseParen, @@ -756,7 +756,7 @@ public MyClass(int x) Class("MyClass"), Punctuation.OpenCurly, Keyword("public"), - Method("MyClass"), + Class("MyClass"), Punctuation.OpenParen, Keyword("int"), Parameter("x"), diff --git a/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests_Dynamic.cs b/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests_Dynamic.cs index 14d5fb34391..3e7d9c919be 100644 --- a/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests_Dynamic.cs +++ b/src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests_Dynamic.cs @@ -707,7 +707,7 @@ public async Task DynamicAsConstructorDeclarationName() Keyword("class"), Class("dynamic"), Punctuation.OpenCurly, - Method("dynamic"), + Class("dynamic"), Punctuation.OpenParen, Punctuation.CloseParen, Punctuation.OpenCurly, diff --git a/src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs b/src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs index d810f09de17..a564fa618d6 100644 --- a/src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs +++ b/src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs @@ -206,11 +206,15 @@ private static string GetClassificationForIdentifier(SyntaxToken token) } else if (token.Parent is ConstructorDeclarationSyntax constructorDeclaration && constructorDeclaration.Identifier == token) { - return ClassificationTypeNames.MethodName; + return constructorDeclaration.IsParentKind(SyntaxKind.ClassDeclaration) + ? ClassificationTypeNames.ClassName + : ClassificationTypeNames.StructName; } else if (token.Parent is DestructorDeclarationSyntax destructorDeclaration && destructorDeclaration.Identifier == token) { - return ClassificationTypeNames.MethodName; + return destructorDeclaration.IsParentKind(SyntaxKind.ClassDeclaration) + ? ClassificationTypeNames.ClassName + : ClassificationTypeNames.StructName; } else if (token.Parent is LocalFunctionStatementSyntax localFunctionStatement && localFunctionStatement.Identifier == token) { diff --git a/src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/NameSyntaxClassifier.cs b/src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/NameSyntaxClassifier.cs index d8b7fe9f2ab..b942c47d1f8 100644 --- a/src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/NameSyntaxClassifier.cs +++ b/src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/NameSyntaxClassifier.cs @@ -257,6 +257,14 @@ private static string GetClassificationForLocal(ILocalSymbol localSymbol) private static string GetClassificationForMethod(IMethodSymbol methodSymbol) { + // Classify constructors by their containing type. We do not need to worry about + // destructors because their declaration is handled by syntactic classification + // and they cannot be invoked, so their is no usage to semantically classify. + if (methodSymbol.MethodKind == MethodKind.Constructor) + { + return methodSymbol.ContainingType?.GetClassification() ?? ClassificationTypeNames.MethodName; + } + // Note: We only classify an extension method if it is in reduced form. // If an extension method is called as a static method invocation (e.g. Enumerable.Select(...)), // it is classified as an ordinary method. -- GitLab