Skip to content

Commit

Permalink
Add an analyzer to prevent use of some internal shared source types (#…
Browse files Browse the repository at this point in the history
…6642)

* Initial implementation of AZC0020

* Updates

* Update

* refactor

* whitespace

* updates

* Allow use of shared source types in Azure.Core

* pr fb

* pr fb

* add back change to file missed in merge

* Update tests

* pr fb; + WIP for local variables

* clean up

* missed cleanup

* missed file

* Address warnings

* Updates

* refactor
  • Loading branch information
annelo-msft authored Aug 11, 2023
1 parent 49201ef commit 76c21ff
Show file tree
Hide file tree
Showing 4 changed files with 270 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Threading.Tasks;
using Xunit;
using Verifier = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<Azure.ClientSdk.Analyzers.BannedTypesAnalyzer>;

namespace Azure.ClientSdk.Analyzers.Tests
{
public class AZC0020Tests
{
private List<(string fileName, string source)> _sharedSourceFiles;

public AZC0020Tests()
{
_sharedSourceFiles = new List<(string fileName, string source)>() {

("MutableJsonDocument.cs", @"
namespace Azure.Core.Json
{
internal sealed partial class MutableJsonDocument
{
}
}
"),

("MutableJsonElement.cs", @"
namespace Azure.Core.Json
{
internal partial struct MutableJsonElement
{
}
}
")
};
}

[Fact]
public async Task AZC0020ProducedForMutableJsonDocumentUsage()
{
string code = @"
using System;
using Azure.Core.Json;
namespace LibraryNamespace
{
public class Model
{
private MutableJsonDocument {|AZC0020:_document|};
internal MutableJsonDocument {|AZC0020:Document|} => {|AZC0020:_document|};
internal event EventHandler<MutableJsonDocument> {|AZC0020:_docEvent|};
internal MutableJsonDocument {|AZC0020:GetDocument|}(MutableJsonDocument {|AZC0020:value|})
{
{|AZC0020:MutableJsonDocument mdoc = new MutableJsonDocument();|}
return mdoc;
}
}
}";
await Verifier.VerifyAnalyzerAsync(code, _sharedSourceFiles);
}

[Fact]
public async Task AZC0020ProducedForMutableJsonElementUsage()
{
string code = @"
using Azure.Core.Json;
namespace LibraryNamespace
{
public class Model
{
private MutableJsonElement {|AZC0020:_element|};
internal MutableJsonElement {|AZC0020:Element|} => {|AZC0020:_element|};
internal MutableJsonElement {|AZC0020:GetDocument|}(MutableJsonElement {|AZC0020:value|})
{
{|AZC0020:MutableJsonElement element = new MutableJsonElement();|}
return element;
}
}
}";
await Verifier.VerifyAnalyzerAsync(code, _sharedSourceFiles);
}

[Fact]
public async Task AZC0020NotProducedForAllowedTypeUsage()
{
string code = @"
using System.Text.Json;
namespace LibraryNamespace
{
public class Model
{
JsonElement _element;
}
}";
await Verifier.VerifyAnalyzerAsync(code, _sharedSourceFiles);
}

[Fact]
public async Task AZC0020NotProducedForTypeWithBannedNameInAllowedNamespace()
{
string code = @"
namespace LibraryNamespace
{
public class MutableJsonDocument
{
}
public class Model
{
MutableJsonDocument _document;
}
}";
await Verifier.VerifyAnalyzerAsync(code, _sharedSourceFiles);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -11,16 +12,16 @@
using Microsoft.CodeAnalysis.Testing;
using Microsoft.CodeAnalysis.Testing.Verifiers;

namespace Azure.ClientSdk.Analyzers.Tests
namespace Azure.ClientSdk.Analyzers.Tests
{
public static class AzureAnalyzerVerifier<TAnalyzer> where TAnalyzer : DiagnosticAnalyzer, new()
{
private static readonly ReferenceAssemblies DefaultReferenceAssemblies =
ReferenceAssemblies.Default.AddPackages(ImmutableArray.Create(
new PackageIdentity("Azure.Core", "1.26.0"),
new PackageIdentity("Microsoft.Bcl.AsyncInterfaces", "1.1.0"),
new PackageIdentity("System.Text.Json", "4.6.0"),
new PackageIdentity("Newtonsoft.Json", "12.0.3"),
new PackageIdentity("System.Text.Json", "4.6.0"),
new PackageIdentity("System.Threading.Tasks.Extensions", "4.5.3")));

public static CSharpAnalyzerTest<TAnalyzer, XUnitVerifier> CreateAnalyzer(string source, LanguageVersion languageVersion = LanguageVersion.Latest)
Expand All @@ -37,7 +38,7 @@ public static CSharpAnalyzerTest<TAnalyzer, XUnitVerifier> CreateAnalyzer(string
TestBehaviors = TestBehaviors.SkipGeneratedCodeCheck
};

public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion = LanguageVersion.Latest)
public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion = LanguageVersion.Latest)
=> CreateAnalyzer(source, languageVersion).RunAsync(CancellationToken.None);

public static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] diagnostics)
Expand All @@ -47,6 +48,16 @@ public static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[]
return test.RunAsync(CancellationToken.None);
}

public static Task VerifyAnalyzerAsync(string source, List<(string fileName, string source)> files)
{
var test = CreateAnalyzer(source);
foreach (var file in files)
{
test.TestState.Sources.Add(file);
}
return test.RunAsync(CancellationToken.None);
}

public static DiagnosticResult Diagnostic(string expectedDescriptor) => AnalyzerVerifier<TAnalyzer>.Diagnostic(expectedDescriptor);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Azure.ClientSdk.Analyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class BannedTypesAnalyzer : DiagnosticAnalyzer
{
private static HashSet<string> BannedTypes = new HashSet<string>()
{
"Azure.Core.Json.MutableJsonDocument",
"Azure.Core.Json.MutableJsonElement",
"Azure.Core.Json.MutableJsonChange",
"Azure.Core.Json.MutableJsonChangeKind",
};

private static readonly string BannedTypesMessageArgs = string.Join(", ", BannedTypes);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptors.AZC0020);

public SymbolKind[] SymbolKinds { get; } = new[]
{
SymbolKind.Event,
SymbolKind.Field,
SymbolKind.Method,
SymbolKind.NamedType,
SymbolKind.Parameter,
SymbolKind.Property,
};

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();
context.RegisterSymbolAction(c => Analyze(c), SymbolKinds);
context.RegisterSyntaxNodeAction(c => AnalyzeNode(c), SyntaxKind.LocalDeclarationStatement);
}

public void Analyze(SymbolAnalysisContext context)
{
if (IsAzureCore(context.Symbol.ContainingAssembly))
{
return;
}

switch (context.Symbol)
{
case IParameterSymbol parameterSymbol:
CheckType(parameterSymbol.Type, parameterSymbol, context.ReportDiagnostic);
break;
case IMethodSymbol methodSymbol:
CheckType(methodSymbol.ReturnType, methodSymbol, context.ReportDiagnostic);
break;
case IEventSymbol eventSymbol:
CheckType(eventSymbol.Type, eventSymbol, context.ReportDiagnostic);
break;
case IPropertySymbol propertySymbol:
CheckType(propertySymbol.Type, propertySymbol, context.ReportDiagnostic);
break;
case IFieldSymbol fieldSymbol:
CheckType(fieldSymbol.Type, fieldSymbol, context.ReportDiagnostic);
break;
case INamedTypeSymbol namedTypeSymbol:
CheckType(namedTypeSymbol.BaseType, namedTypeSymbol, context.ReportDiagnostic);
foreach (var iface in namedTypeSymbol.Interfaces)
{
CheckType(iface, namedTypeSymbol, context.ReportDiagnostic);
}
break;
}
}

public void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
if (IsAzureCore(context.ContainingSymbol.ContainingAssembly))
{
return;
}

if (context.Node is LocalDeclarationStatementSyntax declaration)
{
ITypeSymbol type = context.SemanticModel.GetTypeInfo(declaration.Declaration.Type).Type;

CheckType(type, type, context.ReportDiagnostic, context.Node.GetLocation());
}
}

private static Diagnostic CheckType(ITypeSymbol type, ISymbol symbol, Action<Diagnostic> reportDiagnostic, Location location = default)
{
if (type is INamedTypeSymbol namedTypeSymbol)
{
if (IsBannedType(namedTypeSymbol))
{
reportDiagnostic(Diagnostic.Create(Descriptors.AZC0020, location ?? symbol.Locations.First(), BannedTypesMessageArgs));
}

if (namedTypeSymbol.IsGenericType)
{
foreach (var typeArgument in namedTypeSymbol.TypeArguments)
{
CheckType(typeArgument, symbol, reportDiagnostic);
}
}
}

return null;
}

private static bool IsAzureCore(IAssemblySymbol assembly)
{
return
assembly.Name.Equals("Azure.Core") ||
assembly.Name.Equals("Azure.Core.Experimental");
}

private static bool IsBannedType(INamedTypeSymbol namedTypeSymbol)
{
return BannedTypes.Contains($"{namedTypeSymbol.ContainingNamespace}.{namedTypeSymbol.Name}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,16 @@ internal class Descriptors
"Invalid ServiceVersion member name.",
"All parts of ServiceVersion members' names must begin with a number or uppercase letter and cannot have consecutive underscores.",
"Usage",
DiagnosticSeverity.Warning, true);

public static DiagnosticDescriptor AZC0020 = new DiagnosticDescriptor(
nameof(AZC0020),
"Avoid using banned types in public APIs",
"The Azure.Core internal shared source types {0} should not be used outside of the Azure.Core library.",
"Usage",
DiagnosticSeverity.Warning, true);
#endregion


#region General
public static DiagnosticDescriptor AZC0100 = new DiagnosticDescriptor(
nameof(AZC0100),
Expand Down

0 comments on commit 76c21ff

Please sign in to comment.