Skip to content

Fix S3993 FP: Allow abstract attributes not to decorate Attribute usage #8898

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.

Sign up for GitHub

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

Merged
antonioaversa merged 3 commits into from
Mar 20, 2024
Merged

Fix S3993 FP: Allow abstract attributes not to decorate Attribute usage #8898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa63dec
Allow abstract attributes not to decorate Attribute usage.
Corniel Mar 11, 2024
60fc768
simplify.
Corniel Mar 19, 2024
0dbb8b4
t => x
Corniel Mar 19, 2024
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
58 changes: 26 additions & 32 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/RequireAttributeUsageAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,47 +18,41 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp
namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class RequireAttributeUsageAttribute : SonarDiagnosticAnalyzer
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class RequireAttributeUsageAttribute : SonarDiagnosticAnalyzer
{
internal const string DiagnosticId = "S3993";
private const string MessageFormat = "Specify AttributeUsage on '{0}'{1}.";
internal const string DiagnosticId = "S3993";
private const string MessageFormat = "Specify AttributeUsage on '{0}'{1}.";

private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);
private static readonly DiagnosticDescriptor Rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);

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

protected override void Initialize(SonarAnalysisContext context)
protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(c =>
{
context.RegisterNodeAction(c =>
{
var classDeclaration = (ClassDeclarationSyntax)c.Node;
var classSymbol = c.SemanticModel.GetDeclaredSymbol(classDeclaration);

if (classSymbol == null ||
!classSymbol.DerivesFrom(KnownType.System_Attribute) ||
classSymbol.HasAttribute(KnownType.System_AttributeUsageAttribute))
{
return;
}
var classDeclaration = (ClassDeclarationSyntax)c.Node;

if (c.SemanticModel.GetDeclaredSymbol(classDeclaration) is { IsAbstract: false } classSymbol
&& classSymbol.DerivesFrom(KnownType.System_Attribute)
&& !classSymbol.HasAttribute(KnownType.System_AttributeUsageAttribute))
{
var additionalText = InheritsAttributeUsage(classSymbol)
? " to improve readability, even though it inherits it from its base type"
: string.Empty;

c.ReportIssue(Diagnostic.Create(rule, classDeclaration.Identifier.GetLocation(),
c.ReportIssue(Diagnostic.Create(Rule, classDeclaration.Identifier.GetLocation(),
classSymbol.Name, additionalText));
},
SyntaxKind.ClassDeclaration);
}

private static bool InheritsAttributeUsage(INamedTypeSymbol classSymbol) =>
classSymbol.GetSelfAndBaseTypes()
// System.Attribute already has AttributeUsage, we don't want to report it
.TakeWhile(t => !t.Is(KnownType.System_Attribute))
.Any(t => t.HasAttribute(KnownType.System_AttributeUsageAttribute));
}
}
},
SyntaxKind.ClassDeclaration);

private static bool InheritsAttributeUsage(INamedTypeSymbol classSymbol) =>
classSymbol.GetSelfAndBaseTypes()
// System.Attribute already has AttributeUsage, we don't want to report it
.TakeWhile(x => !x.Is(KnownType.System_Attribute))
.Any(x => x.HasAttribute(KnownType.System_AttributeUsageAttribute));
}
15 changes: 7 additions & 8 deletions analyzers/tests/SonarAnalyzer.Test/Rules/MarkAssemblyWithAttributeUsageAttributeTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@

using SonarAnalyzer.Rules.CSharp;

namespace SonarAnalyzer.Test.Rules
namespace SonarAnalyzer.Test.Rules;

[TestClass]
public class MarkAssemblyWithAttributeUsageAttributeTest
{
[TestClass]
public class MarkAssemblyWithAttributeUsageAttributeTest
{
[TestMethod]
public void RequireAttributeUsageAttribute() =>
new VerifierBuilder<RequireAttributeUsageAttribute>().AddPaths(@"RequireAttributeUsageAttribute.cs").Verify();
}
[TestMethod]
public void RequireAttributeUsageAttribute() =>
new VerifierBuilder<RequireAttributeUsageAttribute>().AddPaths(@"RequireAttributeUsageAttribute.cs").Verify();
}
27 changes: 13 additions & 14 deletions analyzers/tests/SonarAnalyzer.Test/Rules/RequireAttributeUsageAttributeTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,25 @@

using SonarAnalyzer.Rules.CSharp;

namespace SonarAnalyzer.Test.Rules
namespace SonarAnalyzer.Test.Rules;

[TestClass]
public class RequireAttributeUsageAttributeTest
{
[TestClass]
public class RequireAttributeUsageAttributeTest
{
private readonly VerifierBuilder builder = new VerifierBuilder<RequireAttributeUsageAttribute>();
private readonly VerifierBuilder builder = new VerifierBuilder<RequireAttributeUsageAttribute>();

[TestMethod]
public void RequireAttributeUsageAttribute() =>
builder.AddPaths("RequireAttributeUsageAttribute.cs").Verify();
[TestMethod]
public void RequireAttributeUsageAttribute() =>
builder.AddPaths("RequireAttributeUsageAttribute.cs").Verify();

#if NET

[TestMethod]
public void RequireAttributeUsageAttribute_CSharp11() =>
builder.AddPaths("RequireAttributeUsageAttribute.CSharp11.cs")
.WithOptions(ParseOptionsHelper.FromCSharp11)
.Verify();
[TestMethod]
public void RequireAttributeUsageAttribute_CSharp11() =>
builder.AddPaths("RequireAttributeUsageAttribute.CSharp11.cs")
.WithOptions(ParseOptionsHelper.FromCSharp11)
.Verify();

#endif

}
}
35 changes: 18 additions & 17 deletions analyzers/tests/SonarAnalyzer.Test/TestCases/RequireAttributeUsageAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
using System;

namespace Tests.Diagnostics
public class MyInvalidAttribute : Attribute
// ^^^^^^^^^^^^^^^^^^ {{Specify AttributeUsage on 'MyInvalidAttribute'.}}
{
public class MyInvalidAttribute : Attribute
// ^^^^^^^^^^^^^^^^^^ {{Specify AttributeUsage on 'MyInvalidAttribute'.}}
{
}
}

[AttributeUsage(AttributeTargets.Class)]
public class MyCompliantAttribute : Attribute
{
}
[AttributeUsage(AttributeTargets.Class)]
public class MyCompliantAttribute : Attribute
{
}

public class MyInvalidInheritedAttribute : MyCompliantAttribute
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ {{Specify AttributeUsage on 'MyInvalidInheritedAttribute' to improve readability, even though it inherits it from its base type.}}
{
}
public class MyInvalidInheritedAttribute : MyCompliantAttribute
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ {{Specify AttributeUsage on 'MyInvalidInheritedAttribute' to improve readability, even though it inherits it from its base type.}}
{
}

[AttributeUsage(AttributeTargets.Class)]
public class MyInheritedAttribute : MyCompliantAttribute
{
}
[AttributeUsage(AttributeTargets.Class)]
public class MyInheritedAttribute : MyCompliantAttribute
{
}

public abstract class AbstractAttribute : Attribute // Compliant
{
}