Require public component parameters and remove private metadata workaround

- Prior to this a user could have private, protected or public parameters. We now require public and plan to update analyzers to enforce good usage of public component parameters.
- Removed private `ComponentTagHelperDescriptorProvider` workaround. meta data workaround since we no longer operate on private parameters.
- Updated existing tests to not have private parameter setters. I missed these in my last pass.
- Added two new tests to validate parameters which are private or have private setters are ignored.

aspnet/AspNetCoredotnet/aspnetcore-tooling#12291
\n\nCommit migrated from bb543aa166
This commit is contained in:
N. Taylor Mullen 2019-06-26 17:10:36 -07:00
parent 36420ad55f
commit c1d2914398
3 changed files with 106 additions and 30 deletions

View File

@ -4574,7 +4574,7 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public string Message { get; set; }
}
}
"));
@ -4603,7 +4603,7 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public string Message { get; set; }
}
}
"));
@ -4634,7 +4634,7 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public string Message { get; set; }
}
}
"));
@ -4664,9 +4664,9 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public EventCallback<string> MessageChanged { get; private set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; private set; }
[Parameter] public string Message { get; set; }
[Parameter] public EventCallback<string> MessageChanged { get; set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; set; }
}
}
"));
@ -4699,9 +4699,9 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public EventCallback<string> MessageChanged { get; private set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; private set; }
[Parameter] public string Message { get; set; }
[Parameter] public EventCallback<string> MessageChanged { get; set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; set; }
}
}
"));
@ -4734,9 +4734,9 @@ namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter] public string Message { get; private set; }
[Parameter] public EventCallback<string> MessageChanged { get; private set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; private set; }
[Parameter] public string Message { get; set; }
[Parameter] public EventCallback<string> MessageChanged { get; set; }
[Parameter] public Expression<Action<string>> MessageExpression { get; set; }
}
}
"));

View File

@ -4,10 +4,8 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Components;
using Microsoft.CodeAnalysis.CSharp;
namespace Microsoft.CodeAnalysis.Razor
{
@ -18,10 +16,6 @@ namespace Microsoft.CodeAnalysis.Razor
.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)
.WithMiscellaneousOptions(SymbolDisplayFormat.FullyQualifiedFormat.MiscellaneousOptions & (~SymbolDisplayMiscellaneousOptions.UseSpecialTypes));
private static MethodInfo WithMetadataImportOptionsMethodInfo =
typeof(CSharpCompilationOptions)
.GetMethod("WithMetadataImportOptions", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
public bool IncludeDocumentation { get; set; }
public int Order { get; set; }
@ -40,9 +34,6 @@ namespace Microsoft.CodeAnalysis.Razor
return;
}
// We need to see private members too
compilation = WithMetadataImportOptionsAll(compilation);
var symbols = ComponentSymbols.Create(compilation);
var types = new List<INamedTypeSymbol>();
@ -81,13 +72,6 @@ namespace Microsoft.CodeAnalysis.Razor
}
}
private Compilation WithMetadataImportOptionsAll(Compilation compilation)
{
var newCompilationOptions = (CSharpCompilationOptions)WithMetadataImportOptionsMethodInfo
.Invoke(compilation.Options, new object[] { /* All */ (byte)2 });
return compilation.WithOptions(newCompilationOptions);
}
private TagHelperDescriptor CreateShortNameMatchingDescriptor(ComponentSymbols symbols, INamedTypeSymbol type)
{
var builder = CreateDescriptorBuilder(symbols, type);
@ -216,9 +200,9 @@ namespace Microsoft.CodeAnalysis.Razor
}
// We need to check for cases like:
// [Parameter] List<T> MyProperty { get; set; }
// [Parameter] public List<T> MyProperty { get; set; }
// AND
// [Parameter] List<string> MyProperty { get; set; }
// [Parameter] public List<string> MyProperty { get; set; }
//
// We need to inspect the type arguments to tell the difference between a property that
// uses the containing class' type parameter(s) and a vanilla usage of generic types like
@ -333,6 +317,7 @@ namespace Microsoft.CodeAnalysis.Razor
// a dictionary keyed on property name.
//
// We consider parameters to be defined by properties satisfying all of the following:
// - are public
// - are visible (not shadowed)
// - have the [Parameter] attribute
// - have a setter, even if private
@ -367,6 +352,12 @@ namespace Microsoft.CodeAnalysis.Razor
}
var kind = PropertyKind.Default;
if (property.DeclaredAccessibility != Accessibility.Public)
{
// Not public
kind = PropertyKind.Ignored;
}
if (property.Parameters.Length != 0)
{
// Indexer
@ -378,6 +369,11 @@ namespace Microsoft.CodeAnalysis.Razor
// No setter
kind = PropertyKind.Ignored;
}
else if (property.SetMethod.DeclaredAccessibility != Accessibility.Public)
{
// No public setter
kind = PropertyKind.Ignored;
}
if (property.IsStatic)
{

View File

@ -246,6 +246,86 @@ namespace Test
Assert.Equal("System.String", attribute.TypeName);
}
[Fact]
public void Execute_IgnoresPrivateParameters_CreatesDescriptor()
{
// Arrange
var compilation = BaseCompilation.AddSyntaxTrees(Parse(@"
using Microsoft.AspNetCore.Components;
namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter]
private string MyProperty { get; set; }
}
}
"));
Assert.Empty(compilation.GetDiagnostics());
var context = TagHelperDescriptorProviderContext.Create();
context.SetCompilation(compilation);
var provider = new ComponentTagHelperDescriptorProvider();
// Act
provider.Execute(context);
// Assert
var components = ExcludeBuiltInComponents(context);
components = AssertAndExcludeFullyQualifiedNameMatchComponents(components, expectedCount: 1);
var component = Assert.Single(components);
Assert.Equal("TestAssembly", component.AssemblyName);
Assert.Equal("Test.MyComponent", component.Name);
Assert.Empty(component.BoundAttributes);
}
[Fact]
public void Execute_IgnoresParametersWithPrivateSetters_CreatesDescriptor()
{
// Arrange
var compilation = BaseCompilation.AddSyntaxTrees(Parse(@"
using Microsoft.AspNetCore.Components;
namespace Test
{
public class MyComponent : ComponentBase
{
[Parameter]
public string MyProperty { get; private set; }
}
}
"));
Assert.Empty(compilation.GetDiagnostics());
var context = TagHelperDescriptorProviderContext.Create();
context.SetCompilation(compilation);
var provider = new ComponentTagHelperDescriptorProvider();
// Act
provider.Execute(context);
// Assert
var components = ExcludeBuiltInComponents(context);
components = AssertAndExcludeFullyQualifiedNameMatchComponents(components, expectedCount: 1);
var component = Assert.Single(components);
Assert.Equal("TestAssembly", component.AssemblyName);
Assert.Equal("Test.MyComponent", component.Name);
Assert.Empty(component.BoundAttributes);
}
[Fact] // bool properties support minimized attributes
public void Execute_BoolProperty_CreatesDescriptor()
{