Merge pull request #18661 from CyrusNajmabadi/fullyQualifyGenerics

Do not fully qualify type name if it would not resolve error.
dotnet · Apr 13, 2017 · ea0bfc4 · ea0bfc4
2 parents 21ab374 + 9a2269d
commit ea0bfc4
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 16 deletions.
diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/FullyQualify/FullyQualifyTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/FullyQualify/FullyQualifyTests.cs
@@ -298,7 +298,7 @@ class Class
         Foo();
     }
 }",
-count: 2);
+count: 1);
 
             await TestInRegularAndScriptAsync(
 @"using System.Collections.Generic;
@@ -1320,7 +1320,7 @@ await TestInRegularAndScriptAsync(
         }
 
         [WorkItem(18275, "https://github.com/dotnet/roslyn/issues/18275")]
-        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddImport)]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsFullyQualify)]
         public async Task TestContextualKeyword1()
         {
             await TestMissingInRegularAndScriptAsync(
@@ -1338,6 +1338,19 @@ void M()
     {
         [|nameof|]
     }
+}");
+        }
+
+        [WorkItem(18623, "https://github.com/dotnet/roslyn/issues/18623")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsFullyQualify)]
+        public async Task TestDoNotQualifyToTheSameTypeToFixWrongArity()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"
+using System.Collections.Generic;
+
+class Program : [|IReadOnlyCollection|]
+{
 }");
         }
     }

diff --git a/src/Features/CSharp/Portable/CSharpFeatures.csproj b/src/Features/CSharp/Portable/CSharpFeatures.csproj
@@ -127,7 +127,7 @@
     <Compile Include="AddMissingReference\CSharpAddMissingReferenceCodeFixProvider.cs" />
     <Compile Include="CodeFixes\Async\CSharpAddAwaitCodeFixProvider.cs" />
     <Compile Include="CodeFixes\Async\CSharpConvertToAsyncMethodCodeFixProvider.cs" />
-    <Compile Include="CodeFixes\FullyQualify\CSharpFullyQualifyCodeFixProvider.cs" />
+    <Compile Include="FullyQualify\CSharpFullyQualifyCodeFixProvider.cs" />
     <Compile Include="GenerateConstructor\GenerateConstructorCodeFixProvider.cs" />
     <Compile Include="CodeFixes\GenerateEnumMember\GenerateEnumMemberCodeFixProvider.cs" />
     <Compile Include="CodeFixes\GenerateMethod\GenerateConversionCodeFixProvider.cs" />

diff --git a/...lify/CSharpFullyQualifyCodeFixProvider.cs → ...lify/CSharpFullyQualifyCodeFixProvider.cs b/...lify/CSharpFullyQualifyCodeFixProvider.cs → ...lify/CSharpFullyQualifyCodeFixProvider.cs
diff --git a/src/Features/Core/Portable/Features.csproj b/src/Features/Core/Portable/Features.csproj
@@ -235,8 +235,8 @@
     <Compile Include="CodeFixes\FixAllOccurrences\IFixMultipleOccurrencesService.cs" />
     <Compile Include="CodeFixes\ImplementAbstractClass\AbstractImplementAbstractClassCodeFixProvider.cs" />
     <Compile Include="CodeFixes\PreferFrameworkType\PreferFrameworkTypeCodeFixProvider.cs" />
-    <Compile Include="CodeFixes\Qualify\AbstractFullyQualifyCodeFixProvider.SymbolResult.cs" />
-    <Compile Include="CodeFixes\Qualify\AbstractFullyQualifyCodeFixProvider.cs" />
+    <Compile Include="FullyQualify\AbstractFullyQualifyCodeFixProvider.SymbolResult.cs" />
+    <Compile Include="FullyQualify\AbstractFullyQualifyCodeFixProvider.cs" />
     <Compile Include="CodeFixes\GenerateMember\AbstractGenerateMemberCodeFixProvider.cs" />
     <Compile Include="CodeFixes\ICodeFixProviderFactory.cs" />
     <Compile Include="CodeFixes\FixAllOccurrences\IFixAllGetFixesService.cs" />

diff --git a/...llyQualifyCodeFixProvider.SymbolResult.cs → ...llyQualifyCodeFixProvider.SymbolResult.cs b/...llyQualifyCodeFixProvider.SymbolResult.cs → ...llyQualifyCodeFixProvider.SymbolResult.cs
diff --git a/...fy/AbstractFullyQualifyCodeFixProvider.cs → ...fy/AbstractFullyQualifyCodeFixProvider.cs b/...fy/AbstractFullyQualifyCodeFixProvider.cs → ...fy/AbstractFullyQualifyCodeFixProvider.cs
@@ -66,8 +66,8 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
                 var proposedContainers =
                     matchingTypeContainers.Concat(matchingNamespaceContainers)
-                                            .Distinct()
-                                            .Take(MaxResults);
+                                          .Distinct()
+                                          .Take(MaxResults);
 
                 var displayService = project.LanguageServices.GetService<ISymbolDisplayService>();
                 var codeActions = CreateActions(context, document, diagnostic, node, semanticModel, proposedContainers, displayService).ToImmutableArray();
@@ -135,8 +135,8 @@ private async Task<Document> ProcessNode(Document document, SyntaxNode node, str
         private async Task<ImmutableArray<SymbolResult>> GetMatchingTypesAsync(
             Project project, SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken)
         {
-            // Can't be on the right hand side of binary expression (like 'dot').
             cancellationToken.ThrowIfCancellationRequested();
+
             var syntaxFacts = project.LanguageServices.GetService<ISyntaxFactsService>();
             syntaxFacts.GetNameAndArityOfSimpleName(node, out var name, out var arity);
 
@@ -155,22 +155,60 @@ private async Task<ImmutableArray<SymbolResult>> GetMatchingTypesAsync(
                 symbols = symbols.Concat(attributeSymbolAndProjectIds.SelectAsArray(t => t.Symbol));
             }
 
-            var accessibleTypeSymbols = symbols
+            var validSymbols = symbols
                 .OfType<INamedTypeSymbol>()
-                .Where(s => (arity == 0 || s.GetArity() == arity)
-                    && s.IsAccessibleWithin(semanticModel.Compilation.Assembly)
-                    && (!inAttributeContext || s.IsAttribute())
-                    && HasValidContainer(s))
+                .Where(s => IsValidNamedTypeSearchResult(semanticModel, arity, inAttributeContext, s))
                 .ToImmutableArray();
 
-            return accessibleTypeSymbols.SelectAsArray(s => new SymbolResult(s, weight: TypeWeight));
+            // Check what the current node binds to.  If it binds to any symbols, but with
+            // the wrong arity, then we don't want to suggest fully qualifying to the same
+            // type that we're already binding to.  That won't address the WrongArity problem.
+            var currentSymbolInfo = semanticModel.GetSymbolInfo(node, cancellationToken);
+            if (currentSymbolInfo.CandidateReason == CandidateReason.WrongArity)
+            {
+                validSymbols = validSymbols.WhereAsArray(
+                    s => !currentSymbolInfo.CandidateSymbols.Contains(s));
+            }
+
+            return validSymbols.SelectAsArray(s => new SymbolResult(s, weight: TypeWeight));
+        }
+
+        private static bool IsValidNamedTypeSearchResult(
+            SemanticModel semanticModel, int arity, bool inAttributeContext, INamedTypeSymbol searchResult)
+        {
+            if (arity != 0 && searchResult.GetArity() != arity)
+            {
+                // If the user supplied type arguments, then the search result has to match the 
+                // number provided.
+                return false;
+            }
+
+            if (!searchResult.IsAccessibleWithin(semanticModel.Compilation.Assembly))
+            {
+                // Search result has to be accessible from our current location.
+                return false;
+            }
+
+            if (inAttributeContext && !searchResult.IsAttribute())
+            {
+                // If we need an attribute, we have to have found an attribute.
+                return false;
+            }
+
+            if (!HasValidContainer(searchResult))
+            {
+                // Named type we find must be in a namespace, or a non-generic type.
+                return false;
+            }
+
+            return true;
         }
 
         private static bool HasValidContainer(ISymbol symbol)
         {
             var container = symbol.ContainingSymbol;
             return container is INamespaceSymbol ||
-                (container is INamedTypeSymbol && !((INamedTypeSymbol)container).IsGenericType);
+                   (container is INamedTypeSymbol parentType && !parentType.IsGenericType);
         }
 
         private async Task<ImmutableArray<SymbolResult>> GetMatchingNamespacesAsync(

diff --git a/src/Features/VisualBasic/Portable/BasicFeatures.vbproj b/src/Features/VisualBasic/Portable/BasicFeatures.vbproj
@@ -82,7 +82,7 @@
     <Compile Include="CodeFixes\Async\VisualBasicConvertToAsyncFunctionCodeFixProvider.vb" />
     <Compile Include="CodeFixes\Iterator\VisualBasicChangeToYieldCodeFixProvider.vb" />
     <Compile Include="CodeFixes\Iterator\VisualBasicConvertToIteratorCodeFixProvider.vb" />
-    <Compile Include="CodeFixes\FullyQualify\VisualBasicFullyQualifyCodeFixProvider.vb" />
+    <Compile Include="FullyQualify\VisualBasicFullyQualifyCodeFixProvider.vb" />
     <Compile Include="GenerateConstructor\GenerateConstructorCodeFixProvider.vb" />
     <Compile Include="CodeFixes\GenerateEndConstruct\GenerateEndConstructCodeFixProvider.vb" />
     <Compile Include="CodeFixes\GenerateEnumMember\GenerateEnumMemberCodeFixProvider.vb" />

diff --git a/...VisualBasicFullyQualifyCodeFixProvider.vb → ...VisualBasicFullyQualifyCodeFixProvider.vb b/...VisualBasicFullyQualifyCodeFixProvider.vb → ...VisualBasicFullyQualifyCodeFixProvider.vb