Blazor API Review: RenderTree types (#12869)

* Add analzyer for pubternal

This is based on some existing code from EF. I'm having a discussion
with them right now on the best way to share this logic.

I also added support for parameters here which was missing. We might
want to make this code converge with `BannedApiAnalyzer` which is much
more thorough than this.

This is using our existing package for testing analyzers thats the
*official* way to do it in our repo. Filed #12868 to track that.

* Add S C A R Y warnings to render tree types

* PR feedback
This commit is contained in:
Ryan Nowak 2019-08-06 22:41:02 -07:00 committed by GitHub
parent 7e59a26846
commit 15e4b605eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 437 additions and 42 deletions

View File

@ -0,0 +1,39 @@
// 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 Microsoft.AspNetCore.Components.Analyzers;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
namespace Microsoft.Extensions.Internal
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ComponentInternalUsageDiagnosticAnalyzer : DiagnosticAnalyzer
{
private readonly InternalUsageAnalyzer _inner;
public ComponentInternalUsageDiagnosticAnalyzer()
{
// We don't have in *internal* attribute in Blazor.
_inner = new InternalUsageAnalyzer(IsInInternalNamespace, hasInternalAttribute: null, DiagnosticDescriptors.DoNotUseRenderTreeTypes);
}
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseRenderTreeTypes);
public override void Initialize(AnalysisContext context)
{
_inner.Register(context);
}
private static bool IsInInternalNamespace(ISymbol symbol)
{
if (symbol?.ContainingNamespace?.ToDisplayString() is string ns)
{
return string.Equals(ns, "Microsoft.AspNetCore.Components.RenderTree");
}
return false;
}
}
}

View File

@ -55,5 +55,14 @@ namespace Microsoft.AspNetCore.Components.Analyzers
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: new LocalizableResourceString(nameof(Resources.ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Description), Resources.ResourceManager, typeof(Resources)));
public static readonly DiagnosticDescriptor DoNotUseRenderTreeTypes = new DiagnosticDescriptor(
"BL0006",
new LocalizableResourceString(nameof(Resources.DoNotUseRenderTreeTypes_Title), Resources.ResourceManager, typeof(Resources)),
new LocalizableResourceString(nameof(Resources.DoNotUseRenderTreeTypes_Description), Resources.ResourceManager, typeof(Resources)),
"Usage",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: new LocalizableResourceString(nameof(Resources.DoNotUseRenderTreeTypes_Description), Resources.ResourceManager, typeof(Resources)));
}
}

View File

@ -0,0 +1,126 @@
// 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 Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
namespace Microsoft.Extensions.Internal
{
internal class InternalUsageAnalyzer
{
private readonly Func<ISymbol, bool> _isInternalNamespace;
private readonly Func<ISymbol, bool> _hasInternalAttribute;
private readonly DiagnosticDescriptor _descriptor;
/// <summary>
/// Creates a new instance of <see cref="InternalUsageAnalyzer" />. The creator should provide delegates to help determine whether
/// a given symbol is internal or not, and a <see cref="DiagnosticDescriptor" /> to create errors.
/// </summary>
/// <param name="isInInternalNamespace">The delegate used to check if a symbol belongs to an internal namespace.</param>
/// <param name="hasInternalAttribute">The delegate used to check if a symbol has an internal attribute.</param>
/// <param name="descriptor">
/// The <see cref="DiagnosticDescriptor" /> used to create errors. The error message should expect a single parameter
/// used for the display name of the member.
/// </param>
public InternalUsageAnalyzer(Func<ISymbol, bool> isInInternalNamespace, Func<ISymbol, bool> hasInternalAttribute, DiagnosticDescriptor descriptor)
{
_isInternalNamespace = isInInternalNamespace ?? new Func<ISymbol, bool>((_) => false);
_hasInternalAttribute = hasInternalAttribute ?? new Func<ISymbol, bool>((_) => false);
_descriptor = descriptor ?? throw new ArgumentNullException(nameof(descriptor));
}
public void Register(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeNode,
SyntaxKind.SimpleMemberAccessExpression,
SyntaxKind.ObjectCreationExpression,
SyntaxKind.ClassDeclaration,
SyntaxKind.Parameter);
}
private void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
switch (context.Node)
{
case MemberAccessExpressionSyntax memberAccessSyntax:
{
if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly)
{
var containingType = symbol.ContainingType;
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), $"{containingType}.{symbol.Name}"));
return;
}
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), containingType));
return;
}
}
return;
}
case ObjectCreationExpressionSyntax creationSyntax:
{
if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly)
{
var containingType = symbol.ContainingType;
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.GetLocation(), containingType));
return;
}
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.Type.GetLocation(), containingType));
return;
}
}
return;
}
case ClassDeclarationSyntax declarationSyntax:
{
if (context.SemanticModel.GetDeclaredSymbol(declarationSyntax)?.BaseType is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly &&
(IsInInternalNamespace(symbol) || HasInternalAttribute(symbol)) &&
declarationSyntax.BaseList?.Types.Count > 0)
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, declarationSyntax.BaseList.Types[0].GetLocation(), symbol));
}
return;
}
case ParameterSyntax parameterSyntax:
{
if (context.SemanticModel.GetDeclaredSymbol(parameterSyntax)?.Type is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly &&
(IsInInternalNamespace(symbol) || HasInternalAttribute(symbol)))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, parameterSyntax.GetLocation(), symbol));
}
return;
}
}
}
private bool HasInternalAttribute(ISymbol symbol) => _hasInternalAttribute(symbol);
private bool IsInInternalNamespace(ISymbol symbol) => _isInternalNamespace(symbol);
}
}

View File

@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
@ -165,4 +165,13 @@
<data name="ComponentParameterShouldNotBeSetOutsideOfTheirDeclaredComponent_Title" xml:space="preserve">
<value>Component parameter should not be set outside of its component.</value>
</data>
<data name="DoNotUseRenderTreeTypes_Description" xml:space="preserve">
<value>The types in 'Microsoft.AspNetCore.Components.RenderTree' are not recommended for use outside of the Blazor framework. These type definitions will change in future releases.</value>
</data>
<data name="DoNotUseRenderTreeTypes_Format" xml:space="preserve">
<value>The type or member {0} is is not recommended for use outside of the Blazor frameworks. Types defined in 'Microsoft.AspNetCore.Components.RenderTree' will change in future releases.</value>
</data>
<data name="DoNotUseRenderTreeTypes_Title" xml:space="preserve">
<value>Do not use RenderTree types</value>
</data>
</root>

View File

@ -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 System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Analyzer.Testing;
using Microsoft.AspNetCore.Testing;
using Microsoft.AspNetCore.Testing.xunit;
using Microsoft.CodeAnalysis;
namespace Microsoft.AspNetCore.Components.Analyzers
{
public abstract class AnalyzerTestBase
{
private static readonly string ProjectDirectory = GetProjectDirectory();
public TestSource Read(string source)
{
if (!source.EndsWith(".cs"))
{
source = source + ".cs";
}
var filePath = Path.Combine(ProjectDirectory, "TestFiles", GetType().Name, source);
if (!File.Exists(filePath))
{
throw new FileNotFoundException($"TestFile {source} could not be found at {filePath}.", filePath);
}
var fileContent = File.ReadAllText(filePath);
return TestSource.Read(fileContent);
}
public Project CreateProject(string source)
{
if (!source.EndsWith(".cs"))
{
source = source + ".cs";
}
var read = Read(source);
return DiagnosticProject.Create(GetType().Assembly, new[] { read.Source, });
}
public Task<Compilation> CreateCompilationAsync(string source)
{
return CreateProject(source).GetCompilationAsync();
}
private static string GetProjectDirectory()
{
// On helix we use the published test files
if (SkipOnHelixAttribute.OnHelix())
{
return AppContext.BaseDirectory;
}
// This test code needs to be updated to support distributed testing.
// See https://github.com/aspnet/AspNetCore/issues/10422
#pragma warning disable 0618
var solutionDirectory = TestPathUtilities.GetSolutionRootDirectory("Components");
#pragma warning restore 0618
var projectDirectory = Path.Combine(solutionDirectory, "Analyzers", "test");
return projectDirectory;
}
}
}

View File

@ -0,0 +1,31 @@
// 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.Threading.Tasks;
using Microsoft.AspNetCore.Analyzer.Testing;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
namespace Microsoft.AspNetCore.Components.Analyzers
{
internal class ComponentAnalyzerDiagnosticAnalyzerRunner : DiagnosticAnalyzerRunner
{
public ComponentAnalyzerDiagnosticAnalyzerRunner(DiagnosticAnalyzer analyzer)
{
Analyzer = analyzer;
}
public DiagnosticAnalyzer Analyzer { get; }
public Task<Diagnostic[]> GetDiagnosticsAsync(string source)
{
return GetDiagnosticsAsync(sources: new[] { source }, Analyzer, Array.Empty<string>());
}
public Task<Diagnostic[]> GetDiagnosticsAsync(Project project)
{
return GetDiagnosticsAsync(new[] { project }, Analyzer, Array.Empty<string>());
}
}
}

View File

@ -0,0 +1,60 @@
// 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 Microsoft.AspNetCore.Analyzer.Testing;
using Microsoft.Extensions.Internal;
using Xunit;
namespace Microsoft.AspNetCore.Components.Analyzers
{
public class ComponentInternalUsageDiagnoticsAnalyzerTest : AnalyzerTestBase
{
public ComponentInternalUsageDiagnoticsAnalyzerTest()
{
Analyzer = new ComponentInternalUsageDiagnosticAnalyzer();
Runner = new ComponentAnalyzerDiagnosticAnalyzerRunner(Analyzer);
}
private ComponentInternalUsageDiagnosticAnalyzer Analyzer { get; }
private ComponentAnalyzerDiagnosticAnalyzerRunner Runner { get; }
[Fact]
public async Task InternalUsage_FindsUseOfRenderTreeFrameAsParameter()
{
// Arrange
var source = Read("UsesRenderTreeFrameAsParameter");
// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
// Assert
Assert.Collection(
diagnostics,
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
});
}
[Fact]
public async Task InternalUsage_FindsUseOfRenderTreeType()
{
// Arrange
var source = Read("UsesRenderTreeFrameTypeAsLocal");
// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
// Assert
Assert.Collection(
diagnostics,
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
});
}
}
}

View File

@ -2,16 +2,24 @@
<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<Reference Include="Microsoft.AspNetCore.Components" />
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
</ItemGroup>
<!-- Tests do not work on Helix or when bin/ directory is not in project directory due to undeclared dependency on test content. -->
<!-- https://github.com/aspnet/AspNetCore/issues/10422 -->
<BuildHelixPayload>false</BuildHelixPayload>
<BaseOutputPath />
</PropertyGroup>
<ItemGroup>
<!-- This is set to a ProjectReference because analyzers cannot be referenced via Reference. -->
<ProjectReference Include="..\src\Microsoft.AspNetCore.Components.Analyzers.csproj" />
<Reference Include="Microsoft.AspNetCore.Components" />
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
<Reference Include="Microsoft.AspNetCore.Analyzer.Testing" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(SharedSourceRoot)test\SkipOnHelixAttribute.cs" />
<Content Include="TestFiles\**\*.*" CopyToPublishDirectory="PreserveNewest" />
</ItemGroup>
</Project>

View File

@ -0,0 +1,11 @@
using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnoticsAnalyzerTest
{
class UsesRenderTreeFrameAsParameter
{
private void Test(/*MM*/RenderTreeFrame frame)
{
}
}
}

View File

@ -0,0 +1,15 @@
using System;
using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnoticsAnalyzerTest
{
class UsesRenderTreeFrameTypeAsLocal
{
private void Test()
{
var test = RenderTreeFrameType./*MM*/Attribute;
GC.KeepAlive(test);
}
}
}

View File

@ -8,9 +8,12 @@ using System.Collections.Generic;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Represents a range of elements within an instance of <see cref="ArrayBuilder{T}"/>.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
/// <typeparam name="T">The type of the elements in the array</typeparam>
//
// Represents a range of elements within an instance of <see cref="ArrayBuilder{T}"/>.
public readonly struct ArrayBuilderSegment<T> : IEnumerable<T>
{
// The following fields are memory mapped to the WASM client. Do not re-order or use auto-properties.

View File

@ -4,9 +4,12 @@
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Represents a range of elements in an array that are in use.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
/// <typeparam name="T">The array item type.</typeparam>
/// <typeparam name="T"></typeparam>
//
// Represents a range of elements in an array that are in use.
public readonly struct ArrayRange<T>
{
/// <summary>

View File

@ -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 System;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Describes changes to a component's render tree between successive renders.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
//
// Describes changes to a component's render tree between successive renders.
public readonly struct RenderTreeDiff
{
/// <summary>

View File

@ -6,8 +6,11 @@ using System.Runtime.InteropServices;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Represents a single edit operation on a component's render tree.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
//
// Represents a single edit operation on a component's render tree.
[StructLayout(LayoutKind.Explicit)]
public readonly struct RenderTreeEdit
{

View File

@ -4,8 +4,11 @@
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Describes the type of a render tree edit operation.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
//
//Describes the type of a render tree edit operation.
public enum RenderTreeEditType: int
{
/// <summary>

View File

@ -8,8 +8,11 @@ using Microsoft.AspNetCore.Components.Rendering;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Represents an entry in a tree of user interface (UI) items.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
//
// Represents an entry in a tree of user interface (UI) items.
[StructLayout(LayoutKind.Explicit, Pack = 4)]
public readonly struct RenderTreeFrame
{

View File

@ -4,8 +4,11 @@
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Describes the type of a <see cref="RenderTreeFrame"/>.
/// Types in the Microsoft.AspNetCore.Components.RenderTree are not recommended for use outside
/// of the Blazor framework. These types will change in future release.
/// </summary>
//
// Describes the type of a <see cref="RenderTreeFrame"/>.
public enum RenderTreeFrameType: short
{
/// <summary>