Skip to content

Commit

Permalink
Add analyzer to warn for invalid Component parameter usage.
Browse files Browse the repository at this point in the history
- The newly added analyzer warns users if they try to assign another components parameter.It does sanity checks to ensure that
  1. The property reference is indeed a component parameter
  2. The property reference is from a component
  3. The assignment is outside of the parameters type hierarchy. Aka, we don't warn users for setting a components parameter if it's in the same class.

- Updated existing `ComponentsFacts` to add additional utility methods to properly interact with components.
- Added tests to ensure we're analyzing all the goods properly.

#8825
  • Loading branch information
NTaylorMullen committed Jul 17, 2019
1 parent 4f638f8 commit d053be3
Show file tree
Hide file tree
Showing 9 changed files with 496 additions and 3 deletions.
22 changes: 22 additions & 0 deletions src/Components/Analyzers/src/ComponentFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;

namespace Microsoft.AspNetCore.Components.Analyzers
{
Expand Down Expand Up @@ -85,5 +86,26 @@ public static bool IsCascadingParameter(ComponentSymbols symbols, IPropertySymbo

return property.GetAttributes().Any(a => a.AttributeClass == symbols.CascadingParameterAttribute);
}

public static bool IsComponent(ComponentSymbols symbols, Compilation compilation, INamedTypeSymbol type)
{
if (symbols is null)
{
throw new ArgumentNullException(nameof(symbols));
}

if (type is null)
{
throw new ArgumentNullException(nameof(type));
}

var conversion = compilation.ClassifyConversion(symbols.IComponentType, type);
if (!conversion.Exists || !conversion.IsExplicit)
{
return false;
}

return true;
}
}
}
104 changes: 104 additions & 0 deletions src/Components/Analyzers/src/ComponentParameterUsageAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// 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.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.AspNetCore.Components.Analyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ComponentParameterUsageAnalyzer : DiagnosticAnalyzer
{
public ComponentParameterUsageAnalyzer()
{
SupportedDiagnostics = ImmutableArray.Create(new[]
{
DiagnosticDescriptors.ComponentParametersShouldNotBeSetOutsideOfTheirDeclaredComponent,
});
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.RegisterCompilationStartAction(context =>
{
if (!ComponentSymbols.TryCreate(context.Compilation, out var symbols))
{
// Types we need are not defined.
return;
}
context.RegisterOperationBlockStartAction(startBlockContext =>
{
startBlockContext.RegisterOperationAction(context =>
{
var assignmentOperation = (IAssignmentOperation)context.Operation;
var leftHandSide = assignmentOperation.Target;
if (leftHandSide == null)
{
// Malformed assignment, no left hand side.
return;
}
if (leftHandSide.Kind != OperationKind.PropertyReference)
{
// We don't want to capture situations where a user does
// MyOtherProperty = aComponent.SomeParameter
return;
}
var propertyReference = (IPropertyReferenceOperation)leftHandSide;
var componentProperty = (IPropertySymbol)propertyReference.Member;
if (!ComponentFacts.IsParameter(symbols, componentProperty))
{
// This is not a property reference that we care about, it is not decorated with [Parameter].
return;
}
var propertyContainingType = componentProperty.ContainingType;
if (!ComponentFacts.IsComponent(symbols, context.Compilation, propertyContainingType))
{
// Someone referenced a property as [Parameter] inside something that is not a component.
return;
}
var assignmentContainingType = startBlockContext.OwningSymbol?.ContainingType;
if (assignmentContainingType == null)
{
// Assignment location has no containing type. Most likely we're operating on malformed code, don't try and validate.
return;
}
var conversion = context.Compilation.ClassifyConversion(propertyContainingType, assignmentContainingType);
if (conversion.Exists && conversion.IsIdentity)
{
// The assignment is taking place inside of the declaring component.
return;
}
if (conversion.Exists && conversion.IsExplicit)
{
// The assignment is taking place within the components type hierarchy. This means the user is setting this in a supported
// scenario.
return;
}
// At this point the user is referencing a component parameter outside of its declaring class.
context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.ComponentParametersShouldNotBeSetOutsideOfTheirDeclaredComponent,
propertyReference.Syntax.GetLocation(),
propertyReference.Member.Name));
}, OperationKind.SimpleAssignment, OperationKind.CompoundAssignment, OperationKind.CoalesceAssignment);
});
});
}
}
}
20 changes: 17 additions & 3 deletions src/Components/Analyzers/src/ComponentSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// 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
Expand Down Expand Up @@ -30,6 +29,13 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo
return false;
}

var icomponentType = compilation.GetTypeByMetadataName(ComponentsApi.IComponent.MetadataName);
if (icomponentType == 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);
Expand All @@ -41,18 +47,24 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo

var parameterCaptureUnmatchedValuesRuntimeType = dictionary.Construct(@string, @object);

symbols = new ComponentSymbols(parameterAttribute, cascadingParameterAttribute, parameterCaptureUnmatchedValuesRuntimeType);
symbols = new ComponentSymbols(
parameterAttribute,
cascadingParameterAttribute,
parameterCaptureUnmatchedValuesRuntimeType,
icomponentType);
return true;
}

private ComponentSymbols(
INamedTypeSymbol parameterAttribute,
INamedTypeSymbol cascadingParameterAttribute,
INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType)
INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType,
INamedTypeSymbol icomponentType)
{
ParameterAttribute = parameterAttribute;
CascadingParameterAttribute = cascadingParameterAttribute;
ParameterCaptureUnmatchedValuesRuntimeType = parameterCaptureUnmatchedValuesRuntimeType;
IComponentType = icomponentType;
}

public INamedTypeSymbol ParameterAttribute { get; }
Expand All @@ -61,5 +73,7 @@ private ComponentSymbols(
public INamedTypeSymbol ParameterCaptureUnmatchedValuesRuntimeType { get; }

public INamedTypeSymbol CascadingParameterAttribute { get; }

public INamedTypeSymbol IComponentType { get; }
}
}
6 changes: 6 additions & 0 deletions src/Components/Analyzers/src/ComponentsApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@ public static class CascadingParameterAttribute
public static readonly string FullTypeName = "Microsoft.AspNetCore.Components.CascadingParameterAttribute";
public static readonly string MetadataName = FullTypeName;
}

public static class IComponent
{
public static readonly string FullTypeName = "Microsoft.AspNetCore.Components.IComponent";
public static readonly string MetadataName = FullTypeName;
}
}
}
9 changes: 9 additions & 0 deletions src/Components/Analyzers/src/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,14 @@ internal static class DiagnosticDescriptors
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: new LocalizableResourceString(nameof(Resources.ComponentParametersShouldBePublic_Description), Resources.ResourceManager, typeof(Resources)));

public static readonly DiagnosticDescriptor ComponentParametersShouldNotBeSetOutsideOfTheirDeclaredComponent = new DiagnosticDescriptor(
"BL0005",
new LocalizableResourceString(nameof(Resources.ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Title), Resources.ResourceManager, typeof(Resources)),
new LocalizableResourceString(nameof(Resources.ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Format), Resources.ResourceManager, typeof(Resources)),
"Usage",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: new LocalizableResourceString(nameof(Resources.ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Description), Resources.ResourceManager, typeof(Resources)));
}
}
27 changes: 27 additions & 0 deletions src/Components/Analyzers/src/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/Components/Analyzers/src/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,13 @@
<data name="ComponentParametersShouldBePublic_FixTitle" xml:space="preserve">
<value>Make component parameters public.</value>
</data>
<data name="ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Description" xml:space="preserve">
<value>Component parameters should not be set outside of their declared component. Doing so may result in unexpected behavior at runtime.</value>
</data>
<data name="ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Format" xml:space="preserve">
<value>Component parameter '{0}' should not be set outside of its component.</value>
</data>
<data name="ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Title" xml:space="preserve">
<value>Component parameter should not be set outside of its component.</value>
</data>
</root>
Loading

0 comments on commit d053be3

Please sign in to comment.