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

Improved performance of CA1041 #6671

Open
wants to merge 3 commits into
base: main
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,98 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines;

namespace Microsoft.CodeQuality.CSharp.Analyzers.ApiDesignGuidelines
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpProvideObsoleteAttributeMessageAnalyzer : ProvideObsoleteAttributeMessageAnalyzer
{
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(AnalyzeSyntaxNode, SyntaxKind.Attribute);
}

private static void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context)
{
var attribute = (AttributeSyntax)context.Node;

if (!IsObsoleteAttributeName(StripOffNamespace(attribute.Name).Identifier.Text))
return;

// We bail if the attribute has arguments, unless the first argument is null or empty.
if (attribute.ArgumentList is not null && attribute.ArgumentList.Arguments.Count > 0)
{
if (attribute.ArgumentList.Arguments[0].Expression is not LiteralExpressionSyntax literalExpression)
return;
if (!string.IsNullOrEmpty(literalExpression.Token.Value.ToString()))
return;
}

string identifierName = string.Empty;
ISymbol? attributedSymbol = null;

switch (attribute.Parent.Parent)
{
case TypeDeclarationSyntax typeDeclaration:
identifierName = typeDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(typeDeclaration, context.CancellationToken);
break;
case ConstructorDeclarationSyntax constructorDeclaration:
identifierName = constructorDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(constructorDeclaration, context.CancellationToken);
break;
case FieldDeclarationSyntax fieldDeclaration:
identifierName = fieldDeclaration.Declaration.Variables.ToString();
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(fieldDeclaration.Declaration.Variables[0], context.CancellationToken);
break;
case PropertyDeclarationSyntax propertyDeclaration:
identifierName = propertyDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(propertyDeclaration, context.CancellationToken);
break;
case MethodDeclarationSyntax methodDeclaration:
identifierName = methodDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(methodDeclaration, context.CancellationToken);
break;
case EventFieldDeclarationSyntax eventFieldDeclaration:
identifierName = eventFieldDeclaration.Declaration.Variables.ToString();
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(eventFieldDeclaration.Declaration.Variables[0], context.CancellationToken);
break;
case EventDeclarationSyntax eventDeclaration:
identifierName = eventDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(eventDeclaration, context.CancellationToken);
break;
case DelegateDeclarationSyntax delegateDeclaration:
identifierName = delegateDeclaration.Identifier.Text;
attributedSymbol = context.SemanticModel.GetDeclaredSymbol(delegateDeclaration, context.CancellationToken);
break;
}

if (attributedSymbol is not null && attributedSymbol.DeclaredAccessibility is Accessibility.Public)
{
if (attributedSymbol.ContainingType is not null && attributedSymbol.ContainingType.DeclaredAccessibility is not Accessibility.Public)
return;

context.ReportDiagnostic(attribute.CreateDiagnostic(Rule, identifierName));
}

return;

// Local functions

static IdentifierNameSyntax StripOffNamespace(NameSyntax name)
{
while (name is QualifiedNameSyntax qualifiedName)
name = qualifiedName.Right;

return (IdentifierNameSyntax)name;
Copy link
Member

Choose a reason for hiding this comment

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

This might crash for AliasQualifiedNameSyntax or GenericNameSyntax

}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Analyzer.Utilities;
using System.Linq;
using Analyzer.Utilities.Extensions;

namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines
{
Expand All @@ -14,8 +12,7 @@ namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines
/// <summary>
/// CA1041: <inheritdoc cref="ProvideObsoleteAttributeMessageTitle"/>
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class ProvideObsoleteAttributeMessageAnalyzer : DiagnosticAnalyzer
public abstract class ProvideObsoleteAttributeMessageAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA1041";

Expand All @@ -29,55 +26,8 @@ public sealed class ProvideObsoleteAttributeMessageAnalyzer : DiagnosticAnalyzer
isPortedFxCopRule: true,
isDataflowRule: false);

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

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationStartAction(compilationContext =>
{
INamedTypeSymbol? obsoleteAttributeType = compilationContext.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemObsoleteAttribute);
if (obsoleteAttributeType == null)
{
return;
}

compilationContext.RegisterSymbolAction(sc => AnalyzeSymbol(sc, obsoleteAttributeType),
SymbolKind.NamedType,
SymbolKind.Method,
SymbolKind.Field,
SymbolKind.Property,
SymbolKind.Event);
});
}

private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol obsoleteAttributeType)
{
// FxCop compat: only analyze externally visible symbols by default
if (!context.Options.MatchesConfiguredVisibility(Rule, context.Symbol, context.Compilation))
{
return;
}

ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();
foreach (AttributeData attribute in attributes)
{
if (attribute.AttributeClass.Equals(obsoleteAttributeType))
{
// ObsoleteAttribute has a constructor that takes no params and two
// other constructors that take a message as the first param.
// If there are no arguments specified or if the message argument is empty
// then report a diagnostic.
if (attribute.ConstructorArguments.IsEmpty ||
string.IsNullOrEmpty(attribute.ConstructorArguments.First().Value as string))
{
SyntaxNode node = attribute.ApplicationSyntaxReference.GetSyntax(context.CancellationToken);
context.ReportDiagnostic(node.CreateDiagnostic(Rule, context.Symbol.Name));
}
}
}
}
private protected static bool IsObsoleteAttributeName(string attributeName) => attributeName is "Obsolete" or "ObsoleteAttribute";
}
}
61 changes: 40 additions & 21 deletions src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,26 @@
]
}
},
"CA1041": {
"id": "CA1041",
"shortDescription": "Provide ObsoleteAttribute message",
"fullDescription": "A type or member is marked by using a System.ObsoleteAttribute attribute that does not have its ObsoleteAttribute.Message property specified. When a type or member that is marked by using ObsoleteAttribute is compiled, the Message property of the attribute is displayed. This gives the user information about the obsolete type or member.",
"defaultLevel": "note",
"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1041",
"properties": {
"category": "Design",
"isEnabledByDefault": true,
"typeName": "CSharpProvideObsoleteAttributeMessageAnalyzer",
"languages": [
"C#"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1200": {
"id": "CA1200",
"shortDescription": "Avoid using cref tags with a prefix",
Expand Down Expand Up @@ -1042,27 +1062,6 @@
]
}
},
"CA1041": {
"id": "CA1041",
"shortDescription": "Provide ObsoleteAttribute message",
"fullDescription": "A type or member is marked by using a System.ObsoleteAttribute attribute that does not have its ObsoleteAttribute.Message property specified. When a type or member that is marked by using ObsoleteAttribute is compiled, the Message property of the attribute is displayed. This gives the user information about the obsolete type or member.",
"defaultLevel": "note",
"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1041",
"properties": {
"category": "Design",
"isEnabledByDefault": true,
"typeName": "ProvideObsoleteAttributeMessageAnalyzer",
"languages": [
"C#",
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1043": {
"id": "CA1043",
"shortDescription": "Use Integral Or String Argument For Indexers",
Expand Down Expand Up @@ -6056,6 +6055,26 @@
]
}
},
"CA1041": {
"id": "CA1041",
"shortDescription": "Provide ObsoleteAttribute message",
"fullDescription": "A type or member is marked by using a System.ObsoleteAttribute attribute that does not have its ObsoleteAttribute.Message property specified. When a type or member that is marked by using ObsoleteAttribute is compiled, the Message property of the attribute is displayed. This gives the user information about the obsolete type or member.",
"defaultLevel": "note",
"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1041",
"properties": {
"category": "Design",
"isEnabledByDefault": true,
"typeName": "BasicProvideObsoleteAttributeMessageAnalyzer",
"languages": [
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1200": {
"id": "CA1200",
"shortDescription": "Avoid using cref tags with a prefix",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using Test.Utilities;
using Xunit;
using VerifyCS = Test.Utilities.CSharpCodeFixVerifier<
Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.ProvideObsoleteAttributeMessageAnalyzer,
Microsoft.CodeQuality.CSharp.Analyzers.ApiDesignGuidelines.CSharpProvideObsoleteAttributeMessageAnalyzer,
Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
using VerifyVB = Test.Utilities.VisualBasicCodeFixVerifier<
Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.ProvideObsoleteAttributeMessageAnalyzer,
Microsoft.CodeQuality.VisualBasic.Analyzers.ApiDesignGuidelines.BasicProvideObsoleteAttributeMessageAnalyzer,
Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;

namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.UnitTests
Expand All @@ -21,33 +21,40 @@ public async Task CSharpSimpleCasesAsync()
await VerifyCS.VerifyAnalyzerAsync(@"
using System;

[Obsolete]
[{|#0:Obsolete|}]
public class A
{
[Obsolete]
[{|#1:Obsolete|}]
public A() { }
[Obsolete("""")]
[{|#2:Obsolete("""")|}]
public int field;
[Obsolete]
[{|#3:Obsolete|}]
public int Property { get; set; }
[Obsolete]
[{|#4:Obsolete|}]
public void Method() {}
[Obsolete]
public event EventHandler<int> event1;
[{|#5:Obsolete|}]
public event EventHandler<int> FieldEvent;
[{|#6:Obsolete|}]
public event EventHandler<int> PropertyEvent
{
add { }
remove { }
}
}
[Obsolete]
[{|#7:Obsolete|}]
public interface I {}
[Obsolete]
[{|#8:Obsolete|}]
public delegate void del(int x);
",
GetCSharpResultAt(4, 2, "A"),
GetCSharpResultAt(7, 6, ".ctor"),
GetCSharpResultAt(9, 6, "field"),
GetCSharpResultAt(11, 6, "Property"),
GetCSharpResultAt(13, 6, "Method"),
GetCSharpResultAt(15, 6, "event1"),
GetCSharpResultAt(18, 2, "I"),
GetCSharpResultAt(20, 2, "del"));
VerifyCS.Diagnostic().WithLocation(0).WithArguments("A"),
VerifyCS.Diagnostic().WithLocation(1).WithArguments("A"),
VerifyCS.Diagnostic().WithLocation(2).WithArguments("field"),
VerifyCS.Diagnostic().WithLocation(3).WithArguments("Property"),
VerifyCS.Diagnostic().WithLocation(4).WithArguments("Method"),
VerifyCS.Diagnostic().WithLocation(5).WithArguments("FieldEvent"),
VerifyCS.Diagnostic().WithLocation(6).WithArguments("PropertyEvent"),
VerifyCS.Diagnostic().WithLocation(7).WithArguments("I"),
VerifyCS.Diagnostic().WithLocation(8).WithArguments("del"));
}

[Fact]
Expand Down Expand Up @@ -78,7 +85,7 @@ End Interface
Public Delegate Sub del(x As Integer)
",
GetBasicResultAt(4, 2, "A"),
GetBasicResultAt(6, 6, ".ctor"),
GetBasicResultAt(6, 6, "New"),
GetBasicResultAt(9, 6, "field"),
GetBasicResultAt(11, 6, "prop"),
GetBasicResultAt(13, 6, "Method"),
Expand Down Expand Up @@ -186,13 +193,6 @@ End Class
");
}

private static DiagnosticResult GetCSharpResultAt(int line, int column, string symbolName)
#pragma warning disable RS0030 // Do not use banned APIs
=> VerifyCS.Diagnostic()
.WithLocation(line, column)
#pragma warning restore RS0030 // Do not use banned APIs
.WithArguments(symbolName);

private static DiagnosticResult GetBasicResultAt(int line, int column, string symbolName)
#pragma warning disable RS0030 // Do not use banned APIs
=> VerifyVB.Diagnostic()
Expand Down
Loading