Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SA1121 and SA1404 to detect global using aliases, using GetImportScopes and compiling expression trees #3678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.Test.CSharp10.Lightup
{
using StyleCop.Analyzers.Test.CSharp9.Lightup;

public partial class IImportScopeWrapperCSharp10UnitTests : IImportScopeWrapperCSharp9UnitTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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.Test.CSharp11.Lightup
{
using System.Collections.Generic;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Moq;
using StyleCop.Analyzers.Lightup;
using StyleCop.Analyzers.Test.CSharp10.Lightup;
using Xunit;

public partial class IImportScopeWrapperCSharp11UnitTests : IImportScopeWrapperCSharp10UnitTests
{
[Theory]
[InlineData(0)]
[InlineData(1)]
[InlineData(2)]
public void TestCompatibleInstance(int numberOfAliasSymbols)
{
var obj = CreateImportScope(numberOfAliasSymbols);
Assert.True(IImportScopeWrapper.IsInstance(obj));
var wrapper = IImportScopeWrapper.FromObject(obj);
Assert.Equal(obj.Aliases, wrapper.Aliases);
Copy link
Contributor Author

@bjornhellander bjornhellander Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this test to verify the actual alias symbols returned from the wrapper. Used the Moq package to be able to create IAliasSymbol instances more easily. Ok to add that dependency?

}

private static IImportScope CreateImportScope(int numberOfAliasSymbols)
{
var aliasSymbolMocks = new List<IAliasSymbol>();
for (var i = 0; i < numberOfAliasSymbols; i++)
{
aliasSymbolMocks.Add(Mock.Of<IAliasSymbol>());
}

var importScopeMock = new Mock<IImportScope>();
importScopeMock.Setup(x => x.Aliases).Returns(aliasSymbolMocks.ToImmutableArray());
return importScopeMock.Object;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,58 @@

namespace StyleCop.Analyzers.Test.CSharp11.MaintainabilityRules
{
using System.Threading;
using System.Threading.Tasks;
using StyleCop.Analyzers.MaintainabilityRules;
using StyleCop.Analyzers.Test.CSharp10.MaintainabilityRules;
using Xunit;

using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.MaintainabilityRules.SA1404CodeAnalysisSuppressionMustHaveJustification,
StyleCop.Analyzers.MaintainabilityRules.SA1404CodeFixProvider>;

public partial class SA1404CSharp11UnitTests : SA1404CSharp10UnitTests
{
// NOTE: This tests a fix for a c# 10 feature, but the Roslyn API used to solve it wasn't available in the version
// we use in the c# 10 test project, so the test was added here instead.
[Fact]
[WorkItem(3594, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3594")]
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 @@ -3,9 +3,43 @@

namespace StyleCop.Analyzers.Test.CSharp11.ReadabilityRules
{
using System.Threading;
using System.Threading.Tasks;
using StyleCop.Analyzers.Test.CSharp10.ReadabilityRules;
using Xunit;

using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.ReadabilityRules.SA1121UseBuiltInTypeAlias,
StyleCop.Analyzers.ReadabilityRules.SA1121CodeFixProvider>;

public partial class SA1121CSharp11UnitTests : SA1121CSharp10UnitTests
{
// NOTE: This tests a fix for a c# 10 feature, but the Roslyn API used to solve it wasn't available in the version
// we use in the c# 10 test project, so the test was added here instead.
[Fact]
[WorkItem(3594, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3594")]
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.4.0" />
<PackageReference Include="Moq" Version="4.20.70" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.1" PrivateAssets="all" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.Test.CSharp12.Lightup
{
using StyleCop.Analyzers.Test.CSharp11.Lightup;

public partial class IImportScopeWrapperCSharp12UnitTests : IImportScopeWrapperCSharp11UnitTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.Test.CSharp7.Lightup
{
using StyleCop.Analyzers.Test.Lightup;

public partial class IImportScopeWrapperCSharp7UnitTests : IImportScopeWrapperUnitTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.Test.CSharp8.Lightup
{
using StyleCop.Analyzers.Test.CSharp7.Lightup;

public partial class IImportScopeWrapperCSharp8UnitTests : IImportScopeWrapperCSharp7UnitTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.Test.CSharp9.Lightup
{
using StyleCop.Analyzers.Test.CSharp8.Lightup;

public partial class IImportScopeWrapperCSharp9UnitTests : IImportScopeWrapperCSharp8UnitTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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.Test.Lightup
{
using System;
using StyleCop.Analyzers.Lightup;
using Xunit;

public class IImportScopeWrapperUnitTests
{
[Fact]
public void TestNull()
{
object? obj = null;
Assert.False(IImportScopeWrapper.IsInstance(obj!));
var wrapper = IImportScopeWrapper.FromObject(obj!);
Assert.Throws<NullReferenceException>(() => wrapper.Aliases);
}

[Fact]
public void TestIncompatibleInstance()
{
var obj = new object();
Assert.False(IImportScopeWrapper.IsInstance(obj));
Assert.Throws<InvalidCastException>(() => IImportScopeWrapper.FromObject(obj));
}
}
}
15 changes: 11 additions & 4 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)
{
// Check for "local" using aliases
var nodes = tree.GetRoot().DescendantNodes(node => node.IsKind(SyntaxKind.CompilationUnit) || node.IsKind(SyntaxKind.NamespaceDeclaration) || node.IsKind(SyntaxKindEx.FileScopedNamespaceDeclaration));
if (nodes.OfType<UsingDirectiveSyntax>().Any(x => x.Alias != null))
{
return true;
}

return nodes.OfType<UsingDirectiveSyntax>().Any(x => x.Alias != null);
// Check for global using aliases
var scopes = semanticModel.GetImportScopes(0, cancellationToken);
return scopes.Any(x => x.Aliases.Length > 0);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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 => 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);
}
}
}
Loading