Enable using directive IntelliSense auto-completion.

N. Taylor Mullen · N. Taylor Mullen · commit d6f3a1bd223e · 2018-06-15T13:17:09.000-07:00
- Allow identical `@using` directives in the primary document. This means that if a user types `@using System` twice in the primary document (i.e. `Index.cshtml`) then we'll generate that using statement twice; this enables proper C# IntelliSense to light up on both using statements and allows auto-completion to work because there's no more magical distinction logic for Razor code that is in the current document. - Prior to this change if you had two identical using statements in the same document one would be in no-mans land mapped to nothing (not C#, not HTML) and the other would be mapped to C#; the second a user differentiated the two statements (i.e. adding a `.`) we'd distinctify the using statements resulting in an invalid completion. - This PR has an end-user impact where they will now receive the normal C# warning about having duplicate using. I treated this prior behavior more as a bug because we threw away the first using directive instance which impacted editing and in general was a silly thing to correct for the user. - Added new integration test showing how using directives are not de-duplicated in the primary document. #1255
diff --git a/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorIntermediateNodeLoweringPhase.cs b/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorIntermediateNodeLoweringPhase.cs
@@ -34,13 +34,13 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument)
 
             document.Options = codeDocument.GetCodeGenerationOptions() ?? _optionsFeature.GetOptions();
 
-            var namespaces = new Dictionary<string, SourceSpan?>(StringComparer.Ordinal);
+            IReadOnlyList<UsingReference> importedUsings = Array.Empty<UsingReference>();
 
             // The import documents should be inserted logically before the main document.
             var imports = codeDocument.GetImportSyntaxTrees();
             if (imports != null)
             {
-                var importsVisitor = new ImportsVisitor(document, builder, namespaces, syntaxTree.Options.FeatureFlags);
+                var importsVisitor = new ImportsVisitor(document, builder, syntaxTree.Options.FeatureFlags);
 
                 for (var j = 0; j < imports.Count; j++)
                 {
@@ -49,26 +49,40 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument)
                     importsVisitor.FilePath = import.Source.FilePath;
                     importsVisitor.VisitBlock(import.Root);
                 }
+
+                importedUsings = importsVisitor.Usings;
             }
 
             var tagHelperPrefix = tagHelperContext?.Prefix;
-            var visitor = new MainSourceVisitor(document, builder, namespaces, tagHelperPrefix, syntaxTree.Options.FeatureFlags)
+            var visitor = new MainSourceVisitor(document, builder, tagHelperPrefix, syntaxTree.Options.FeatureFlags)
             {
                 FilePath = syntaxTree.Source.FilePath,
             };
 
             visitor.VisitBlock(syntaxTree.Root);
 
+            // 1. Prioritize non-imported usings over imported ones.
+            // 2. Don't import usings that already exist in primary document.
+            // 3. Allow duplicate usings in primary document (C# warning).
+            var usingReferences = new List<UsingReference>(visitor.Usings);
+            for (var j = importedUsings.Count - 1; j >= 0; j--)
+            {
+                if (!usingReferences.Contains(importedUsings[j]))
+                {
+                    usingReferences.Insert(0, importedUsings[j]);
+                }
+            }
+
             // In each lowering piece above, namespaces were tracked. We render them here to ensure every
             // lowering action has a chance to add a source location to a namespace. Ultimately, closest wins.
 
             var i = 0;
-            foreach (var @namespace in namespaces)
+            foreach (var reference in usingReferences)
             {
                 var @using = new UsingDirectiveIntermediateNode()
                 {
-                    Content = @namespace.Key,
-                    Source = @namespace.Value,
+                    Content = reference.Namespace,
+                    Source = reference.Source,
                 };
 
                 builder.Insert(i++, @using);
@@ -147,21 +161,51 @@ private void ImportDirectives(DocumentIntermediateNode document)
             }
         }
 
+        private struct UsingReference : IEquatable<UsingReference>
+        {
+            public UsingReference(string @namespace, SourceSpan? source)
+            {
+                Namespace = @namespace;
+                Source = source;
+            }
+            public string Namespace { get; }
+
+            public SourceSpan? Source { get; }
+
+            public override bool Equals(object other)
+            {
+                if (other is UsingReference reference)
+                {
+                    return Equals(reference);
+                }
+
+                return false;
+            }
+            public bool Equals(UsingReference other)
+            {
+                return string.Equals(Namespace, other.Namespace, StringComparison.Ordinal);
+            }
+
+            public override int GetHashCode() => Namespace.GetHashCode();
+        }
+
         private class LoweringVisitor : ParserVisitor
         {
             protected readonly IntermediateNodeBuilder _builder;
             protected readonly DocumentIntermediateNode _document;
-            protected readonly Dictionary<string, SourceSpan?> _namespaces;
+            protected readonly List<UsingReference> _usings;
             protected readonly RazorParserFeatureFlags _featureFlags;
 
-            public LoweringVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, Dictionary<string, SourceSpan?> namespaces, RazorParserFeatureFlags featureFlags)
+            public LoweringVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, RazorParserFeatureFlags featureFlags)
             {
                 _document = document;
                 _builder = builder;
-                _namespaces = namespaces;
+                _usings = new List<UsingReference>();
                 _featureFlags = featureFlags;
             }
 
+            public IReadOnlyList<UsingReference> Usings => _usings;
+
             public string FilePath { get; set; }
 
             public override void VisitDirectiveToken(DirectiveTokenChunkGenerator chunkGenerator, Span span)
@@ -212,7 +256,7 @@ public override void VisitImportSpan(AddImportChunkGenerator chunkGenerator, Spa
             {
                 var namespaceImport = chunkGenerator.Namespace.Trim();
                 var namespaceSpan = BuildSourceSpanFromNode(span);
-                _namespaces[namespaceImport] = namespaceSpan;
+                _usings.Add(new UsingReference(namespaceImport, namespaceSpan));
             }
 
             public override void VisitAddTagHelperSpan(AddTagHelperChunkGenerator chunkGenerator, Span span)
@@ -353,8 +397,8 @@ private class MainSourceVisitor : LoweringVisitor
         {
             private readonly string _tagHelperPrefix;
 
-            public MainSourceVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, Dictionary<string, SourceSpan?> namespaces, string tagHelperPrefix, RazorParserFeatureFlags featureFlags)
-                : base(document, builder, namespaces, featureFlags)
+            public MainSourceVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, string tagHelperPrefix, RazorParserFeatureFlags featureFlags)
+                : base(document, builder, featureFlags)
             {
                 _tagHelperPrefix = tagHelperPrefix;
             }
@@ -731,8 +775,8 @@ private void AddTagHelperAttributes(IList<TagHelperAttributeNode> attributes, Ta
 
         private class ImportsVisitor : LoweringVisitor
         {
-            public ImportsVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, Dictionary<string, SourceSpan?> namespaces, RazorParserFeatureFlags featureFlags)
-                : base(document, new ImportBuilder(builder), namespaces, featureFlags)
+            public ImportsVisitor(DocumentIntermediateNode document, IntermediateNodeBuilder builder, RazorParserFeatureFlags featureFlags)
+                : base(document, new ImportBuilder(builder), featureFlags)
             {
             }
 
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs
@@ -23,6 +23,15 @@ public class CodeGenerationIntegrationTest : IntegrationTestBase
         private CSharpCompilation BaseCompilation => MvcShim.BaseCompilation.WithAssemblyName("AppCode");
 
         #region Runtime
+
+        [Fact]
+        public void UsingDirectives_Runtime()
+        {
+            var compilation = BaseCompilation;
+
+            RunRuntimeTest(compilation, new[] { "The using directive for 'System' appeared previously in this namespace" });
+        }
+
         [Fact]
         public void InvalidNamespaceAtEOF_Runtime()
         {
@@ -301,6 +310,15 @@ public void RazorPageWithNoLeadingPageDirective_Runtime()
         #endregion
 
         #region DesignTime
+
+        [Fact]
+        public void UsingDirectives_DesignTime()
+        {
+            var compilation = BaseCompilation;
+
+            RunDesignTimeTest(compilation, new[] { "The using directive for 'System' appeared previously in this namespace" });
+        }
+
         [Fact]
         public void InvalidNamespaceAtEOF_DesignTime()
         {
@@ -597,7 +615,7 @@ public void RazorPageWithNoLeadingPageDirective_DesignTime()
 
         private void RunRuntimeTest(
             CSharpCompilation baseCompilation,
-            IEnumerable<string> expectedErrors = null)
+            IEnumerable<string> expectedWarnings = null)
         {
             Assert.Empty(baseCompilation.GetDiagnostics());
 
@@ -611,12 +629,12 @@ private void RunRuntimeTest(
             // Assert
             AssertDocumentNodeMatchesBaseline(document.GetDocumentIntermediateNode());
             AssertCSharpDocumentMatchesBaseline(document.GetCSharpDocument());
-            AssertDocumentCompiles(document, baseCompilation, expectedErrors);
+            AssertDocumentCompiles(document, baseCompilation, expectedWarnings);
         }
 
         private void RunDesignTimeTest(
             CSharpCompilation baseCompilation,
-            IEnumerable<string> expectedErrors = null)
+            IEnumerable<string> expectedWarnings = null)
         {
             Assert.Empty(baseCompilation.GetDiagnostics());
 
@@ -631,13 +649,13 @@ private void RunDesignTimeTest(
             AssertDocumentNodeMatchesBaseline(document.GetDocumentIntermediateNode());
             AssertCSharpDocumentMatchesBaseline(document.GetCSharpDocument());
             AssertSourceMappingsMatchBaseline(document);
-            AssertDocumentCompiles(document, baseCompilation, expectedErrors);
+            AssertDocumentCompiles(document, baseCompilation, expectedWarnings);
         }
 
         private void AssertDocumentCompiles(
             RazorCodeDocument document,
             CSharpCompilation baseCompilation,
-            IEnumerable<string> expectedErrors = null)
+            IEnumerable<string> expectedWarnings = null)
         {
             var cSharp = document.GetCSharpDocument().GeneratedCode;
 
@@ -649,15 +667,15 @@ private void AssertDocumentCompiles(
 
             var diagnostics = compilation.GetDiagnostics();
 
-            var errors = diagnostics.Where(d => d.Severity >= DiagnosticSeverity.Warning);
+            var warnings = diagnostics.Where(d => d.Severity >= DiagnosticSeverity.Warning);
 
-            if (expectedErrors == null)
+            if (expectedWarnings == null)
             {
-                Assert.Empty(errors.Select(e => e.GetMessage()));
+                Assert.Empty(warnings);
             }
             else
             {
-                Assert.Equal(expectedErrors, errors.Select(e => e.GetMessage()));
+                Assert.Equal(expectedWarnings, warnings.Select(e => e.GetMessage()));
             }
         }
 
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml
@@ -0,0 +1,4 @@
+﻿@using System.ComponentModel
+@using System.Collections
+@using System
+@using System
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.codegen.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.codegen.cs
@@ -0,0 +1,59 @@
+// <auto-generated/>
+#pragma warning disable 1591
+namespace AspNetCore
+{
+    #line hidden
+    using TModel = global::System.Object;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Threading.Tasks;
+    using Microsoft.AspNetCore.Mvc;
+    using Microsoft.AspNetCore.Mvc.Rendering;
+    using Microsoft.AspNetCore.Mvc.ViewFeatures;
+#line 1 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml"
+using System.ComponentModel;
+
+#line default
+#line hidden
+#line 2 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml"
+using System.Collections;
+
+#line default
+#line hidden
+#line 3 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml"
+using System;
+
+#line default
+#line hidden
+#line 4 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml"
+using System;
+
+#line default
+#line hidden
+    public class TestFiles_IntegrationTests_CodeGenerationIntegrationTest_UsingDirectives : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<dynamic>
+    {
+        #pragma warning disable 219
+        private void __RazorDirectiveTokenHelpers__() {
+        }
+        #pragma warning restore 219
+        #pragma warning disable 0414
+        private static System.Object __o = null;
+        #pragma warning restore 0414
+        #pragma warning disable 1998
+        public async override global::System.Threading.Tasks.Task ExecuteAsync()
+        {
+        }
+        #pragma warning restore 1998
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; }
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; }
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; }
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; }
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper<dynamic> Html { get; private set; }
+    }
+}
+#pragma warning restore 1591
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.ir.txt
@@ -0,0 +1,46 @@
+Document - 
+    NamespaceDeclaration -  - AspNetCore
+        UsingDirective -  - TModel = global::System.Object
+        UsingDirective - (16:1,1 [32] ) - System.Collections.Generic
+        UsingDirective - (51:2,1 [17] ) - System.Linq
+        UsingDirective - (71:3,1 [28] ) - System.Threading.Tasks
+        UsingDirective - (102:4,1 [30] ) - Microsoft.AspNetCore.Mvc
+        UsingDirective - (135:5,1 [40] ) - Microsoft.AspNetCore.Mvc.Rendering
+        UsingDirective - (178:6,1 [43] ) - Microsoft.AspNetCore.Mvc.ViewFeatures
+        UsingDirective - (1:0,1 [27] UsingDirectives.cshtml) - System.ComponentModel
+        UsingDirective - (31:1,1 [24] UsingDirectives.cshtml) - System.Collections
+        UsingDirective - (58:2,1 [12] UsingDirectives.cshtml) - System
+        UsingDirective - (73:3,1 [12] UsingDirectives.cshtml) - System
+        ClassDeclaration -  - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_UsingDirectives - global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<dynamic> - 
+            DesignTimeDirective - 
+                DirectiveToken - (231:7,8 [62] ) - global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper<TModel>
+                DirectiveToken - (294:7,71 [4] ) - Html
+                DirectiveToken - (308:8,8 [54] ) - global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper
+                DirectiveToken - (363:8,63 [4] ) - Json
+                DirectiveToken - (377:9,8 [53] ) - global::Microsoft.AspNetCore.Mvc.IViewComponentHelper
+                DirectiveToken - (431:9,62 [9] ) - Component
+                DirectiveToken - (450:10,8 [43] ) - global::Microsoft.AspNetCore.Mvc.IUrlHelper
+                DirectiveToken - (494:10,52 [3] ) - Url
+                DirectiveToken - (507:11,8 [70] ) - global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider
+                DirectiveToken - (578:11,79 [23] ) - ModelExpressionProvider
+                DirectiveToken - (617:12,14 [96] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.UrlResolutionTagHelper, Microsoft.AspNetCore.Mvc.Razor
+                DirectiveToken - (729:13,14 [87] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.HeadTagHelper, Microsoft.AspNetCore.Mvc.Razor
+                DirectiveToken - (832:14,14 [87] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.BodyTagHelper, Microsoft.AspNetCore.Mvc.Razor
+            CSharpCode - 
+                IntermediateToken -  - CSharp - #pragma warning disable 0414
+            CSharpCode - 
+                IntermediateToken -  - CSharp - private static System.Object __o = null;
+            CSharpCode - 
+                IntermediateToken -  - CSharp - #pragma warning restore 0414
+            MethodDeclaration -  - public async override - global::System.Threading.Tasks.Task - ExecuteAsync
+                HtmlContent - (28:0,28 [2] UsingDirectives.cshtml)
+                    IntermediateToken - (28:0,28 [2] UsingDirectives.cshtml) - Html - \n
+                HtmlContent - (55:1,25 [2] UsingDirectives.cshtml)
+                    IntermediateToken - (55:1,25 [2] UsingDirectives.cshtml) - Html - \n
+                HtmlContent - (70:2,13 [2] UsingDirectives.cshtml)
+                    IntermediateToken - (70:2,13 [2] UsingDirectives.cshtml) - Html - \n
+            Inject - 
+            Inject - 
+            Inject - 
+            Inject - 
+            Inject - 
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.mappings.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_DesignTime.mappings.txt
@@ -0,0 +1,20 @@
+Source Location: (1:0,1 [27] TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml)
+|using System.ComponentModel|
+Generated Location: (461:13,0 [27] )
+|using System.ComponentModel|
+
+Source Location: (31:1,1 [24] TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml)
+|using System.Collections|
+Generated Location: (613:18,0 [24] )
+|using System.Collections|
+
+Source Location: (58:2,1 [12] TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml)
+|using System|
+Generated Location: (762:23,0 [12] )
+|using System|
+
+Source Location: (73:3,1 [12] TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives.cshtml)
+|using System|
+Generated Location: (899:28,0 [12] )
+|using System|
+
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_Runtime.codegen.cs
diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_Runtime.ir.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/UsingDirectives_Runtime.ir.txt
diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorIntermediateNodeLoweringPhaseIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorIntermediateNodeLoweringPhaseIntegrationTest.cs
