From 6ca30bbfc9075bc1285df9d8a1e8d81fa20d9f9e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 28 May 2019 17:17:50 -0700 Subject: [PATCH] Add runtime support for Blazor attribute splatting (#10357) * Add basic tests for duplicate attributes * Add AddMultipleAttributes improve RTB - Adds AddMultipleAttributes - Fix RTB to de-dupe attributes - Fix RTB behaviour with boxed EventCallback (#8336) - Add lots of tests for new RTB behaviour and EventCallback * Harden EventCallback test This was being flaky while I was running E2E tests locally, and it wasn't using a resiliant equality comparison. * Add new setting on ParameterAttribute Adds the option to mark a parameter as an *extra* parameter. Why is this on ParameterAttribute and not a new type? It makes sense to make it a modifier on Parameter so you can use it both ways (explicitly set it, or allow it to collect *extras*). Added unit tests and validations for interacting with the new setting. * Add renderer tests for 'extra' parameters * Refactor Diagnostics for more analyzers * Simplify analyzer and fix CascadingParameter This is the *easy way* to write an analyzer that looks at declarations. The information that's avaialable from symbols is much more high level than syntax. Much of what's in this code today is needed to reverse engineer what the compiler does already. If you use symbols you get to benefit from all of that. Also added validation for cascading parameters to the analyzer that I think was just missing due to oversight. The overall design pattern here is what I've been converging on for the ASP.NET Core analyzers as a whole, and it seems to scale really well. * Add analyzer for types * Add analyzer for uniqueness This involved a refactor to run the analyzer per-type instead of per-property. * Fix project file * Adjust name * PR feedback on PCE and more renames * Remove unused parameter * Fix #10398 * Add E2E test * Pranavs cool feedback * Optimize silent frame removal * code check * pr feedback --- .../Analyzers/src/ComponentFacts.cs | 89 +++ .../src/ComponentParameterAnalyzer.cs | 111 +++ ...nentParametersShouldNotBePublicAnalyzer.cs | 76 -- ...ametersShouldNotBePublicCodeFixProvider.cs | 12 +- .../Analyzers/src/ComponentSymbols.cs | 65 ++ src/Components/Analyzers/src/ComponentsApi.cs | 10 +- .../Analyzers/src/DiagnosticDescriptors.cs | 41 + .../Analyzers/src/Properties/AssemblyInfo.cs | 3 + .../Analyzers/src/Resources.Designer.cs | 59 +- src/Components/Analyzers/src/Resources.resx | 74 +- ...rCaptureUnmatchedValuesHasWrongTypeTest.cs | 80 ++ ...rCaptureUnmatchedValuesMustBeUniqueTest.cs | 68 ++ ...omponentParametersShouldNotBePublicTest.cs | 42 +- .../test/ComponentsTestDeclarations.cs | 22 + ...ft.AspNetCore.Components.netstandard2.0.cs | 3 + .../Components/src/EventCallback.cs | 24 +- .../Components/src/ParameterAttribute.cs | 21 + .../src/ParameterCollectionExtensions.cs | 136 +++- .../Components/src/RenderTree/ArrayBuilder.cs | 2 +- .../src/RenderTree/RenderTreeBuilder.cs | 219 +++++- .../src/RenderTree/RenderTreeFrameType.cs | 5 + ...meterCollectionAssignmentExtensionsTest.cs | 157 +++- .../Components/test/RenderTreeBuilderTest.cs | 700 +++++++++++++++++- .../Components/test/RendererTest.cs | 85 +++ .../test/Rendering/HtmlRendererTestBase.cs | 34 + .../E2ETest/Tests/ComponentRenderingTest.cs | 20 + .../test/E2ETest/Tests/EventCallbackTest.cs | 5 +- .../DuplicateAttributesComponent.razor | 28 + ...licateAttributesOnElementChildComponent.cs | 37 + .../test/testassets/BasicTestApp/Index.razor | 1 + 30 files changed, 2060 insertions(+), 169 deletions(-) create mode 100644 src/Components/Analyzers/src/ComponentFacts.cs create mode 100644 src/Components/Analyzers/src/ComponentParameterAnalyzer.cs delete mode 100644 src/Components/Analyzers/src/ComponentParametersShouldNotBePublicAnalyzer.cs create mode 100644 src/Components/Analyzers/src/ComponentSymbols.cs create mode 100644 src/Components/Analyzers/src/DiagnosticDescriptors.cs create mode 100644 src/Components/Analyzers/src/Properties/AssemblyInfo.cs create mode 100644 src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs create mode 100644 src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs create mode 100644 src/Components/Analyzers/test/ComponentsTestDeclarations.cs create mode 100644 src/Components/test/testassets/BasicTestApp/DuplicateAttributesComponent.razor create mode 100644 src/Components/test/testassets/BasicTestApp/DuplicateAttributesOnElementChildComponent.cs diff --git a/src/Components/Analyzers/src/ComponentFacts.cs b/src/Components/Analyzers/src/ComponentFacts.cs new file mode 100644 index 0000000000..031df5586f --- /dev/null +++ b/src/Components/Analyzers/src/ComponentFacts.cs @@ -0,0 +1,89 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + internal static class ComponentFacts + { + public static bool IsAnyParameter(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + return property.GetAttributes().Any(a => + { + return a.AttributeClass == symbols.ParameterAttribute || a.AttributeClass == symbols.CascadingParameterAttribute; + }); + } + + public static bool IsParameter(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + return property.GetAttributes().Any(a => a.AttributeClass == symbols.ParameterAttribute); + } + + public static bool IsParameterWithCaptureUnmatchedValues(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + var attribute = property.GetAttributes().FirstOrDefault(a => a.AttributeClass == symbols.ParameterAttribute); + if (attribute == null) + { + return false; + } + + foreach (var kvp in attribute.NamedArguments) + { + if (string.Equals(kvp.Key, ComponentsApi.ParameterAttribute.CaptureUnmatchedValues, StringComparison.Ordinal)) + { + return kvp.Value.Value as bool? ?? false; + } + } + + return false; + } + + public static bool IsCascadingParameter(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + return property.GetAttributes().Any(a => a.AttributeClass == symbols.CascadingParameterAttribute); + } + } +} diff --git a/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs new file mode 100644 index 0000000000..20ad4ab6bd --- /dev/null +++ b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs @@ -0,0 +1,111 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ComponentParameterAnalyzer : DiagnosticAnalyzer + { + public ComponentParameterAnalyzer() + { + SupportedDiagnostics = ImmutableArray.Create(new[] + { + DiagnosticDescriptors.ComponentParametersShouldNotBePublic, + DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique, + DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType, + }); + } + + public override ImmutableArray SupportedDiagnostics { get; } + + public override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(context => + { + if (!ComponentSymbols.TryCreate(context.Compilation, out var symbols)) + { + // Types we need are not defined. + return; + } + + // This operates per-type because one of the validations we need has to look for duplicates + // defined on the same type. + context.RegisterSymbolStartAction(context => + { + var properties = new List(); + + var type = (INamedTypeSymbol)context.Symbol; + foreach (var member in type.GetMembers()) + { + if (member is IPropertySymbol property && ComponentFacts.IsAnyParameter(symbols, property)) + { + // Annotated with [Parameter] or [CascadingParameter] + properties.Add(property); + } + } + + if (properties.Count == 0) + { + return; + } + + context.RegisterSymbolEndAction(context => + { + var captureUnmatchedValuesParameters = new List(); + + // Per-property validations + foreach (var property in properties) + { + if (property.SetMethod?.DeclaredAccessibility == Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ComponentParametersShouldNotBePublic, + property.Locations[0], + property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + + if (ComponentFacts.IsParameterWithCaptureUnmatchedValues(symbols, property)) + { + captureUnmatchedValuesParameters.Add(property); + + // Check the type, we need to be able to assign a Dictionary + var conversion = context.Compilation.ClassifyConversion(symbols.ParameterCaptureUnmatchedValuesRuntimeType, property.Type); + if (!conversion.Exists || conversion.IsExplicit) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType, + property.Locations[0], + property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat), + property.Type.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat), + symbols.ParameterCaptureUnmatchedValuesRuntimeType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + } + } + + // Check if the type defines multiple CaptureUnmatchedValues parameters. Doing this outside the loop means we place the + // errors on the type. + if (captureUnmatchedValuesParameters.Count > 1) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique, + context.Symbol.Locations[0], + type.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat), + Environment.NewLine, + string.Join( + Environment.NewLine, + captureUnmatchedValuesParameters.Select(p => p.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)).OrderBy(n => n)))); + } + }); + }, SymbolKind.NamedType); + }); + } + } +} diff --git a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicAnalyzer.cs b/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicAnalyzer.cs deleted file mode 100644 index a9ef40f12c..0000000000 --- a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicAnalyzer.cs +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Components.Shared; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; -using System.Collections.Immutable; -using System.Linq; - -namespace Microsoft.AspNetCore.Components.Analyzers -{ - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class ComponentParametersShouldNotBePublicAnalyzer : DiagnosticAnalyzer - { - public const string DiagnosticId = "BL9993"; - private const string Category = "Encapsulation"; - - private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Title), Resources.ResourceManager, typeof(Resources)); - private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Format), Resources.ResourceManager, typeof(Resources)); - private static readonly LocalizableString Description = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Description), Resources.ResourceManager, typeof(Resources)); - private static DiagnosticDescriptor Rule = new DiagnosticDescriptor( - DiagnosticId, - Title, - MessageFormat, - Category, - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: Description); - - public override ImmutableArray SupportedDiagnostics - => ImmutableArray.Create(Rule); - - public override void Initialize(AnalysisContext context) - { - context.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.PropertyDeclaration); - } - - private void AnalyzeSyntax(SyntaxNodeAnalysisContext context) - { - var semanticModel = context.SemanticModel; - var declaration = (PropertyDeclarationSyntax)context.Node; - - var parameterAttribute = declaration.AttributeLists - .SelectMany(list => list.Attributes) - .Where(attr => semanticModel.GetTypeInfo(attr).Type?.ToDisplayString() == ComponentsApi.ParameterAttribute.FullTypeName) - .FirstOrDefault(); - - if (parameterAttribute != null && IsPubliclySettable(declaration)) - { - var identifierText = declaration.Identifier.Text; - if (!string.IsNullOrEmpty(identifierText)) - { - context.ReportDiagnostic(Diagnostic.Create( - Rule, - declaration.GetLocation(), - identifierText)); - } - } - } - - private static bool IsPubliclySettable(PropertyDeclarationSyntax declaration) - { - // If the property has a setter explicitly marked private/protected/internal, then it's not public - var setter = declaration.AccessorList?.Accessors.SingleOrDefault(x => x.Keyword.IsKind(SyntaxKind.SetKeyword)); - if (setter != null && setter.Modifiers.Any(x => x.IsKind(SyntaxKind.PrivateKeyword) || x.IsKind(SyntaxKind.ProtectedKeyword) || x.IsKind(SyntaxKind.InternalKeyword))) - { - return false; - } - - // Otherwise fallback to the property declaration modifiers - return declaration.Modifiers.Any(x => x.IsKind(SyntaxKind.PublicKeyword)); - } - } -} diff --git a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs b/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs index 001e761571..84555969b9 100644 --- a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs +++ b/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs @@ -1,15 +1,15 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using System.Collections.Immutable; -using System.Composition; -using System.Linq; -using System.Threading.Tasks; namespace Microsoft.AspNetCore.Components.Analyzers { @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_FixTitle), Resources.ResourceManager, typeof(Resources)); public override ImmutableArray FixableDiagnosticIds - => ImmutableArray.Create(ComponentParametersShouldNotBePublicAnalyzer.DiagnosticId); + => ImmutableArray.Create(DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id); public sealed override FixAllProvider GetFixAllProvider() { diff --git a/src/Components/Analyzers/src/ComponentSymbols.cs b/src/Components/Analyzers/src/ComponentSymbols.cs new file mode 100644 index 0000000000..ba1bfc4869 --- /dev/null +++ b/src/Components/Analyzers/src/ComponentSymbols.cs @@ -0,0 +1,65 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + internal class ComponentSymbols + { + public static bool TryCreate(Compilation compilation, out ComponentSymbols symbols) + { + if (compilation == null) + { + throw new ArgumentNullException(nameof(compilation)); + } + + var parameterAttribute = compilation.GetTypeByMetadataName(ComponentsApi.ParameterAttribute.MetadataName); + if (parameterAttribute == null) + { + symbols = null; + return false; + } + + var cascadingParameterAttribute = compilation.GetTypeByMetadataName(ComponentsApi.CascadingParameterAttribute.MetadataName); + if (cascadingParameterAttribute == null) + { + symbols = null; + return false; + } + + var dictionary = compilation.GetTypeByMetadataName("System.Collections.Generic.Dictionary`2"); + var @string = compilation.GetSpecialType(SpecialType.System_String); + var @object = compilation.GetSpecialType(SpecialType.System_Object); + if (dictionary == null || @string == null || @object == null) + { + symbols = null; + return false; + } + + var parameterCaptureUnmatchedValuesRuntimeType = dictionary.Construct(@string, @object); + + symbols = new ComponentSymbols(parameterAttribute, cascadingParameterAttribute, parameterCaptureUnmatchedValuesRuntimeType); + return true; + } + + private ComponentSymbols( + INamedTypeSymbol parameterAttribute, + INamedTypeSymbol cascadingParameterAttribute, + INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType) + { + ParameterAttribute = parameterAttribute; + CascadingParameterAttribute = cascadingParameterAttribute; + ParameterCaptureUnmatchedValuesRuntimeType = parameterCaptureUnmatchedValuesRuntimeType; + } + + public INamedTypeSymbol ParameterAttribute { get; } + + // Dictionary + public INamedTypeSymbol ParameterCaptureUnmatchedValuesRuntimeType { get; } + + public INamedTypeSymbol CascadingParameterAttribute { get; } + } +} diff --git a/src/Components/Analyzers/src/ComponentsApi.cs b/src/Components/Analyzers/src/ComponentsApi.cs index fdb83c6f05..83eb39cc2c 100644 --- a/src/Components/Analyzers/src/ComponentsApi.cs +++ b/src/Components/Analyzers/src/ComponentsApi.cs @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -namespace Microsoft.AspNetCore.Components.Shared +namespace Microsoft.AspNetCore.Components.Analyzers { // Constants for type and method names used in code-generation // Keep these in sync with the actual definitions @@ -13,6 +13,14 @@ namespace Microsoft.AspNetCore.Components.Shared { public static readonly string FullTypeName = "Microsoft.AspNetCore.Components.ParameterAttribute"; public static readonly string MetadataName = FullTypeName; + + public static readonly string CaptureUnmatchedValues = "CaptureUnmatchedValues"; + } + + public static class CascadingParameterAttribute + { + public static readonly string FullTypeName = "Microsoft.AspNetCore.Components.CascadingParameterAttribute"; + public static readonly string MetadataName = FullTypeName; } } } diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs new file mode 100644 index 0000000000..cb380e5646 --- /dev/null +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -0,0 +1,41 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + internal static class DiagnosticDescriptors + { + // Note: The Razor Compiler (including Components features) use the RZ prefix for diagnostics, so there's currently + // no change of clashing between that and the BL prefix used here. + // + // Tracking https://github.com/aspnet/AspNetCore/issues/10382 to rationalize this + public static readonly DiagnosticDescriptor ComponentParametersShouldNotBePublic = new DiagnosticDescriptor( + "BL0001", + new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Format), Resources.ResourceManager, typeof(Resources)), + "Encapsulation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Description), Resources.ResourceManager, typeof(Resources))); + + public static readonly DiagnosticDescriptor ComponentParameterCaptureUnmatchedValuesMustBeUnique = new DiagnosticDescriptor( + "BL0002", + new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesMustBeUnique_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesMustBeUnique_Format), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesMustBeUnique_Description), Resources.ResourceManager, typeof(Resources))); + + public static readonly DiagnosticDescriptor ComponentParameterCaptureUnmatchedValuesHasWrongType = new DiagnosticDescriptor( + "BL0003", + new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesHasWrongType_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesHasWrongType_Format), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesHasWrongType_Description), Resources.ResourceManager, typeof(Resources))); + } +} diff --git a/src/Components/Analyzers/src/Properties/AssemblyInfo.cs b/src/Components/Analyzers/src/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..6e6689b592 --- /dev/null +++ b/src/Components/Analyzers/src/Properties/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Analyzers.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] diff --git a/src/Components/Analyzers/src/Resources.Designer.cs b/src/Components/Analyzers/src/Resources.Designer.cs index df4ae809b0..e7f866fe08 100644 --- a/src/Components/Analyzers/src/Resources.Designer.cs +++ b/src/Components/Analyzers/src/Resources.Designer.cs @@ -10,7 +10,6 @@ namespace Microsoft.AspNetCore.Components.Analyzers { using System; - using System.Reflection; /// @@ -20,7 +19,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "15.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { @@ -40,7 +39,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.AspNetCore.Components.Analyzers.Resources", typeof(Resources).GetTypeInfo().Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.AspNetCore.Components.Analyzers.Resources", typeof(Resources).Assembly); resourceMan = temp; } return resourceMan; @@ -61,6 +60,60 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } } + /// + /// Looks up a localized string similar to Component parameters with CaptureUnmatchedValuess must be a correct type.. + /// + internal static string ComponentParameterCaptureUnmatchedValuesHasWrongType_Description { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesHasWrongType_Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component parameter '{0}' defines CaptureUnmatchedValuess but has an unsupported type '{1}'. Use a type assignable from '{2}'.. + /// + internal static string ComponentParameterCaptureUnmatchedValuesHasWrongType_Format { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesHasWrongType_Format", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component parameter with CaptureUnmatchedValues has the wrong type. + /// + internal static string ComponentParameterCaptureUnmatchedValuesHasWrongType_Title { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesHasWrongType_Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Components may only define a single parameter with CaptureUnmatchedValues.. + /// + internal static string ComponentParameterCaptureUnmatchedValuesMustBeUnique_Description { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesMustBeUnique_Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component type '{0}' defines properties multiple parameters with CaptureUnmatchedValues. Properties: {1}{2}. + /// + internal static string ComponentParameterCaptureUnmatchedValuesMustBeUnique_Format { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesMustBeUnique_Format", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component has multiple CaptureUnmatchedValues parameters. + /// + internal static string ComponentParameterCaptureUnmatchedValuesMustBeUnique_Title { + get { + return ResourceManager.GetString("ComponentParameterCaptureUnmatchedValuesMustBeUnique_Title", resourceCulture); + } + } + /// /// Looks up a localized string similar to Component parameters should not have public setters.. /// diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index ec781527eb..2996ec3082 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -1,17 +1,17 @@ - @@ -129,4 +129,22 @@ Component parameter has public setter - \ No newline at end of file + + Components may only define a single parameter with CaptureUnmatchedValues. + + + Component type '{0}' defines properties multiple parameters with CaptureUnmatchedValues. Properties: {1}{2} + + + Component has multiple CaptureUnmatchedValues parameters + + + Component parameters with CaptureUnmatchedValues must be a correct type. + + + Component parameter '{0}' defines CaptureUnmatchedValues but has an unsupported type '{1}'. Use a type assignable from '{2}'. + + + Component parameter with CaptureUnmatchedValues has the wrong type + + diff --git a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs new file mode 100644 index 0000000000..ba547ff85b --- /dev/null +++ b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs @@ -0,0 +1,80 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TestHelper; +using Xunit; + +namespace Microsoft.AspNetCore.Components.Analyzers.Test +{ + public class ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest : DiagnosticVerifier + { + [Theory] + [InlineData("System.Collections.Generic.IEnumerable>")] + [InlineData("System.Collections.Generic.Dictionary")] + [InlineData("System.Collections.Generic.IDictionary")] + [InlineData("System.Collections.Generic.IReadOnlyDictionary")] + public void IgnoresPropertiesWithSupportedType(string propertyType) + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter(CaptureUnmatchedValues = true)] {propertyType} MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPropertiesWithCaptureUnmatchedValuesFalse() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter(CaptureUnmatchedValues = false)] string MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void AddsDiagnosticForInvalidType() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter(CaptureUnmatchedValues = true)] string MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty' defines CaptureUnmatchedValues but has an unsupported type 'string'. Use a type assignable from 'System.Collections.Generic.Dictionary'.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 63) + } + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ComponentParameterAnalyzer(); + } + } +} diff --git a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs new file mode 100644 index 0000000000..0a9668182c --- /dev/null +++ b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs @@ -0,0 +1,68 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TestHelper; +using Xunit; + +namespace Microsoft.AspNetCore.Components.Analyzers.Test +{ + public class ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest : DiagnosticVerifier + { + [Fact] + public void IgnoresPropertiesWithCaptureUnmatchedValuesFalse() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using System.Collections.Generic; + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter(CaptureUnmatchedValues = false)] string MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] Dictionary MyOtherProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void AddsDiagnosticForMultipleCaptureUnmatchedValuesProperties() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using System.Collections.Generic; + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter(CaptureUnmatchedValues = true)] Dictionary MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] Dictionary MyOtherProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + var message = @"Component type 'ConsoleApplication1.TypeName' defines properties multiple parameters with CaptureUnmatchedValues. Properties: +ConsoleApplication1.TypeName.MyOtherProperty +ConsoleApplication1.TypeName.MyProperty"; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique.Id, + Message = message, + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 6, 15) + } + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ComponentParameterAnalyzer(); + } + } +} diff --git a/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs b/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs index 1a4f93a5a6..8d5df9053f 100644 --- a/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs +++ b/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Components; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; @@ -12,15 +11,6 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test { public class ComponentParametersShouldNotBePublic : CodeFixVerifier { - static string ParameterSource = $@" - namespace {typeof(ParameterAttribute).Namespace} - {{ - public class {typeof(ParameterAttribute).Name} : System.Attribute - {{ - }} - }} -"; - [Fact] public void IgnoresPublicPropertiesWithoutParameterAttribute() { @@ -31,7 +21,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test { public string MyProperty { get; set; } } - }" + ParameterSource; + }" + ComponentsTestDeclarations.Source; VerifyCSharpDiagnostic(test); } @@ -48,10 +38,10 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test { [Parameter] string MyPropertyNoModifer { get; set; } [Parameter] private string MyPropertyPrivate { get; set; } - [Parameter] protected string MyPropertyProtected { get; set; } - [Parameter] internal string MyPropertyInternal { get; set; } + [CascadingParameter] protected string MyPropertyProtected { get; set; } + [CascadingParameter] internal string MyPropertyInternal { get; set; } } - }" + ParameterSource; + }" + ComponentsTestDeclarations.Source; VerifyCSharpDiagnostic(test); } @@ -67,29 +57,29 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test class TypeName { [Parameter] public string BadProperty1 { get; set; } - [Parameter] public object BadProperty2 { get; set; } + [CascadingParameter] public object BadProperty2 { get; set; } } - }" + ParameterSource; + }" + ComponentsTestDeclarations.Source; VerifyCSharpDiagnostic(test, new DiagnosticResult { - Id = "BL9993", - Message = "Component parameter 'BadProperty1' has a public setter, but component parameters should not be publicly settable.", + Id = DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty1' has a public setter, but component parameters should not be publicly settable.", Severity = DiagnosticSeverity.Warning, Locations = new[] { - new DiagnosticResultLocation("Test0.cs", 8, 13) + new DiagnosticResultLocation("Test0.cs", 8, 39) } }, new DiagnosticResult { - Id = "BL9993", - Message = "Component parameter 'BadProperty2' has a public setter, but component parameters should not be publicly settable.", + Id = DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty2' has a public setter, but component parameters should not be publicly settable.", Severity = DiagnosticSeverity.Warning, Locations = new[] { - new DiagnosticResultLocation("Test0.cs", 9, 13) + new DiagnosticResultLocation("Test0.cs", 9, 48) } }); @@ -101,9 +91,9 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test class TypeName { [Parameter] string BadProperty1 { get; set; } - [Parameter] object BadProperty2 { get; set; } + [CascadingParameter] object BadProperty2 { get; set; } } - }" + ParameterSource); + }" + ComponentsTestDeclarations.Source); } [Fact] @@ -120,7 +110,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test [Parameter] public object MyProperty2 { get; protected set; } [Parameter] public object MyProperty2 { get; internal set; } } - }" + ParameterSource; + }" + ComponentsTestDeclarations.Source; VerifyCSharpDiagnostic(test); } @@ -132,7 +122,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() { - return new ComponentParametersShouldNotBePublicAnalyzer(); + return new ComponentParameterAnalyzer(); } } } diff --git a/src/Components/Analyzers/test/ComponentsTestDeclarations.cs b/src/Components/Analyzers/test/ComponentsTestDeclarations.cs new file mode 100644 index 0000000000..e09f156ff4 --- /dev/null +++ b/src/Components/Analyzers/test/ComponentsTestDeclarations.cs @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + public static class ComponentsTestDeclarations + { + public static readonly string Source = $@" + namespace {typeof(ParameterAttribute).Namespace} + {{ + public class {typeof(ParameterAttribute).Name} : System.Attribute + {{ + public bool CaptureUnmatchedValues {{ get; set; }} + }} + + public class {typeof(CascadingParameterAttribute).Name} : System.Attribute + {{ + }} + }} +"; + } +} diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs index a1b3d1e2f8..eecff40f30 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs @@ -373,6 +373,7 @@ namespace Microsoft.AspNetCore.Components public sealed partial class ParameterAttribute : System.Attribute { public ParameterAttribute() { } + public bool CaptureUnmatchedValues { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } } [System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)] public readonly partial struct ParameterCollection @@ -769,6 +770,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree public void AddContent(int sequence, Microsoft.AspNetCore.Components.RenderFragment fragment, T value) { } public void AddElementReferenceCapture(int sequence, System.Action elementReferenceCaptureAction) { } public void AddMarkupContent(int sequence, string markupContent) { } + public void AddMultipleAttributes(int sequence, System.Collections.Generic.IEnumerable> attributes) { } public void Clear() { } public void CloseComponent() { } public void CloseElement() { } @@ -813,6 +815,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree } public enum RenderTreeFrameType { + None = 0, Element = 1, Text = 2, Attribute = 3, diff --git a/src/Components/Components/src/EventCallback.cs b/src/Components/Components/src/EventCallback.cs index ca3e3d6adc..5a396e306b 100644 --- a/src/Components/Components/src/EventCallback.cs +++ b/src/Components/Components/src/EventCallback.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Components /// /// A bound event handler delegate. /// - public readonly struct EventCallback + public readonly struct EventCallback : IEventCallback { /// /// Gets a reference to the . @@ -60,12 +60,17 @@ namespace Microsoft.AspNetCore.Components return Receiver.HandleEventAsync(new EventCallbackWorkItem(Delegate), arg); } + + object IEventCallback.UnpackForRenderTree() + { + return RequiresExplicitReceiver ? (object)this : Delegate; + } } /// /// A bound event handler delegate. /// - public readonly struct EventCallback + public readonly struct EventCallback : IEventCallback { internal readonly MulticastDelegate Delegate; internal readonly IHandleEvent Receiver; @@ -86,7 +91,7 @@ namespace Microsoft.AspNetCore.Components /// public bool HasDelegate => Delegate != null; - // This is a hint to the runtime that Reciever is a different object than what + // This is a hint to the runtime that Receiver is a different object than what // Delegate.Target points to. This allows us to avoid boxing the command object // when building the render tree. See logic where this is used. internal bool RequiresExplicitReceiver => Receiver != null && !object.ReferenceEquals(Receiver, Delegate?.Target); @@ -111,5 +116,18 @@ namespace Microsoft.AspNetCore.Components { return new EventCallback(Receiver ?? Delegate?.Target as IHandleEvent, Delegate); } + + object IEventCallback.UnpackForRenderTree() + { + return RequiresExplicitReceiver ? (object)AsUntyped() : Delegate; + } + } + + // Used to understand boxed generic EventCallbacks + internal interface IEventCallback + { + bool HasDelegate { get; } + + object UnpackForRenderTree(); } } diff --git a/src/Components/Components/src/ParameterAttribute.cs b/src/Components/Components/src/ParameterAttribute.cs index ffe39191bf..5fb5ec2088 100644 --- a/src/Components/Components/src/ParameterAttribute.cs +++ b/src/Components/Components/src/ParameterAttribute.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Components.RenderTree; namespace Microsoft.AspNetCore.Components { @@ -11,5 +13,24 @@ namespace Microsoft.AspNetCore.Components [AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = false)] public sealed class ParameterAttribute : Attribute { + /// + /// Gets or sets a value that determines whether the parameter will capture values that + /// don't match any other parameter. + /// + /// + /// + /// allows a component to accept arbitrary additional + /// attributes, and pass them to another component, or some element of the underlying markup. + /// + /// + /// can be used on at most one parameter per component. + /// + /// + /// should only be applied to parameters of a type that + /// can be used with + /// such as . + /// + /// + public bool CaptureUnmatchedValues { get; set; } } } diff --git a/src/Components/Components/src/ParameterCollectionExtensions.cs b/src/Components/Components/src/ParameterCollectionExtensions.cs index 323662f403..fb23b5de49 100644 --- a/src/Components/Components/src/ParameterCollectionExtensions.cs +++ b/src/Components/Components/src/ParameterCollectionExtensions.cs @@ -1,11 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Components.Reflection; using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; using System.Reflection; +using Microsoft.AspNetCore.Components.Reflection; namespace Microsoft.AspNetCore.Components { @@ -41,17 +43,72 @@ namespace Microsoft.AspNetCore.Components _cachedWritersByType[targetType] = writers; } - foreach (var parameter in parameterCollection) + // The logic is split up for simplicity now that we have CaptureUnmatchedValues parameters. + if (writers.CaptureUnmatchedValuesWriter == null) { - var parameterName = parameter.Name; - if (!writers.WritersByName.TryGetValue(parameterName, out var writerWithIndex)) + // Logic for components without a CaptureUnmatchedValues parameter + foreach (var parameter in parameterCollection) { - ThrowForUnknownIncomingParameterName(targetType, parameterName); + var parameterName = parameter.Name; + if (!writers.WritersByName.TryGetValue(parameterName, out var writer)) + { + // Case 1: There is nowhere to put this value. + ThrowForUnknownIncomingParameterName(targetType, parameterName); + throw null; // Unreachable + } + + SetProperty(target, writer, parameterName, parameter.Value); + } + } + else + { + // Logic with components with a CaptureUnmatchedValues parameter + var isCaptureUnmatchedValuesParameterSetExplicitly = false; + Dictionary unmatched = null; + foreach (var parameter in parameterCollection) + { + var parameterName = parameter.Name; + if (string.Equals(parameterName, writers.CaptureUnmatchedValuesPropertyName, StringComparison.OrdinalIgnoreCase)) + { + isCaptureUnmatchedValuesParameterSetExplicitly = true; + } + + var isUnmatchedValue = !writers.WritersByName.TryGetValue(parameterName, out var writer); + if (isUnmatchedValue) + { + unmatched ??= new Dictionary(StringComparer.OrdinalIgnoreCase); + unmatched[parameterName] = parameter.Value; + } + else + { + Debug.Assert(writer != null); + SetProperty(target, writer, parameterName, parameter.Value); + } } + if (unmatched != null && isCaptureUnmatchedValuesParameterSetExplicitly) + { + // This has to be an error because we want to allow users to set the CaptureUnmatchedValues + // parameter explicitly and .... + // 1. We don't ever want to mutate a value the user gives us. + // 2. We also don't want to implicitly copy a value the user gives us. + // + // Either one of those implementation choices would do something unexpected. + ThrowForCaptureUnmatchedValuesConflict(targetType, writers.CaptureUnmatchedValuesPropertyName, unmatched); + throw null; // Unreachable + } + else if (unmatched != null) + { + // We had some unmatched values, set the CaptureUnmatchedValues property + SetProperty(target, writers.CaptureUnmatchedValuesWriter, writers.CaptureUnmatchedValuesPropertyName, unmatched); + } + } + + static void SetProperty(object target, IPropertySetter writer, string parameterName, object value) + { try { - writerWithIndex.SetValue(target, parameter.Value); + writer.SetValue(target, value); } catch (Exception ex) { @@ -93,18 +150,49 @@ namespace Microsoft.AspNetCore.Components } } - class WritersForType + private static void ThrowForCaptureUnmatchedValuesConflict(Type targetType, string parameterName, Dictionary unmatched) { - public Dictionary WritersByName { get; } + throw new InvalidOperationException( + $"The property '{parameterName}' on component type '{targetType.FullName}' cannot be set explicitly " + + $"when also used to capture unmatched values. Unmatched values:" + Environment.NewLine + + string.Join(Environment.NewLine, unmatched.Keys.OrderBy(k => k))); + } + private static void ThrowForMultipleCaptureUnmatchedValuesParameters(Type targetType) + { + // We don't care about perf here, we want to report an accurate and useful error. + var propertyNames = targetType + .GetProperties(_bindablePropertyFlags) + .Where(p => p.GetCustomAttribute()?.CaptureUnmatchedValues == true) + .Select(p => p.Name) + .OrderBy(p => p) + .ToArray(); + + throw new InvalidOperationException( + $"Multiple properties were found on component type '{targetType.FullName}' with " + + $"'{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}'. Only a single property " + + $"per type can use '{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}'. Properties:" + Environment.NewLine + + string.Join(Environment.NewLine, propertyNames)); + } + + private static void ThrowForInvalidCaptureUnmatchedValuesParameterType(Type targetType, PropertyInfo propertyInfo) + { + throw new InvalidOperationException( + $"The property '{propertyInfo.Name}' on component type '{targetType.FullName}' cannot be used " + + $"with '{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}' because it has the wrong type. " + + $"The property must be assignable from 'Dictionary'."); + } + + private class WritersForType + { public WritersForType(Type targetType) { WritersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var propertyInfo in GetCandidateBindableProperties(targetType)) { - var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute)) - || propertyInfo.IsDefined(typeof(CascadingParameterAttribute)); - if (!shouldCreateWriter) + var parameterAttribute = propertyInfo.GetCustomAttribute(); + var isParameter = parameterAttribute != null || propertyInfo.IsDefined(typeof(CascadingParameterAttribute)); + if (!isParameter) { continue; } @@ -120,8 +208,34 @@ namespace Microsoft.AspNetCore.Components } WritersByName.Add(propertyName, propertySetter); + + if (parameterAttribute != null && parameterAttribute.CaptureUnmatchedValues) + { + // This is an "Extra" parameter. + // + // There should only be one of these. + if (CaptureUnmatchedValuesWriter != null) + { + ThrowForMultipleCaptureUnmatchedValuesParameters(targetType); + } + + // It must be able to hold a Dictionary since that's what we create. + if (!propertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary))) + { + ThrowForInvalidCaptureUnmatchedValuesParameterType(targetType, propertyInfo); + } + + CaptureUnmatchedValuesWriter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo); + CaptureUnmatchedValuesPropertyName = propertyInfo.Name; + } } } + + public Dictionary WritersByName { get; } + + public IPropertySetter CaptureUnmatchedValuesWriter { get; } + + public string CaptureUnmatchedValuesPropertyName { get; } } } } diff --git a/src/Components/Components/src/RenderTree/ArrayBuilder.cs b/src/Components/Components/src/RenderTree/ArrayBuilder.cs index 918766aee9..6d38c95b32 100644 --- a/src/Components/Components/src/RenderTree/ArrayBuilder.cs +++ b/src/Components/Components/src/RenderTree/ArrayBuilder.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; diff --git a/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs b/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs index 78a174885b..79290f8e07 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Rendering; @@ -27,6 +28,8 @@ namespace Microsoft.AspNetCore.Components.RenderTree private readonly ArrayBuilder _entries = new ArrayBuilder(10); private readonly Stack _openElementIndices = new Stack(); private RenderTreeFrameType? _lastNonAttributeFrameType; + private bool _hasSeenAddMultipleAttributes; + private Dictionary _seenAttributeNames; /// /// The reserved parameter name used for supplying child content. @@ -52,6 +55,14 @@ namespace Microsoft.AspNetCore.Components.RenderTree /// A value representing the type of the element. public void OpenElement(int sequence, string elementName) { + // We are entering a new scope, since we track the "duplicate attributes" per + // element/component we might need to clean them up now. + if (_hasSeenAddMultipleAttributes) + { + var indexOfLastElementOrComponent = _openElementIndices.Peek(); + ProcessDuplicateAttributes(first: indexOfLastElementOrComponent + 1); + } + _openElementIndices.Push(_entries.Count); Append(RenderTreeFrame.Element(sequence, elementName)); } @@ -63,6 +74,14 @@ namespace Microsoft.AspNetCore.Components.RenderTree public void CloseElement() { var indexOfEntryBeingClosed = _openElementIndices.Pop(); + + // We might be closing an element with only attributes, run the duplicate cleanup pass + // if necessary. + if (_hasSeenAddMultipleAttributes) + { + ProcessDuplicateAttributes(first: indexOfEntryBeingClosed + 1); + } + ref var entry = ref _entries.Buffer[indexOfEntryBeingClosed]; entry = entry.WithElementSubtreeLength(_entries.Count - indexOfEntryBeingClosed); } @@ -157,6 +176,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree // or absence of an attribute, and false => "False" which isn't falsy in js. Append(RenderTreeFrame.Attribute(sequence, name, BoxedTrue)); } + else + { + TrackAttributeName(name); + } } /// @@ -178,6 +201,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree { Append(RenderTreeFrame.Attribute(sequence, name, value)); } + else + { + TrackAttributeName(name); + } } /// @@ -275,6 +302,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree { Append(RenderTreeFrame.Attribute(sequence, name, value)); } + else + { + TrackAttributeName(name); + } } /// @@ -308,12 +339,17 @@ namespace Microsoft.AspNetCore.Components.RenderTree // so we can get it out on the other side. Append(RenderTreeFrame.Attribute(sequence, name, (object)value)); } - else + else if (value.HasDelegate) { // In the common case the receiver is also the delegate's target, so we // just need to retain the delegate. This allows us to avoid an allocation. Append(RenderTreeFrame.Attribute(sequence, name, value.Delegate)); } + else + { + // Track the attribute name if needed since we elided the frame. + TrackAttributeName(name); + } } /// @@ -347,12 +383,17 @@ namespace Microsoft.AspNetCore.Components.RenderTree // need to preserve the type of an EventCallback when it's invoked from the DOM. Append(RenderTreeFrame.Attribute(sequence, name, (object)value.AsUntyped())); } - else + else if (value.HasDelegate) { // In the common case the receiver is also the delegate's target, so we // just need to retain the delegate. This allows us to avoid an allocation. Append(RenderTreeFrame.Attribute(sequence, name, value.Delegate)); } + else + { + // Track the attribute name if needed since we elided the frame. + TrackAttributeName(name); + } } /// @@ -372,7 +413,8 @@ namespace Microsoft.AspNetCore.Components.RenderTree { if (value == null) { - // Do nothing, treat 'null' attribute values for elements as a conditional attribute. + // Treat 'null' attribute values for elements as a conditional attribute. + TrackAttributeName(name); } else if (value is bool boolValue) { @@ -380,8 +422,22 @@ namespace Microsoft.AspNetCore.Components.RenderTree { Append(RenderTreeFrame.Attribute(sequence, name, BoxedTrue)); } - - // Don't add anything for false bool value. + else + { + // Don't add anything for false bool value. + TrackAttributeName(name); + } + } + else if (value is IEventCallback callbackValue) + { + if (callbackValue.HasDelegate) + { + Append(RenderTreeFrame.Attribute(sequence, name, callbackValue.UnpackForRenderTree())); + } + else + { + TrackAttributeName(name); + } } else if (value is MulticastDelegate) { @@ -395,6 +451,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree } else if (_lastNonAttributeFrameType == RenderTreeFrameType.Component) { + // If this is a component, we always want to preserve the original type. Append(RenderTreeFrame.Attribute(sequence, name, value)); } else @@ -425,6 +482,40 @@ namespace Microsoft.AspNetCore.Components.RenderTree Append(frame.WithAttributeSequence(sequence)); } + /// + /// Adds frames representing multiple attributes with the same sequence number. + /// + /// The attribute value type. + /// An integer that represents the position of the instruction in the source code. + /// A collection of key-value pairs representing attributes. + public void AddMultipleAttributes(int sequence, IEnumerable> attributes) + { + // NOTE: The IEnumerable> is the simplest way to support a variety of + // different types like IReadOnlyDictionary<>, Dictionary<>, and IDictionary<>. + // + // None of those types are contravariant, and since we want to support attributes having a value + // of type object, the simplest thing to do is drop down to IEnumerable> which + // is contravariant. This also gives us things like List> and KeyValuePair<>[] + // for free even though we don't expect those types to be common. + + // Calling this up-front just to make sure we validate before mutating anything. + AssertCanAddAttribute(); + + if (attributes != null) + { + _hasSeenAddMultipleAttributes = true; + + foreach (var attribute in attributes) + { + // This will call the AddAttribute(int, string, object) overload. + // + // This is fine because we try to make the object overload behave identically + // to the others. + AddAttribute(sequence, attribute.Key, attribute.Value); + } + } + } + /// /// Appends a frame representing a child component. /// @@ -482,6 +573,14 @@ namespace Microsoft.AspNetCore.Components.RenderTree private void OpenComponentUnchecked(int sequence, Type componentType) { + // We are entering a new scope, since we track the "duplicate attributes" per + // element/component we might need to clean them up now. + if (_hasSeenAddMultipleAttributes) + { + var indexOfLastElementOrComponent = _openElementIndices.Peek(); + ProcessDuplicateAttributes(first: indexOfLastElementOrComponent + 1); + } + _openElementIndices.Push(_entries.Count); Append(RenderTreeFrame.ChildComponent(sequence, componentType)); } @@ -493,6 +592,14 @@ namespace Microsoft.AspNetCore.Components.RenderTree public void CloseComponent() { var indexOfEntryBeingClosed = _openElementIndices.Pop(); + + // We might be closing a component with only attributes. Run the attribute cleanup pass + // if necessary. + if (_hasSeenAddMultipleAttributes) + { + ProcessDuplicateAttributes(first: indexOfEntryBeingClosed + 1); + } + ref var entry = ref _entries.Buffer[indexOfEntryBeingClosed]; entry = entry.WithComponentSubtreeLength(_entries.Count - indexOfEntryBeingClosed); } @@ -579,6 +686,8 @@ namespace Microsoft.AspNetCore.Components.RenderTree _entries.Clear(); _openElementIndices.Clear(); _lastNonAttributeFrameType = null; + _hasSeenAddMultipleAttributes = false; + _seenAttributeNames?.Clear(); } /// @@ -590,13 +699,111 @@ namespace Microsoft.AspNetCore.Components.RenderTree private void Append(in RenderTreeFrame frame) { + var frameType = frame.FrameType; _entries.Append(frame); - var frameType = frame.FrameType; if (frameType != RenderTreeFrameType.Attribute) { _lastNonAttributeFrameType = frame.FrameType; } } + + // Internal for testing + internal void ProcessDuplicateAttributes(int first) + { + Debug.Assert(_hasSeenAddMultipleAttributes); + + // When AddMultipleAttributes method has been called, we need to postprocess attributes while closing + // the element/component. However, we also don't know the end index we should look at because it + // will contain nested content. + var buffer = _entries.Buffer; + var last = _entries.Count - 1; + + for (var i = first; i <= last; i++) + { + if (buffer[i].FrameType != RenderTreeFrameType.Attribute) + { + last = i - 1; + break; + } + } + + // Now that we've found the last attribute, we can iterate backwards and process duplicates. + var seenAttributeNames = (_seenAttributeNames ??= new Dictionary(StringComparer.OrdinalIgnoreCase)); + for (var i = last; i >= first; i--) + { + ref var frame = ref buffer[i]; + Debug.Assert(frame.FrameType == RenderTreeFrameType.Attribute, $"Frame type is {frame.FrameType} at {i}"); + + if (!seenAttributeNames.TryGetValue(frame.AttributeName, out var index)) + { + // This is the first time seeing this attribute name. Add to the dictionary and move on. + seenAttributeNames.Add(frame.AttributeName, i); + } + else if (index < i) + { + // This attribute is overriding a "silent frame" where we didn't create a frame for an AddAttribute call. + // This is the case for a null event handler, or bool false value. + // + // We need to update our tracking, in case the attribute appeared 3 or more times. + seenAttributeNames[frame.AttributeName] = i; + } + else if (index > i) + { + // This attribute has been overridden. For now, blank out its name to *mark* it. We'll do a pass + // later to wipe it out. + frame = default; + } + else + { + // OK so index == i. How is that possible? Well it's possible for a "silent frame" immediately + // followed by setting the same attribute. Think of it this way, when we create a "silent frame" + // we have to track that attribute name with *some* index. + // + // The only index value we can safely use is _entries.Count (next available). This is fine because + // we never use these indexes to look stuff up, only for comparison. + // + // That gets you here, and there's no action to take. + } + } + + // This is the pass where we cleanup attributes that have been wiped out. + // + // We copy the entries we're keeping into the earlier parts of the list (preserving order). + // + // Note that we iterate to the end of the list here, there might be additional frames after the attributes + // (ref) or content) that need to move to the left. + var offset = first; + for (var i = first; i < _entries.Count; i++) + { + ref var frame = ref buffer[i]; + if (frame.FrameType != RenderTreeFrameType.None) + { + buffer[offset++] = frame; + } + } + + // Clean up now unused space at the end of the list. + var residue = _entries.Count - offset; + for (var i = 0; i < residue; i++) + { + _entries.RemoveLast(); + } + + seenAttributeNames.Clear(); + _hasSeenAddMultipleAttributes = false; + } + + // Internal for testing + internal void TrackAttributeName(string name) + { + if (!_hasSeenAddMultipleAttributes) + { + return; + } + + var seenAttributeNames = (_seenAttributeNames ??= new Dictionary(StringComparer.OrdinalIgnoreCase)); + seenAttributeNames[name] = _entries.Count; // See comment in ProcessAttributes for why this is OK. + } } } diff --git a/src/Components/Components/src/RenderTree/RenderTreeFrameType.cs b/src/Components/Components/src/RenderTree/RenderTreeFrameType.cs index 799185e202..09630e3fcf 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeFrameType.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeFrameType.cs @@ -8,6 +8,11 @@ namespace Microsoft.AspNetCore.Components.RenderTree /// public enum RenderTreeFrameType: int { + /// + /// Used only for unintialized frames. + /// + None = 0, + /// /// Represents a container for other frames. /// diff --git a/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs b/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs index 44c58dd4d6..7193081888 100644 --- a/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs @@ -4,8 +4,8 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.Test.Helpers; using Xunit; @@ -138,6 +138,142 @@ namespace Microsoft.AspNetCore.Components.Test ex.Message); } + [Fact] + public void SettingCaptureUnmatchedValuesParameterExplicitlyWorks() + { + // Arrange + var target = new HasCaptureUnmatchedValuesProperty(); + var value = new Dictionary(); + var parameterCollection = new ParameterCollectionBuilder + { + { nameof(HasCaptureUnmatchedValuesProperty.CaptureUnmatchedValues), value }, + }.Build(); + + // Act + parameterCollection.SetParameterProperties(target); + + // Assert + Assert.Same(value, target.CaptureUnmatchedValues); + } + + [Fact] + public void SettingCaptureUnmatchedValuesParameterWithUnmatchedValuesWorks() + { + // Arrange + var target = new HasCaptureUnmatchedValuesProperty(); + var parameterCollection = new ParameterCollectionBuilder + { + { nameof(HasCaptureUnmatchedValuesProperty.StringProp), "hi" }, + { "test1", 123 }, + { "test2", 456 }, + }.Build(); + + // Act + parameterCollection.SetParameterProperties(target); + + // Assert + Assert.Equal("hi", target.StringProp); + Assert.Collection( + target.CaptureUnmatchedValues.OrderBy(kvp => kvp.Key), + kvp => + { + Assert.Equal("test1", kvp.Key); + Assert.Equal(123, kvp.Value); + }, + kvp => + { + Assert.Equal("test2", kvp.Key); + Assert.Equal(456, kvp.Value); + }); + } + + [Fact] + public void SettingCaptureUnmatchedValuesParameterExplicitlyAndImplicitly_Throws() + { + // Arrange + var target = new HasCaptureUnmatchedValuesProperty(); + var parameterCollection = new ParameterCollectionBuilder + { + { nameof(HasCaptureUnmatchedValuesProperty.CaptureUnmatchedValues), new Dictionary() }, + { "test1", 123 }, + { "test2", 456 }, + }.Build(); + + // Act + var ex = Assert.Throws(() => parameterCollection.SetParameterProperties(target)); + + // Assert + Assert.Equal( + $"The property '{nameof(HasCaptureUnmatchedValuesProperty.CaptureUnmatchedValues)}' on component type '{typeof(HasCaptureUnmatchedValuesProperty).FullName}' cannot be set explicitly when " + + $"also used to capture unmatched values. Unmatched values:" + Environment.NewLine + + $"test1" + Environment.NewLine + + $"test2", + ex.Message); + } + + [Fact] + public void SettingCaptureUnmatchedValuesParameterExplicitlyAndImplicitly_ReverseOrder_Throws() + { + // Arrange + var target = new HasCaptureUnmatchedValuesProperty(); + var parameterCollection = new ParameterCollectionBuilder + { + { "test2", 456 }, + { "test1", 123 }, + { nameof(HasCaptureUnmatchedValuesProperty.CaptureUnmatchedValues), new Dictionary() }, + }.Build(); + + // Act + var ex = Assert.Throws(() => parameterCollection.SetParameterProperties(target)); + + // Assert + Assert.Equal( + $"The property '{nameof(HasCaptureUnmatchedValuesProperty.CaptureUnmatchedValues)}' on component type '{typeof(HasCaptureUnmatchedValuesProperty).FullName}' cannot be set explicitly when " + + $"also used to capture unmatched values. Unmatched values:" + Environment.NewLine + + $"test1" + Environment.NewLine + + $"test2", + ex.Message); + } + + [Fact] + public void HasDuplicateCaptureUnmatchedValuesParameters_Throws() + { + // Arrange + var target = new HasDupliateCaptureUnmatchedValuesProperty(); + var parameterCollection = new ParameterCollectionBuilder().Build(); + + // Act + var ex = Assert.Throws(() => parameterCollection.SetParameterProperties(target)); + + // Assert + Assert.Equal( + $"Multiple properties were found on component type '{typeof(HasDupliateCaptureUnmatchedValuesProperty).FullName}' " + + $"with '{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}'. " + + $"Only a single property per type can use '{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}'. " + + $"Properties:" + Environment.NewLine + + $"{nameof(HasDupliateCaptureUnmatchedValuesProperty.CaptureUnmatchedValuesProp1)}" + Environment.NewLine + + $"{nameof(HasDupliateCaptureUnmatchedValuesProperty.CaptureUnmatchedValuesProp2)}", + ex.Message); + } + + [Fact] + public void HasCaptureUnmatchedValuesParameteterWithWrongType_Throws() + { + // Arrange + var target = new HasWrongTypeCaptureUnmatchedValuesProperty(); + var parameterCollection = new ParameterCollectionBuilder().Build(); + + // Act + var ex = Assert.Throws(() => parameterCollection.SetParameterProperties(target)); + + // Assert + Assert.Equal( + $"The property '{nameof(HasWrongTypeCaptureUnmatchedValuesProperty.CaptureUnmatchedValuesProp)}' on component type '{typeof(HasWrongTypeCaptureUnmatchedValuesProperty).FullName}' cannot be used with " + + $"'{nameof(ParameterAttribute)}.{nameof(ParameterAttribute.CaptureUnmatchedValues)}' because it has the wrong type. " + + $"The property must be assignable from 'Dictionary'.", + ex.Message); + } + [Fact] public void IncomingParameterValueMismatchesDeclaredParameterType_Throws() { @@ -273,6 +409,25 @@ namespace Microsoft.AspNetCore.Components.Test [Parameter] new int IntProp { get; set; } } + class HasCaptureUnmatchedValuesProperty + { + [Parameter] internal int IntProp { get; set; } + [Parameter] internal string StringProp { get; set; } + [Parameter] internal object ObjectProp { get; set; } + [Parameter(CaptureUnmatchedValues = true)] internal IReadOnlyDictionary CaptureUnmatchedValues { get; set; } + } + + class HasDupliateCaptureUnmatchedValuesProperty + { + [Parameter(CaptureUnmatchedValues = true)] internal Dictionary CaptureUnmatchedValuesProp1 { get; set; } + [Parameter(CaptureUnmatchedValues = true)] internal IDictionary CaptureUnmatchedValuesProp2 { get; set; } + } + + class HasWrongTypeCaptureUnmatchedValuesProperty + { + [Parameter(CaptureUnmatchedValues = true)] internal KeyValuePair[] CaptureUnmatchedValuesProp { get; set; } + } + class ParameterCollectionBuilder : IEnumerable { private readonly List<(string Name, object Value)> _keyValuePairs diff --git a/src/Components/Components/test/RenderTreeBuilderTest.cs b/src/Components/Components/test/RenderTreeBuilderTest.cs index ac05e9b195..c92ee63658 100644 --- a/src/Components/Components/test/RenderTreeBuilderTest.cs +++ b/src/Components/Components/test/RenderTreeBuilderTest.cs @@ -1,13 +1,14 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Components; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.Test.Helpers; -using System; -using System.Linq; -using System.Threading.Tasks; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Components.Test @@ -245,6 +246,200 @@ namespace Microsoft.AspNetCore.Components.Test frame => AssertFrame.Text(frame, "some text")); } + [Fact] + public void CanAddMultipleAttributes_AllowsNull() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + + // Act + builder.OpenElement(0, "myelement"); + builder.AddMultipleAttributes(0, null); + builder.CloseElement(); + + // Assert + var frames = builder.GetFrames().AsEnumerable().ToArray(); + Assert.Collection( + frames, + frame => AssertFrame.Element(frame, "myelement", 1)); + } + + [Fact] + public void CanAddMultipleAttributes_InterspersedWithOtherAttributes() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + Action eventHandler = eventInfo => { }; + + // Act + builder.OpenElement(0, "myelement"); + builder.AddAttribute(0, "attribute1", "value 1"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "attribute1", "test1" }, + { "attribute2", true }, + { "attribute3", eventHandler }, + }); + builder.AddAttribute(0, "ATTRIBUTE2", true); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "attribute4", "test4" }, + { "attribute5", false }, + { "attribute6", eventHandler }, + }); + + // Null or false values don't create frames of their own, but they can + // "knock out" earlier values. + builder.AddAttribute(0, "attribute6", false); + builder.AddAttribute(0, "attribute4", (string)null); + + builder.AddAttribute(0, "attribute7", "the end"); + builder.CloseElement(); + + // Assert + var frames = builder.GetFrames().AsEnumerable().ToArray(); + Assert.Collection( + frames, + frame => AssertFrame.Element(frame, "myelement", 5), + frame => AssertFrame.Attribute(frame, "attribute1", "test1"), + frame => AssertFrame.Attribute(frame, "attribute3", eventHandler), + frame => AssertFrame.Attribute(frame, "ATTRIBUTE2", true), + frame => AssertFrame.Attribute(frame, "attribute7", "the end")); + } + + [Fact] + public void CanAddMultipleAttributes_DictionaryString() + { + var attributes = new Dictionary + { + { "attribute1", "test1" }, + { "attribute2", "123" }, + { "attribute3", "456" }, + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + [Fact] + public void CanAddMultipleAttributes_DictionaryObject() + { + var attributes = new Dictionary + { + { "attribute1", "test1" }, + { "attribute2", "123" }, + { "attribute3", true }, + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + [Fact] + public void CanAddMultipleAttributes_IReadOnlyDictionaryString() + { + var attributes = new Dictionary + { + { "attribute1", "test1" }, + { "attribute2", "123" }, + { "attribute3", "456" }, + }; + + // Act & Assert + CanAddMultipleAttributesTest((IReadOnlyDictionary)attributes); + } + + [Fact] + public void CanAddMultipleAttributes_IReadOnlyDictionaryObject() + { + var attributes = new Dictionary + { + { "attribute1", "test1" }, + { "attribute2", "123" }, + { "attribute3", true }, + }; + + // Act & Assert + CanAddMultipleAttributesTest((IReadOnlyDictionary)attributes); + } + + [Fact] + public void CanAddMultipleAttributes_ListKvpString() + { + var attributes = new List>() + { + new KeyValuePair("attribute1", "test1"), + new KeyValuePair("attribute2", "123"), + new KeyValuePair("attribute3", "456"), + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + [Fact] + public void CanAddMultipleAttributes_ListKvpObject() + { + var attributes = new List>() + { + new KeyValuePair("attribute1", "test1"), + new KeyValuePair("attribute2", "123"), + new KeyValuePair("attribute3", true), + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + [Fact] + public void CanAddMultipleAttributes_ArrayKvpString() + { + var attributes = new KeyValuePair[] + { + new KeyValuePair("attribute1", "test1"), + new KeyValuePair("attribute2", "123"), + new KeyValuePair("attribute3", "456"), + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + [Fact] + public void CanAddMultipleAttributes_ArrayKvpObject() + { + var attributes = new KeyValuePair[] + { + new KeyValuePair("attribute1", "test1"), + new KeyValuePair("attribute2", "123"), + new KeyValuePair("attribute3", true), + }; + + // Act & Assert + CanAddMultipleAttributesTest(attributes); + } + + private void CanAddMultipleAttributesTest(IEnumerable> attributes) + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + + // Act + builder.OpenElement(0, "myelement"); + builder.AddMultipleAttributes(0, attributes); + builder.CloseElement(); + + // Assert + var frames = builder.GetFrames().AsEnumerable().ToArray(); + + var i = 1; + foreach (var attribute in attributes) + { + var frame = frames[i++]; + AssertFrame.Attribute(frame, attribute.Key, attribute.Value); + } + } + [Fact] public void CannotAddAttributeAtRoot() { @@ -859,6 +1054,160 @@ namespace Microsoft.AspNetCore.Components.Test frame => AssertFrame.Attribute(frame, "attr", value, 1)); } + [Fact] + public void AddAttribute_Element_EventCallback_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = new EventCallback(null, new Action(() => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback.Delegate, 1)); + } + + [Fact] + public void AddAttribute_Element_EventCallback_Default_DoesNotAddFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = default(EventCallback); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 1, 0)); + } + + [Fact] + public void AddAttribute_Element_EventCallbackWithReceiver_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action(() => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback, 1)); + } + + [Fact] + public void AddAttribute_Component_EventCallback_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action(() => { })); + + // Act + builder.OpenComponent(0); + builder.AddAttribute(1, "attr", callback); + builder.CloseComponent(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Component(frame, 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback, 1)); + } + + [Fact] + public void AddAttribute_Element_EventCallbackOfT_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = new EventCallback(null, new Action((s) => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback.Delegate, 1)); + } + + [Fact] + public void AddAttribute_Element_EventCallbackOfT_Default_DoesNotAddFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = default(EventCallback); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 1, 0)); + } + + [Fact] + public void AddAttribute_Element_EventCallbackWithReceiverOfT_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action((s) => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", new EventCallback(callback.Receiver, callback.Delegate), 1)); + } + + [Fact] + public void AddAttribute_Component_EventCallbackOfT_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action((s) => { })); + + // Act + builder.OpenComponent(0); + builder.AddAttribute(1, "attr", callback); + builder.CloseComponent(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Component(frame, 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback, 1)); + } + [Fact] public void AddAttribute_Element_ObjectBoolTrue_AddsFrame() { @@ -1030,6 +1379,140 @@ namespace Microsoft.AspNetCore.Components.Test frame => AssertFrame.Attribute(frame, "attr", value, 1)); } + [Fact] + public void AddAttribute_Element_ObjectEventCallback_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = new EventCallback(null, new Action(() => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback.Delegate, 1)); + } + + [Fact] + public void AddAttribute_Element_ObjectEventCallback_Default_DoesNotAddFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = default(EventCallback); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 1, 0)); + } + + [Fact] + public void AddAttribute_Element_ObjectEventCallbackWithReceiver_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action(() => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback, 1)); + } + + [Fact] + public void AddAttribute_Component_ObjectEventCallback_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action(() => { })); + + // Act + builder.OpenComponent(0); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseComponent(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Component(frame, 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback, 1)); + } + + [Fact] + public void AddAttribute_Element_ObjectEventCallbackOfT_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = new EventCallback(null, new Action((s) => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", callback.Delegate, 1)); + } + + [Fact] + public void AddAttribute_Element_ObjectEventCallbackOfT_Default_DoesNotAddFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var callback = default(EventCallback); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 1, 0)); + } + + [Fact] + public void AddAttribute_Element_ObjectEventCallbackWithReceiverOfT_AddsFrame() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + var receiver = Mock.Of(); + var callback = new EventCallback(receiver, new Action((s) => { })); + + // Act + builder.OpenElement(0, "elem"); + builder.AddAttribute(1, "attr", (object)callback); + builder.CloseElement(); + + // Assert + Assert.Collection( + builder.GetFrames().AsEnumerable(), + frame => AssertFrame.Element(frame, "elem", 2, 0), + frame => AssertFrame.Attribute(frame, "attr", new EventCallback(callback.Receiver, callback.Delegate), 1)); + } + [Fact] public void AddAttribute_Element_ObjectNull_IgnoresFrame() { @@ -1148,6 +1631,215 @@ namespace Microsoft.AspNetCore.Components.Test Assert.Equal("value", ex.ParamName); } + [Fact] + public void ProcessDuplicateAttributes_DoesNotRemoveDuplicatesWithoutAddMultipleAttributes() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "id", "hi"); + builder.AddAttribute(0, "id", "bye"); + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 3, 0), + f => AssertFrame.Attribute(f, "id", "hi"), + f => AssertFrame.Attribute(f, "id", "bye")); + } + + + [Fact] + public void ProcessDuplicateAttributes_StopsAtFirstNonAttributeFrame_Capture() + { + // Arrange + var capture = (Action)((_) => { }); + + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "id", "hi"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.AddElementReferenceCapture(0, capture); + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 3, 0), + f => AssertFrame.Attribute(f, "id", "bye"), + f => AssertFrame.ElementReferenceCapture(f, capture)); + } + + [Fact] + public void ProcessDuplicateAttributes_StopsAtFirstNonAttributeFrame_Content() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "id", "hi"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.AddContent(0, "hey"); + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 3, 0), + f => AssertFrame.Attribute(f, "id", "bye"), + f => AssertFrame.Text(f, "hey")); + } + + [Fact] + public void ProcessDuplicateAttributes_CanRemoveDuplicateInsideElement() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "id", "hi"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 2, 0), + f => AssertFrame.Attribute(f, "id", "bye")); + } + + [Fact] + public void ProcessDuplicateAttributes_CanRemoveDuplicateInsideComponent() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenComponent(0); + builder.AddAttribute(0, "id", "hi"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.CloseComponent(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Component(f, 2, 0), + f => AssertFrame.Attribute(f, "id", "bye")); + } + + // This covers a special case we have to handle explicitly in the RTB logic. + [Fact] + public void ProcessDuplicateAttributes_SilentFrameFollowedBySameAttribute() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenComponent(0); + builder.AddAttribute(0, "id", (string)null); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.CloseComponent(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Component(f, 2, 0), + f => AssertFrame.Attribute(f, "id", "bye")); + } + + [Fact] + public void ProcessDuplicateAttributes_DoesNotRemoveDuplicatesInsideChildElement() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "id", "hi"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "id", "bye" }, + }); + builder.OpenElement(0, "strong"); + builder.AddAttribute(0, "id", "hi"); + builder.AddAttribute(0, "id", "bye"); + builder.CloseElement(); + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 5, 0), + f => AssertFrame.Attribute(f, "id", "bye"), + f => AssertFrame.Element(f, "strong", 3), + f => AssertFrame.Attribute(f, "id", "hi"), + f => AssertFrame.Attribute(f, "id", "bye")); + } + + [Fact] + public void ProcessDuplicateAttributes_CanRemoveOverwrittenAttributes() + { + // Arrange + var builder = new RenderTreeBuilder(new TestRenderer()); + builder.OpenElement(0, "div"); + builder.AddAttribute(0, "A", "hi"); + builder.AddAttribute(0, "2", new EventCallback(null, (Action)(() => { }))); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "a", null }, // Replace with null value (case-insensitive) + { "2", false }, // Replace with 'false' + { "3", "hey there" }, // Add a new value + }); + builder.AddAttribute(0, "3", "see ya"); // Overwrite value added by splat + builder.AddAttribute(0, "4", false); // Add a false value + builder.AddAttribute(0, "5", "another one"); + builder.AddMultipleAttributes(0, new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "5", null }, // overwrite value with null + { "6", new EventCallback(null, (Action)(() =>{ })) }, + }); + builder.AddAttribute(0, "6", default(EventCallback)); // Replace with a 'silent' EventCallback + builder.CloseElement(); + + // Act + var frames = builder.GetFrames().AsEnumerable(); + + // Assert + Assert.Collection( + frames, + f => AssertFrame.Element(f, "div", 2, 0), + f => AssertFrame.Attribute(f, "3", "see ya")); + } + private class TestComponent : IComponent { public void Configure(RenderHandle renderHandle) { } diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs index 8c8d9b1d49..2c3e110e41 100644 --- a/src/Components/Components/test/RendererTest.cs +++ b/src/Components/Components/test/RendererTest.cs @@ -1900,6 +1900,76 @@ namespace Microsoft.AspNetCore.Components.Test AssertFrame.Text(renderer.Batches[1].ReferenceFrames[0], "second"); } + [Fact] + public void ReRendersChildComponentWhenUnmatchedValuesChange() + { + // Arrange: First render + var renderer = new TestRenderer(); + var firstRender = true; + var component = new TestComponent(builder => + { + builder.OpenComponent(1); + builder.AddAttribute(1, "class", firstRender ? "first" : "second"); + builder.AddAttribute(2, "id", "some_text"); + builder.AddAttribute(3, nameof(MyStrongComponent.Text), "hi there."); + builder.CloseComponent(); + }); + + var rootComponentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + + var childComponentId = renderer.Batches.Single() + .ReferenceFrames + .Single(frame => frame.FrameType == RenderTreeFrameType.Component) + .ComponentId; + + // Act: Second render + firstRender = false; + component.TriggerRender(); + var diff = renderer.Batches[1].DiffsByComponentId[childComponentId].Single(); + + // Assert + Assert.Collection(diff.Edits, + edit => + { + Assert.Equal(RenderTreeEditType.SetAttribute, edit.Type); + Assert.Equal(0, edit.ReferenceFrameIndex); + }); + AssertFrame.Attribute(renderer.Batches[1].ReferenceFrames[0], "class", "second"); + } + + // This is a sanity check that diffs of "unmatched" values *just work* without any specialized + // code in the renderer to handle it. All of the data that's used in the diff is contained in + // the render tree, and the diff process does not need to inspect the state of the component. + [Fact] + public void ReRendersDoesNotReRenderChildComponentWhenUnmatchedValuesDoNotChange() + { + // Arrange: First render + var renderer = new TestRenderer(); + var component = new TestComponent(builder => + { + builder.OpenComponent(1); + builder.AddAttribute(1, "class", "cool-beans"); + builder.AddAttribute(2, "id", "some_text"); + builder.AddAttribute(3, nameof(MyStrongComponent.Text), "hi there."); + builder.CloseComponent(); + }); + + var rootComponentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + + var childComponentId = renderer.Batches.Single() + .ReferenceFrames + .Single(frame => frame.FrameType == RenderTreeFrameType.Component) + .ComponentId; + + // Act: Second render + component.TriggerRender(); + + // Assert + Assert.False(renderer.Batches[1].DiffsByComponentId.ContainsKey(childComponentId)); + } + [Fact] public void RenderBatchIncludesListOfDisposedComponents() { @@ -3270,6 +3340,21 @@ namespace Microsoft.AspNetCore.Components.Test } } + private class MyStrongComponent : AutoRenderComponent + { + [Parameter(CaptureUnmatchedValues = true)] internal IDictionary Attributes { get; set; } + + [Parameter] internal string Text { get; set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenElement(0, "strong"); + builder.AddMultipleAttributes(1, Attributes); + builder.AddContent(2, Text); + builder.CloseElement(); + } + } + private class FakeComponent : IComponent { [Parameter] diff --git a/src/Components/Components/test/Rendering/HtmlRendererTestBase.cs b/src/Components/Components/test/Rendering/HtmlRendererTestBase.cs index 8d4ba09a46..fbf5a8f9bf 100644 --- a/src/Components/Components/test/Rendering/HtmlRendererTestBase.cs +++ b/src/Components/Components/test/Rendering/HtmlRendererTestBase.cs @@ -125,6 +125,40 @@ namespace Microsoft.AspNetCore.Components.Rendering Assert.Equal(expectedHtml, result); } + [Fact] + public void RenderComponentAsync_SkipsDuplicatedAttribute() + { + // Arrange + var expectedHtml = new[] + { + "<", "p", " ", + "another", "=", "\"", "another-value", "\"", " ", + "Class", "=", "\"", "test2", "\"", ">", + "Hello world!", + "" + }; + var serviceProvider = new ServiceCollection().AddSingleton(new RenderFragment(rtb => + { + rtb.OpenElement(0, "p"); + rtb.AddAttribute(1, "class", "test1"); + rtb.AddAttribute(2, "another", "another-value"); + rtb.AddMultipleAttributes(3, new Dictionary() + { + { "Class", "test2" }, // Matching is case-insensitive. + }); + rtb.AddContent(4, "Hello world!"); + rtb.CloseElement(); + })).BuildServiceProvider(); + + var htmlRenderer = GetHtmlRenderer(serviceProvider); + + // Act + var result = GetResult(Dispatcher.InvokeAsync(() => htmlRenderer.RenderComponentAsync(ParameterCollection.Empty))); + + // Assert + Assert.Equal(expectedHtml, result); + } + [Fact] public void RenderComponentAsync_HtmlEncodesAttributeValues() { diff --git a/src/Components/test/E2ETest/Tests/ComponentRenderingTest.cs b/src/Components/test/E2ETest/Tests/ComponentRenderingTest.cs index c88d2997c6..59d6c73211 100644 --- a/src/Components/test/E2ETest/Tests/ComponentRenderingTest.cs +++ b/src/Components/test/E2ETest/Tests/ComponentRenderingTest.cs @@ -597,6 +597,26 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests Browser.Equal("Hello from interop call", () => appElement.FindElement(By.Id("val-set-by-interop")).GetAttribute("value")); } + [Fact] + public void CanUseAddMultipleAttributes() + { + var appElement = MountTestComponent(); + + var selector = By.CssSelector("#duplicate-on-element > div"); + WaitUntilExists(selector); + + var element = appElement.FindElement(selector); + Assert.Equal(string.Empty, element.GetAttribute("bool")); // attribute is present + Assert.Equal("middle-value", element.GetAttribute("string")); + Assert.Equal("unmatched-value", element.GetAttribute("unmatched")); + + selector = By.CssSelector("#duplicate-on-element-override > div"); + element = appElement.FindElement(selector); + Assert.Null(element.GetAttribute("bool")); // attribute is not present + Assert.Equal("other-text", element.GetAttribute("string")); + Assert.Equal("unmatched-value", element.GetAttribute("unmatched")); + } + static IAlert SwitchToAlert(IWebDriver driver) { try diff --git a/src/Components/test/E2ETest/Tests/EventCallbackTest.cs b/src/Components/test/E2ETest/Tests/EventCallbackTest.cs index 01d53d6b20..2ab5bebe05 100644 --- a/src/Components/test/E2ETest/Tests/EventCallbackTest.cs +++ b/src/Components/test/E2ETest/Tests/EventCallbackTest.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Threading.Tasks; using BasicTestApp; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; @@ -40,9 +39,9 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests { var target = Browser.FindElement(By.CssSelector($"#{@case} button")); var count = Browser.FindElement(By.Id("render_count")); - Assert.Equal("Render Count: 1", count.Text); + Browser.Equal("Render Count: 1", () => count.Text); target.Click(); - Assert.Equal("Render Count: 2", count.Text); + Browser.Equal("Render Count: 2", () => count.Text); } } } diff --git a/src/Components/test/testassets/BasicTestApp/DuplicateAttributesComponent.razor b/src/Components/test/testassets/BasicTestApp/DuplicateAttributesComponent.razor new file mode 100644 index 0000000000..6993a5e400 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/DuplicateAttributesComponent.razor @@ -0,0 +1,28 @@ +
+ +
+ +
+ +
+ +@functions { + void SomeMethod() + { + } + + Dictionary elementValues = new Dictionary() + { + { "bool", true }, + { "string", "middle-value" }, + { "unmatched", "unmatched-value" }, + }; +} diff --git a/src/Components/test/testassets/BasicTestApp/DuplicateAttributesOnElementChildComponent.cs b/src/Components/test/testassets/BasicTestApp/DuplicateAttributesOnElementChildComponent.cs new file mode 100644 index 0000000000..055ecf7149 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/DuplicateAttributesOnElementChildComponent.cs @@ -0,0 +1,37 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.AspNetCore.Components; +using Microsoft.AspNetCore.Components.RenderTree; + +namespace BasicTestApp +{ + // Written in C# for flexibility and because we don't currently have the ability to write this in .razor. + public class DuplicateAttributesOnElementChildComponent : ComponentBase + { + [Parameter] public string StringAttributeBefore { get; private set; } + [Parameter] public bool BoolAttributeBefore { get; private set; } + [Parameter] public string StringAttributeAfter { get; private set; } + [Parameter] public bool? BoolAttributeAfter { get; private set; } + + [Parameter(CaptureUnmatchedValues = true)] public Dictionary UnmatchedValues { get; private set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenElement(0, "div"); + builder.AddAttribute(1, "string", StringAttributeBefore); + builder.AddAttribute(2, "bool", BoolAttributeBefore); + builder.AddMultipleAttributes(3, UnmatchedValues); + if (StringAttributeAfter != null) + { + builder.AddAttribute(4, "string", StringAttributeAfter); + } + if (BoolAttributeAfter != null) + { + builder.AddAttribute(5, "bool", BoolAttributeAfter); + } + builder.CloseElement(); + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index 7570d92e99..c3f4468cde 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -56,6 +56,7 @@ + @if (SelectedComponentType != null)