Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 17, 2025

Summary

Successfully implemented MA0179: Use Attribute.IsDefined instead of GetCustomAttribute(s)

Completed Tasks

  • Add new rule identifier MA0179 for UseAttributeIsDefined
  • Create UseAttributeIsDefinedAnalyzer to detect inefficient attribute checks
    • Detect GetCustomAttribute() comparisons with null (!=, ==, is, is not)
    • Detect GetCustomAttributes().Any() pattern (without predicate)
    • Detect GetCustomAttributes().Count() comparisons (without predicate)
    • Detect GetCustomAttributes().Length comparisons (only clear-cut patterns)
    • Support Assembly, Module, MemberInfo, and Type types
    • Use specific IMethodSymbol for Any and Count detection
    • Extract documentation IDs as constants
    • Properly handle extension methods
  • Create UseAttributeIsDefinedFixer to replace with Attribute.IsDefined
    • Use specific IMethodSymbol for Any and Count detection
    • Extract documentation IDs as constants
    • Properly handle extension methods
  • Create comprehensive tests for the analyzer and fixer
    • Fixed test diagnostic markers to use proper span scope
    • Fixed GetCustomAttributes tests to use non-generic version for Length property
  • Address all code review feedback
  • Update documentation
  • Build validation - SUCCESS

Latest Changes

  • Fixed analyzer and fixer to properly handle extension methods (GetCustomAttribute, GetCustomAttributes)
  • For extension methods, the instance is in the first argument, not in the Instance property
  • Added null checks for IMethodSymbol comparisons to prevent silent failures
Original prompt

Can you create a new rule that detect GetCustomAttribute() != null, GetCustomAttribute() == null, GetCustomAttribute() is null, GetCustomAttribute() is not null, or GetCustomAttributes().Any() on all Assembly, Module, MemberInfo, Type.
It should suggest to use Attribute.IsDefined instead.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add rule to replace GetCustomAttribute checks Add MA0179: Detect inefficient attribute existence checks Dec 17, 2025
Copilot AI requested a review from meziantou December 17, 2025 19:12
@meziantou
Copy link
Owner

@copilot Can you also detect GetCustomAttributes().Length

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot Can you also detect GetCustomAttributes().Length

Done! Added support for detecting GetCustomAttributes().Length comparisons. The analyzer now detects clear-cut patterns like:

  • length > 0, length >= 1, length != 0IsDefined
  • length == 0, length <= 0, length < 1!IsDefined
  • Reversed patterns like 0 < length or 1 > length

Commit: a491f10

@meziantou
Copy link
Owner

@copilot Can you add a test for CustomAttributes().Any(attr => true) to be sure it is not reported. Also, can you support the Count() extension method in addition to Any?

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot Can you add a test for CustomAttributes().Any(attr => true) to be sure it is not reported. Also, can you support the Count() extension method in addition to Any?

Done!

  1. Added test for Any(attr => true) to confirm it's not reported (predicates are correctly ignored)
  2. Added support for Count() extension method - detects patterns like Count() > 0, Count() == 0, etc. (without predicate only)
  3. Added test for Count(predicate) to verify it's not reported

Commits: 92cf7c5, e195b78, 3bad1cb

{
var operation = (IInvocationOperation)context.Operation;

if (operation.TargetMethod.Name != "Any")
Copy link
Owner

Choose a reason for hiding this comment

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

@copilot Can you be more specific and use a IMethodSymbol to actually compare it or check if the method is defined in System.Linq.Enumerable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Changed to use specific IMethodSymbol comparison via DocumentationCommentId.GetFirstSymbolForDeclarationId for both Any and Count methods. This ensures we only detect the exact System.Linq.Enumerable.Any<T>(IEnumerable<T>) and System.Linq.Enumerable.Count<T>(IEnumerable<T>) methods, making the detection more precise and robust.

Commits: 4440f70, 877ccaa

{
void Test(MemberInfo member)
{
_ = [||]member.GetCustomAttribute<ObsoleteAttribute>() != null;
Copy link
Owner

Choose a reason for hiding this comment

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

@copilot Use the correct scope here and in every tests

Suggested change
_ = [||]member.GetCustomAttribute<ObsoleteAttribute>() != null;
_ = [|member.GetCustomAttribute<ObsoleteAttribute>() != null|];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! Changed all test diagnostic markers from [||]expression to [|expression|] to properly indicate the diagnostic span scope.

Commit: 6ea0409

This was referenced Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants