From cc2f397a4d09e5f812334846ddb62d06190a30d7 Mon Sep 17 00:00:00 2001 From: Tanay Parikh <TanayParikh@users.noreply.github.com> Date: Fri, 24 Sep 2021 13:38:29 -0700 Subject: [PATCH] Fix Missing `RZ10012` diagnostic for components written in certain cultures (#36907) * Update test to account for localized component names * Support Titlecase/OtherLetter * Update ComponentChildContentIntegrationTest.cs * Add SupportLocalizedComponentNames Configurable Property * PR Feedback * Test supportLocalizedComponentNames configuration * PR Feedback * Update PublicAPI.Unshipped.txt * RC2 Tooling Support for Support Localized Component Names (#36936) --- .../src/DefaultRazorCodeGenerationOptions.cs | 2 + ...efaultRazorCodeGenerationOptionsBuilder.cs | 3 ++ ...faultRazorIntermediateNodeLoweringPhase.cs | 23 +++++++-- .../src/PublicAPI.Unshipped.txt | 5 ++ .../src/RazorCodeGenerationOptions.cs | 10 ++++ .../src/RazorCodeGenerationOptionsBuilder.cs | 8 +++ .../RazorProjectEngineBuilderExtensions.cs | 31 ++++++++++++ .../ComponentChildContentIntegrationTest.cs | 49 +++++++++++++++++-- ...ComponentDiagnosticRazorIntegrationTest.cs | 44 +++++++++++++++-- .../RazorIntegrationTestBase.cs | 31 +++++++++--- 10 files changed, 189 insertions(+), 17 deletions(-) diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptions.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptions.cs index 4592d518ad0..34b642f00d4 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptions.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptions.cs @@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Razor.Language bool suppressPrimaryMethodBody, bool suppressNullabilityEnforcement, bool omitMinimizedComponentAttributeValues, + bool supportLocalizedComponentNames, bool useEnhancedLinePragma) { IndentWithTabs = indentWithTabs; @@ -26,6 +27,7 @@ namespace Microsoft.AspNetCore.Razor.Language SuppressPrimaryMethodBody = suppressPrimaryMethodBody; SuppressNullabilityEnforcement = suppressNullabilityEnforcement; OmitMinimizedComponentAttributeValues = omitMinimizedComponentAttributeValues; + SupportLocalizedComponentNames = supportLocalizedComponentNames; UseEnhancedLinePragma = useEnhancedLinePragma; } diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptionsBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptionsBuilder.cs index 005d9a0c5a7..09e912881c3 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptionsBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorCodeGenerationOptionsBuilder.cs @@ -41,6 +41,8 @@ namespace Microsoft.AspNetCore.Razor.Language public override bool OmitMinimizedComponentAttributeValues { get; set; } + public override bool SupportLocalizedComponentNames { get; set; } + public override bool UseEnhancedLinePragma { get; set; } public override RazorCodeGenerationOptions Build() @@ -55,6 +57,7 @@ namespace Microsoft.AspNetCore.Razor.Language SuppressPrimaryMethodBody, SuppressNullabilityEnforcement, OmitMinimizedComponentAttributeValues, + SupportLocalizedComponentNames, UseEnhancedLinePragma) { SuppressMetadataSourceChecksumAttributes = SuppressMetadataSourceChecksumAttributes, diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorIntermediateNodeLoweringPhase.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorIntermediateNodeLoweringPhase.cs index ff75555a1f3..d082dcbdc93 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorIntermediateNodeLoweringPhase.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorIntermediateNodeLoweringPhase.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Linq; using Microsoft.AspNetCore.Razor.Language.Components; using Microsoft.AspNetCore.Razor.Language.Extensions; @@ -1200,10 +1201,8 @@ namespace Microsoft.AspNetCore.Razor.Language { // We only want this error during the second phase of the two phase compilation. var startTagName = node.StartTag.GetTagNameWithOptionalBang(); - if (startTagName != null && startTagName.Length > 0 && char.IsUpper(startTagName, 0)) + if (!string.IsNullOrEmpty(startTagName) && LooksLikeAComponentName(_document, startTagName)) { - // A markup element that starts with an uppercase character. - // It is most likely intended to be a component. Add a warning. element.Diagnostics.Add( ComponentDiagnosticFactory.Create_UnexpectedMarkupElement(startTagName, BuildSourceSpanFromNode(node.StartTag))); } @@ -1214,6 +1213,24 @@ namespace Microsoft.AspNetCore.Razor.Language base.VisitMarkupElement(node); _builder.Pop(); + + static bool LooksLikeAComponentName(DocumentIntermediateNode document, string startTagName) + { + var category = char.GetUnicodeCategory(startTagName, 0); + + // A markup element which starts with an uppercase character is likely a component. + // + // In certain cultures, characters are not explicitly Uppercase/Lowercase, hence we must check + // the specific UnicodeCategory to see if we may still be able to treat it as a component. + // + // The goal here is to avoid clashing with any future standard-HTML elements. + // + // To avoid a breaking change, the support of localized component names (without explicit + // Uppercase classification) is behind a `SupportLocalizedComponentNames` feature flag. + return category is UnicodeCategory.UppercaseLetter || + (document.Options.SupportLocalizedComponentNames && + (category is UnicodeCategory.TitlecaseLetter || category is UnicodeCategory.OtherLetter)); + } } public override void VisitMarkupStartTag(MarkupStartTagSyntax node) diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/PublicAPI.Unshipped.txt b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/PublicAPI.Unshipped.txt index 9e776d21b99..151766771ab 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/PublicAPI.Unshipped.txt +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/PublicAPI.Unshipped.txt @@ -33,3 +33,8 @@ Microsoft.AspNetCore.Razor.Language.SourceSpan.EndCharacterIndex.get -> int virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptionsBuilder.UseEnhancedLinePragma.get -> bool virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptionsBuilder.UseEnhancedLinePragma.set -> void virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptions.UseEnhancedLinePragma.get -> bool +virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptionsBuilder.SupportLocalizedComponentNames.get -> bool +virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptionsBuilder.SupportLocalizedComponentNames.set -> void +virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptions.SupportLocalizedComponentNames.get -> bool +virtual Microsoft.AspNetCore.Razor.Language.RazorCodeGenerationOptions.SupportLocalizedComponentNames.set -> void +~static Microsoft.AspNetCore.Razor.Language.RazorProjectEngineBuilderExtensions.SetSupportLocalizedComponentNames(this Microsoft.AspNetCore.Razor.Language.RazorProjectEngineBuilder builder) -> Microsoft.AspNetCore.Razor.Language.RazorProjectEngineBuilder diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptions.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptions.cs index 437ebd9e063..69ea363f2c6 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptions.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptions.cs @@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.Razor.Language suppressPrimaryMethodBody: false, suppressNullabilityEnforcement: false, omitMinimizedComponentAttributeValues: false, + supportLocalizedComponentNames: false, useEnhancedLinePragma: true); } @@ -34,6 +35,7 @@ namespace Microsoft.AspNetCore.Razor.Language suppressPrimaryMethodBody: false, suppressNullabilityEnforcement: false, omitMinimizedComponentAttributeValues: false, + supportLocalizedComponentNames: false, useEnhancedLinePragma: true); } @@ -133,6 +135,14 @@ namespace Microsoft.AspNetCore.Razor.Language /// </summary> public virtual bool OmitMinimizedComponentAttributeValues { get; } + /// <summary> + /// Gets a value that determines if localized component names are to be supported. + /// </summary> + public virtual bool SupportLocalizedComponentNames { get; set; } + + /// <summary> + /// Gets a value that determines if enhanced line pragmas are to be utilized. + /// </summary> public virtual bool UseEnhancedLinePragma { get; } } } diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptionsBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptionsBuilder.cs index 2a51d25e8f4..19c174f30b8 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptionsBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorCodeGenerationOptionsBuilder.cs @@ -73,6 +73,14 @@ namespace Microsoft.AspNetCore.Razor.Language /// </summary> public virtual bool OmitMinimizedComponentAttributeValues { get; set; } + /// <summary> + /// Gets or sets a value that determines if localized component names are to be supported. + /// </summary> + public virtual bool SupportLocalizedComponentNames { get; set; } + + /// <summary> + /// Gets a value that determines if enhanced line pragmas are to be utilized. + /// </summary> public virtual bool UseEnhancedLinePragma { get; set; } public abstract RazorCodeGenerationOptions Build(); diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorProjectEngineBuilderExtensions.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorProjectEngineBuilderExtensions.cs index ce9e921616d..759762d52d4 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorProjectEngineBuilderExtensions.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorProjectEngineBuilderExtensions.cs @@ -92,6 +92,22 @@ namespace Microsoft.AspNetCore.Razor.Language return builder; } + /// <summary> + /// Sets the SupportLocalizedComponentNames property to make localized component name diagnostics available. + /// </summary> + /// <param name="builder">The <see cref="RazorProjectEngineBuilder"/>.</param> + /// <returns>The <see cref="RazorProjectEngineBuilder"/>.</returns> + public static RazorProjectEngineBuilder SetSupportLocalizedComponentNames(this RazorProjectEngineBuilder builder) + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + builder.Features.Add(new SetSupportLocalizedComponentNamesFeature()); + return builder; + } + public static void SetImportFeature(this RazorProjectEngineBuilder builder, IImportProjectFeature feature) { if (builder == null) @@ -302,6 +318,21 @@ namespace Microsoft.AspNetCore.Razor.Language } } + private class SetSupportLocalizedComponentNamesFeature : RazorEngineFeatureBase, IConfigureRazorCodeGenerationOptionsFeature + { + public int Order { get; set; } + + public void Configure(RazorCodeGenerationOptionsBuilder options) + { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + options.SupportLocalizedComponentNames = true; + } + } + private class ConfigureRootNamespaceFeature : IConfigureRazorCodeGenerationOptionsFeature { private readonly string _rootNamespace; diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentChildContentIntegrationTest.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentChildContentIntegrationTest.cs index 1dd72f53846..dcb1a1afc58 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentChildContentIntegrationTest.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentChildContentIntegrationTest.cs @@ -115,8 +115,10 @@ Some Content diagnostic.GetMessage(CultureInfo.CurrentCulture)); } - [Fact] - public void ChildContent_ExplicitChildContent_UnrecogizedElement_ProducesDiagnostic() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ChildContent_ExplicitChildContent_UnrecogizedElement_ProducesDiagnostic(bool supportLocalizedComponentNames) { // Arrange AdditionalSyntaxTrees.Add(RenderChildContentComponent); @@ -127,7 +129,7 @@ Some Content <ChildContent> </ChildContent> <UnrecognizedChildContent></UnrecognizedChildContent> -</RenderChildContent>"); +</RenderChildContent>", supportLocalizedComponentNames: supportLocalizedComponentNames); // Assert Assert.Collection( @@ -136,6 +138,47 @@ Some Content d => Assert.Equal("RZ9996", d.Id)); } + [Fact] + public void ChildContent_ExplicitChildContent_StartsWithCharThatIsOtherLetterCategory_WhenLocalizedComponentNamesIsAllowed() + { + // Arrange + AdditionalSyntaxTrees.Add(RenderChildContentComponent); + + // Act + var generated = CompileToCSharp(@$" +<RenderChildContent> +<ChildContent> +</ChildContent> +<ç¹ä½“å—></ç¹ä½“å—> +</RenderChildContent>", supportLocalizedComponentNames: true); + + // Assert + Assert.Collection( + generated.Diagnostics, + d => Assert.Equal("RZ10012", d.Id), + d => Assert.Equal("RZ9996", d.Id)); + } + + [Fact] + public void ChildContent_ExplicitChildContent_StartsWithCharThatIsOtherLetterCategory_WhenLocalizedComponentNamesIsDisallowed() + { + // Arrange + AdditionalSyntaxTrees.Add(RenderChildContentComponent); + + // Act + var generated = CompileToCSharp(@$" +<RenderChildContent> +<ChildContent> +</ChildContent> +<ç¹ä½“å—></ç¹ä½“å—> +</RenderChildContent>", supportLocalizedComponentNames: false); + + // Assert + Assert.Collection( + generated.Diagnostics, + d => Assert.Equal("RZ9996", d.Id)); + } + [Fact] public void ChildContent_ExplicitChildContent_UnrecogizedAttribute_ProducesDiagnostic() { diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs index 53e6857a7d3..e4942670f0c 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs @@ -43,7 +43,6 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests var diagnostic = Assert.Single(generated.Diagnostics); Assert.Equal("RZ9979", diagnostic.Id); Assert.NotNull(diagnostic.GetMessage(CultureInfo.CurrentCulture)); - } [Fact] @@ -147,8 +146,10 @@ namespace Test diagnostic.GetMessage(CultureInfo.CurrentCulture)); } - [Fact] - public void Element_DoesNotStartWithLowerCase_ReportsWarning() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Component_NotFound_ReportsWarning(bool supportLocalizedComponentNames) { // Arrange & Act var generated = CompileToCSharp(@" @@ -156,7 +157,7 @@ namespace Test @functions { public string Text { get; set; } = ""text""; -}"); +}", supportLocalizedComponentNames: supportLocalizedComponentNames); // Assert var diagnostic = Assert.Single(generated.Diagnostics); @@ -167,6 +168,41 @@ namespace Test diagnostic.GetMessage(CultureInfo.CurrentCulture)); } + [Fact] + public void Component_NotFound_StartsWithOtherLetter_WhenLocalizedComponentNamesIsAllowed_ReportsWarning() + { + // Arrange & Act + var generated = CompileToCSharp(@" +<ç¹ä½“å—></ç¹ä½“å—> + +@functions { + public string Text { get; set; } = ""text""; +}", supportLocalizedComponentNames: true); + + // Assert + var diagnostic = Assert.Single(generated.Diagnostics); + Assert.Equal("RZ10012", diagnostic.Id); + Assert.Equal(RazorDiagnosticSeverity.Warning, diagnostic.Severity); + Assert.Equal( + "Found markup element with unexpected name 'ç¹ä½“å—'. If this is intended to be a component, add a @using directive for its namespace.", + diagnostic.GetMessage(CultureInfo.CurrentCulture)); + } + + [Fact] + public void Component_NotFound_StartsWithOtherLetter_WhenLocalizedComponentNamesIsDisallowed() + { + // Arrange & Act + var generated = CompileToCSharp(@" +<ç¹ä½“å—></ç¹ä½“å—> + +@functions { + public string Text { get; set; } = ""text""; +}", supportLocalizedComponentNames: false); + + // Assert + Assert.Empty(generated.Diagnostics); + } + [Fact] public void Element_DoesNotStartWithLowerCase_OverrideWithBang_NoWarning() { diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common/Language/IntegrationTests/RazorIntegrationTestBase.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common/Language/IntegrationTests/RazorIntegrationTestBase.cs index 19f1acf7a6a..c8f23cba86b 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common/Language/IntegrationTests/RazorIntegrationTestBase.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common/Language/IntegrationTests/RazorIntegrationTestBase.cs @@ -112,7 +112,7 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests internal virtual string WorkingDirectory { get; } // intentionally private - we don't want individual tests messing with the project engine - private RazorProjectEngine CreateProjectEngine(RazorConfiguration configuration, MetadataReference[] references) + private RazorProjectEngine CreateProjectEngine(RazorConfiguration configuration, MetadataReference[] references, bool supportLocalizedComponentNames) { return RazorProjectEngine.Create(configuration, FileSystem, b => { @@ -121,6 +121,11 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests // Turn off checksums, we're testing code generation. b.Features.Add(new SuppressChecksum()); + if (supportLocalizedComponentNames) + { + b.Features.Add(new SupportLocalizedComponentNames()); + } + b.Features.Add(new TestImportProjectFeature(ImportItems)); if (LineEnding != null) @@ -168,12 +173,12 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests }; } - protected CompileToCSharpResult CompileToCSharp(string cshtmlContent, bool throwOnFailure=true, string cssScope = null) + protected CompileToCSharpResult CompileToCSharp(string cshtmlContent, bool throwOnFailure=true, string cssScope = null, bool supportLocalizedComponentNames = false) { - return CompileToCSharp(DefaultFileName, cshtmlContent, throwOnFailure, cssScope: cssScope); + return CompileToCSharp(DefaultFileName, cshtmlContent, throwOnFailure, cssScope: cssScope, supportLocalizedComponentNames: supportLocalizedComponentNames); } - protected CompileToCSharpResult CompileToCSharp(string cshtmlRelativePath, string cshtmlContent, bool throwOnFailure = true, string fileKind = null, string cssScope = null) + protected CompileToCSharpResult CompileToCSharp(string cshtmlRelativePath, string cshtmlContent, bool throwOnFailure = true, string fileKind = null, string cssScope = null, bool supportLocalizedComponentNames = false) { if (DeclarationOnly && DesignTime) { @@ -189,7 +194,7 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests { // The first phase won't include any metadata references for component discovery. This mirrors // what the build does. - var projectEngine = CreateProjectEngine(Configuration, Array.Empty<MetadataReference>()); + var projectEngine = CreateProjectEngine(Configuration, Array.Empty<MetadataReference>(), supportLocalizedComponentNames); RazorCodeDocument codeDocument; foreach (var item in AdditionalRazorItems) @@ -218,7 +223,7 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests // Add the 'temp' compilation as a metadata reference var references = BaseCompilation.References.Concat(new[] { tempAssembly.Compilation.ToMetadataReference() }).ToArray(); - projectEngine = CreateProjectEngine(Configuration, references); + projectEngine = CreateProjectEngine(Configuration, references, supportLocalizedComponentNames); // Now update the any additional files foreach (var item in AdditionalRazorItems) @@ -247,7 +252,7 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests { // For single phase compilation tests just use the base compilation's references. // This will include the built-in components. - var projectEngine = CreateProjectEngine(Configuration, BaseCompilation.References.ToArray()); + var projectEngine = CreateProjectEngine(Configuration, BaseCompilation.References.ToArray(), supportLocalizedComponentNames); var projectItem = CreateProjectItem(cshtmlRelativePath, cshtmlContent, fileKind, cssScope); @@ -446,6 +451,18 @@ namespace Microsoft.AspNetCore.Razor.Language.IntegrationTests } } + private class SupportLocalizedComponentNames : IConfigureRazorCodeGenerationOptionsFeature + { + public int Order => 0; + + public RazorEngine Engine { get; set; } + + public void Configure(RazorCodeGenerationOptionsBuilder options) + { + options.SupportLocalizedComponentNames = true; + } + } + private class ForceLineEndingPhase : RazorEnginePhaseBase { public ForceLineEndingPhase(string lineEnding) -- GitLab