Skip to content

Commit

Permalink
Update SA1121 and SA1404 to detect global using aliases
Browse files Browse the repository at this point in the history
  • Loading branch information
bjornhellander committed Jul 13, 2023
1 parent b209e4e commit 0d855cd
Show file tree
Hide file tree
Showing 8 changed files with 333 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,54 @@

namespace StyleCop.Analyzers.Test.CSharp10.MaintainabilityRules
{
using System.Threading;
using System.Threading.Tasks;
using StyleCop.Analyzers.MaintainabilityRules;
using StyleCop.Analyzers.Test.CSharp9.MaintainabilityRules;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.MaintainabilityRules.SA1404CodeAnalysisSuppressionMustHaveJustification,
StyleCop.Analyzers.MaintainabilityRules.SA1404CodeFixProvider>;

public class SA1404CSharp10UnitTests : SA1404CSharp9UnitTests
{
[Fact]
public async Task TestUsingNameChangeInGlobalUsingInAnotherFileAsync()
{
var testCode1 = @"
global using MySuppressionAttribute = System.Diagnostics.CodeAnalysis.SuppressMessageAttribute;";

var testCode2 = @"
public class Foo
{
[[|MySuppression(null, null)|]]
public void Bar()
{
}
}";

var fixedCode2 = @"
public class Foo
{
[MySuppression(null, null, Justification = """ + SA1404CodeAnalysisSuppressionMustHaveJustification.JustificationPlaceholder + @""")]
public void Bar()
{
}
}";

await new CSharpTest()
{
TestSources = { testCode1, testCode2 },
FixedSources = { testCode1, fixedCode2 },
RemainingDiagnostics =
{
Diagnostic().WithLocation("/0/Test1.cs", 4, 32),
},
NumberOfIncrementalIterations = 2,
NumberOfFixAllIterations = 2,
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,30 @@ class Bar
FixedCode = newSource,
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
}

[Fact]
public async Task TestUsingNameChangeInGlobalUsingInAnotherFileAsync()
{
var source1 = @"
global using MyDouble = System.Double;";

var oldSource2 = @"
class TestClass
{
private [|MyDouble|] x;
}";

var newSource2 = @"
class TestClass
{
private double x;
}";

await new CSharpTest()
{
TestSources = { source1, oldSource2 },
FixedSources = { source1, newSource2 },
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
}
}
}
19 changes: 13 additions & 6 deletions StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxTreeHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static bool IsWhitespaceOnly(this SyntaxTree tree, CancellationToken canc
&& TriviaHelper.IndexOfFirstNonWhitespaceTrivia(firstToken.LeadingTrivia) == -1;
}

internal static bool ContainsUsingAlias(this SyntaxTree tree, ConcurrentDictionary<SyntaxTree, bool> cache)
internal static bool ContainsUsingAlias(this SyntaxTree tree, ConcurrentDictionary<SyntaxTree, bool> cache, SemanticModel semanticModel, CancellationToken cancellationToken)
{
if (tree == null)
{
Expand All @@ -85,16 +85,23 @@ internal static bool ContainsUsingAlias(this SyntaxTree tree, ConcurrentDictiona
return result;
}

bool generated = ContainsUsingAliasNoCache(tree);
bool generated = ContainsUsingAliasNoCache(tree, semanticModel, cancellationToken);
cache.TryAdd(tree, generated);
return generated;
}

private static bool ContainsUsingAliasNoCache(SyntaxTree tree)
private static bool ContainsUsingAliasNoCache(SyntaxTree tree, SemanticModel semanticModel, CancellationToken cancellationToken)
{
var nodes = tree.GetRoot().DescendantNodes(node => node.IsKind(SyntaxKind.CompilationUnit) || node.IsKind(SyntaxKind.NamespaceDeclaration) || node.IsKind(SyntaxKindEx.FileScopedNamespaceDeclaration));

return nodes.OfType<UsingDirectiveSyntax>().Any(x => x.Alias != null);
if (SemanticModelExtensions.SupportsGetImportScopes)
{
var scopes = semanticModel.GetImportScopes(0, cancellationToken);
return scopes.Any(x => x.Aliases.Length > 0);
}
else
{
var nodes = tree.GetRoot().DescendantNodes(node => node.IsKind(SyntaxKind.CompilationUnit) || node.IsKind(SyntaxKind.NamespaceDeclaration) || node.IsKind(SyntaxKindEx.FileScopedNamespaceDeclaration));
return nodes.OfType<UsingDirectiveSyntax>().Any(x => x.Alias != null);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace StyleCop.Analyzers.Lightup
{
using System;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;

internal readonly struct IImportScopeWrapper
{
internal const string WrappedTypeName = "Microsoft.CodeAnalysis.IImportScope";
private static readonly Type WrappedType;

private static readonly Func<object?, ImmutableArray<IAliasSymbol>> AliasesAccessor;

private readonly object node;

static IImportScopeWrapper()
{
WrappedType = WrapperHelper.GetWrappedType(typeof(IImportScopeWrapper));

AliasesAccessor = LightupHelpers.CreateSyntaxPropertyAccessor<object?, ImmutableArray<IAliasSymbol>>(WrappedType, nameof(Aliases));
}

private IImportScopeWrapper(object node)
{
this.node = node;
}

public ImmutableArray<IAliasSymbol> Aliases
{
get
{
if (this.node == null && WrappedType == null)
{
// Gracefully fall back to a collection with no values
return ImmutableArray<IAliasSymbol>.Empty;
}

return AliasesAccessor(this.node);
}
}

// NOTE: Referenced via reflection
public static IImportScopeWrapper FromObject(object node)
{
if (node == null)
{
return default;
}

if (!IsInstance(node))
{
throw new InvalidCastException($"Cannot cast '{node.GetType().FullName}' to '{WrappedTypeName}'");
}

return new IImportScopeWrapper(node);
}

public static bool IsInstance(object obj)
{
return obj != null && LightupHelpers.CanWrapObject(obj, WrappedType);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace StyleCop.Analyzers.Lightup
{
using System;
using System.Collections.Immutable;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Threading;
using Microsoft.CodeAnalysis;

internal static class SemanticModelExtensions
{
private static readonly Func<SemanticModel, int, CancellationToken, ImmutableArray<IImportScopeWrapper>> GetImportScopesAccessor;

static SemanticModelExtensions()
{
CreateGetImportScopesAccessor(out var accessor, out var isSupported);
GetImportScopesAccessor = accessor;
SupportsGetImportScopes = isSupported;
}

public static bool SupportsGetImportScopes { get; }

public static ImmutableArray<IImportScopeWrapper> GetImportScopes(this SemanticModel semanticModel, int position, CancellationToken cancellationToken = default)
{
return GetImportScopesAccessor(semanticModel, position, cancellationToken);
}

private static void CreateGetImportScopesAccessor(
out Func<SemanticModel, int, CancellationToken, ImmutableArray<IImportScopeWrapper>> accessor,
out bool isSupported)
{
// Very error-prone code and potentially brittle regarding future API changes.
// Wrapping everything in a try statement just in case something breaks later on.
try
{
var semanticModelType = typeof(SemanticModel);

var codeAnalysisWorkspacesAssembly = semanticModelType.GetTypeInfo().Assembly;
var nativeImportScopeType = codeAnalysisWorkspacesAssembly.GetType("Microsoft.CodeAnalysis.IImportScope");
if (nativeImportScopeType is null)
{
accessor = FallbackAccessor;
isSupported = false;
}

var methods = semanticModelType.GetTypeInfo().GetDeclaredMethods("GetImportScopes").Where(IsCorrectGetImportScopesMethod);
var method = methods.FirstOrDefault();
if (method == null)
{
accessor = FallbackAccessor;
isSupported = false;
}

var importScopeWrapperFromObjectMethod = typeof(IImportScopeWrapper).GetTypeInfo().GetDeclaredMethod("FromObject");
var nativeImportScopeArrayType = typeof(ImmutableArray<>).MakeGenericType(nativeImportScopeType);
var nativeImportScopeArrayGetItemMethod = nativeImportScopeArrayType.GetTypeInfo().GetDeclaredMethod("get_Item");
var wrapperImportScopeArrayType = typeof(ImmutableArray<IImportScopeWrapper>);
var wrapperImportScopeArrayBuilderType = typeof(ImmutableArray<IImportScopeWrapper>.Builder);
var wrapperImportScopeArrayBuilderAddMethod = wrapperImportScopeArrayBuilderType.GetTypeInfo().GetDeclaredMethod("Add");
var wrapperImportScopeArrayBuilderToImmutableMethod = wrapperImportScopeArrayBuilderType.GetTypeInfo().GetDeclaredMethod("ToImmutable");
var arrayCreateImportScopeBuilderMethod = typeof(ImmutableArray).GetTypeInfo().GetDeclaredMethods("CreateBuilder").Single(IsCorrectCreateBuilderMethod).MakeGenericMethod(typeof(IImportScopeWrapper));

var semanticModelParameter = Expression.Parameter(typeof(SemanticModel), "semanticModel");
var positionParameter = Expression.Parameter(typeof(int), "position");
var cancellationTokenParameter = Expression.Parameter(typeof(CancellationToken), "cancellationToken");

var nativeImportScopesVariable = Expression.Variable(nativeImportScopeArrayType, "nativeImportScopes");
var nativeImportScopeVariable = Expression.Variable(nativeImportScopeType, "nativeImportScope");
var countVariable = Expression.Variable(typeof(int), "count");
var indexVariable = Expression.Variable(typeof(int), "index");
var wrapperImportScopesBuilderVariable = Expression.Variable(wrapperImportScopeArrayBuilderType, "wrapperImportScopesBuilder");
var wrapperImportScopesVariable = Expression.Variable(wrapperImportScopeArrayType, "wrapperImoprtScopes");
var wrapperImportScopeVariable = Expression.Variable(typeof(IImportScopeWrapper), "wrapperImportScope");

var breakLabel = Expression.Label("break");
var block = Expression.Block(
new[] { nativeImportScopesVariable, countVariable, indexVariable, wrapperImportScopesBuilderVariable, wrapperImportScopesVariable },
//// nativeImportScopes = semanticModel.GetImportScopes(position, cancellationToken);
Expression.Assign(
nativeImportScopesVariable,
Expression.Call(
semanticModelParameter,
method,
new[] { positionParameter, cancellationTokenParameter })),
//// index = 0;
Expression.Assign(indexVariable, Expression.Constant(0)),
//// count = nativeImportScopes.Length;
Expression.Assign(
countVariable,
Expression.Property(nativeImportScopesVariable, "Length")),
//// wrapperImportScopesBuilder = ImmutableArray.CreateBuilder<IImportScopesWrapper>();
Expression.Assign(
wrapperImportScopesBuilderVariable,
Expression.Call(null, arrayCreateImportScopeBuilderMethod)),
Expression.Loop(
Expression.Block(
new[] { nativeImportScopeVariable, wrapperImportScopeVariable },
//// if (index >= count) break;
Expression.IfThen(
Expression.GreaterThanOrEqual(indexVariable, countVariable),
Expression.Break(breakLabel)),
//// nativeImportScope = nativeImportScopes[index];
Expression.Assign(
nativeImportScopeVariable,
Expression.Call(
nativeImportScopesVariable,
nativeImportScopeArrayGetItemMethod,
indexVariable)),
//// wrapperImportScope = IImportScopeWrapper.FromObject(nativeImportScope);
Expression.Assign(
wrapperImportScopeVariable,
Expression.Call(
null,
importScopeWrapperFromObjectMethod,
nativeImportScopeVariable)),
//// wrapperImportScopesBuilder.Add(wrapperImportScope);
Expression.Call(
wrapperImportScopesBuilderVariable,
wrapperImportScopeArrayBuilderAddMethod,
new[] { wrapperImportScopeVariable }),
//// index++;
Expression.PostIncrementAssign(indexVariable)),
breakLabel),
//// wrapperImportScopes = wrapperImportScopesBuilder.ToImmutable();
Expression.Assign(
wrapperImportScopesVariable,
Expression.Call(
wrapperImportScopesBuilderVariable,
wrapperImportScopeArrayBuilderToImmutableMethod)));

var lambda = Expression.Lambda<Func<SemanticModel, int, CancellationToken, ImmutableArray<IImportScopeWrapper>>>(
block,
new[] { semanticModelParameter, positionParameter, cancellationTokenParameter });

accessor = lambda.Compile();
isSupported = true;
}
catch (Exception)
{
accessor = FallbackAccessor;
isSupported = false;
}

// NOTE: At time of implementation, there is no other overload of GetImportScopes, but check in case more are added later on
static bool IsCorrectGetImportScopesMethod(MethodInfo method)
{
var parameters = method.GetParameters();
if (parameters.Length != 2)
{
return false;
}

return
parameters[0].ParameterType == typeof(int) &&
parameters[1].ParameterType == typeof(CancellationToken);
}

static bool IsCorrectCreateBuilderMethod(MethodInfo method)
{
var parameters = method.GetParameters();
return parameters.Length == 0;
}

static ImmutableArray<IImportScopeWrapper> FallbackAccessor(SemanticModel semanticModel, int position, CancellationToken cancellationToken)
{
if (semanticModel == null)
{
// Unlike an extension method which would throw ArgumentNullException here, the light-up
// behavior needs to match the behavior of the underlying method.
throw new NullReferenceException();
}

return ImmutableArray<IImportScopeWrapper>.Empty;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ static WrapperHelper()

builder.Add(typeof(AnalyzerConfigOptionsProviderWrapper), codeAnalysisAssembly.GetType(AnalyzerConfigOptionsProviderWrapper.WrappedTypeName));
builder.Add(typeof(AnalyzerConfigOptionsWrapper), codeAnalysisAssembly.GetType(AnalyzerConfigOptionsWrapper.WrappedTypeName));
builder.Add(typeof(IImportScopeWrapper), codeAnalysisAssembly.GetType(IImportScopeWrapper.WrappedTypeName));

WrappedTypes = builder.ToImmutable();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void HandleAttributeNode(SyntaxNodeAnalysisContext context)
var attribute = (AttributeSyntax)context.Node;

// Return fast if the name doesn't match and the file doesn't contain any using alias directives
if (!attribute.SyntaxTree.ContainsUsingAlias(this.usingAliasCache))
if (!attribute.SyntaxTree.ContainsUsingAlias(this.usingAliasCache, context.SemanticModel, context.CancellationToken))
{
if (!(attribute.Name is SimpleNameSyntax simpleNameSyntax))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context, StyleC
// Most source files will not have any using alias directives. Then we don't have to use semantics
// if the identifier name doesn't match the name of a special type
if (settings.ReadabilityRules.AllowBuiltInTypeAliases
|| !identifierNameSyntax.SyntaxTree.ContainsUsingAlias(this.usingAliasCache))
|| !identifierNameSyntax.SyntaxTree.ContainsUsingAlias(this.usingAliasCache, context.SemanticModel, context.CancellationToken))
{
switch (identifierNameSyntax.Identifier.ValueText)
{
Expand Down

0 comments on commit 0d855cd

Please sign in to comment.