diff --git a/docs/Rules/MA0179.md b/docs/Rules/MA0179.md index 9d2b04c78..dbda88e37 100644 --- a/docs/Rules/MA0179.md +++ b/docs/Rules/MA0179.md @@ -45,3 +45,21 @@ if (attr != null) _ = attr.Message; } ```` + +## Attribute inheritance on properties and events + +The instance methods `MemberInfo.GetCustomAttributes(bool)` and `MemberInfo.GetCustomAttributes(Type, bool)` ignore the `inherit` parameter for properties and events, whereas `Attribute.IsDefined(MemberInfo, Type, bool)` also searches the overridden properties and events when `inherit` is `true`. To keep the same behavior, the code fix uses the instance method `MemberInfo.IsDefined(Type, bool)` when the member can be a property or an event and `inherit` is not `false`: + +````csharp +using System; +using System.Reflection; + +void Sample(MemberInfo member) +{ + // non-compliant + _ = member.GetCustomAttributes(typeof(ObsoleteAttribute), inherit: true).Length > 0; + + // compliant (fixed code) + _ = member.IsDefined(typeof(ObsoleteAttribute), inherit: true); +} +```` diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/UseAttributeIsDefinedFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/UseAttributeIsDefinedFixer.cs index 4e73983b2..99798c939 100644 --- a/src/Meziantou.Analyzer.CodeFixers/Rules/UseAttributeIsDefinedFixer.cs +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/UseAttributeIsDefinedFixer.cs @@ -271,7 +271,7 @@ private static SyntaxNode CreateAttributeIsDefinedInvocation(SyntaxGenerator gen arguments.Add(typeSyntax); // Find inherit argument - SyntaxNode? inheritSyntax = null; + IArgumentOperation? inheritArgument = null; foreach (var arg in invocation.Arguments) { // Skip instance argument for extension methods @@ -284,19 +284,30 @@ private static SyntaxNode CreateAttributeIsDefinedInvocation(SyntaxGenerator gen if (arg.Parameter?.Type.SpecialType == SpecialType.System_Boolean && arg.Parameter.Name == "inherit") { - inheritSyntax = arg.Syntax; + inheritArgument = arg; break; } } - if (inheritSyntax is not null) + SyntaxNode isDefinedInvocation; + if (instanceSyntax is not null && inheritArgument is not null && MustUseInstanceIsDefined(semanticModel.Compilation, invocation, inheritArgument)) { - arguments.Add(inheritSyntax); + isDefinedInvocation = generator.InvocationExpression( + generator.MemberAccessExpression(instanceSyntax, "IsDefined"), + typeSyntax, + inheritArgument.Syntax); } + else + { + if (inheritArgument is not null) + { + arguments.Add(inheritArgument.Syntax); + } - var isDefinedInvocation = generator.InvocationExpression( - generator.MemberAccessExpression(attributeTypeSyntax, "IsDefined"), - arguments); + isDefinedInvocation = generator.InvocationExpression( + generator.MemberAccessExpression(attributeTypeSyntax, "IsDefined"), + arguments); + } if (negate) { @@ -305,4 +316,24 @@ private static SyntaxNode CreateAttributeIsDefinedInvocation(SyntaxGenerator gen return isDefinedInvocation; } + + // The instance methods, such as MemberInfo.GetCustomAttributes(Type, bool), ignore 'inherit' for properties and events, + // while Attribute.IsDefined(MemberInfo, Type, bool) walks the chain of the overridden properties and events. + // MemberInfo.IsDefined(Type, bool) behaves like the instance methods, so it is used when the results could differ. + private static bool MustUseInstanceIsDefined(Compilation compilation, IInvocationOperation invocation, IArgumentOperation inheritArgument) + { + if (invocation.Instance?.Type is not { } instanceType) + return false; + + if (inheritArgument.Value.ConstantValue is { HasValue: true, Value: false }) + return false; + + return CanBeInstanceOf(instanceType, compilation.GetBestTypeByMetadataName("System.Reflection.PropertyInfo")) || + CanBeInstanceOf(instanceType, compilation.GetBestTypeByMetadataName("System.Reflection.EventInfo")); + + static bool CanBeInstanceOf(ITypeSymbol type, INamedTypeSymbol? expectedType) + { + return expectedType is not null && (type.IsOrInheritsFrom(expectedType) || expectedType.IsOrInheritsFrom(type)); + } + } } diff --git a/tests/Meziantou.Analyzer.Test/Rules/UseAttributeIsDefinedAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/UseAttributeIsDefinedAnalyzerTests.cs index 0420030b7..a9c163129 100644 --- a/tests/Meziantou.Analyzer.Test/Rules/UseAttributeIsDefinedAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/UseAttributeIsDefinedAnalyzerTests.cs @@ -853,7 +853,7 @@ class TestClass { void Test(MemberInfo member) { - _ = Attribute.IsDefined(member, typeof(ObsoleteAttribute), inherit: true); + _ = member.IsDefined(typeof(ObsoleteAttribute), inherit: true); } } """; @@ -885,7 +885,7 @@ class TestClass { void Test(MemberInfo member) { - _ = Attribute.IsDefined(member, typeof(Attribute), inherit: true); + _ = member.IsDefined(typeof(Attribute), inherit: true); } } """; @@ -987,7 +987,174 @@ class TestClass { void Test(MemberInfo member) { - _ = Attribute.IsDefined(member, typeof(ObsoleteAttribute), inherit: true); + _ = member.IsDefined(typeof(ObsoleteAttribute), inherit: true); + } + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("PropertyInfo")] + [InlineData("EventInfo")] + public Task InstanceGetCustomAttributes_WithInherit_PropertyOrEvent_UsesInstanceIsDefined(string memberType) + { + var test = CreateTest(); + test.TestCode = $$""" + using System; + using System.Reflection; + + class TestClass + { + void Test({{memberType}} member) + { + _ = {|MA0179:member.GetCustomAttributes(typeof(ObsoleteAttribute), true).Length > 0|}; + } + } + """; + test.FixedCode = $$""" + using System; + using System.Reflection; + + class TestClass + { + void Test({{memberType}} member) + { + _ = member.IsDefined(typeof(ObsoleteAttribute), true); + } + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task InstanceGetCustomAttributes_WithNonConstantInherit_UsesInstanceIsDefined() + { + var test = CreateTest(); + test.TestCode = """ + using System; + using System.Reflection; + + class TestClass + { + void Test(MemberInfo member, bool inherit) + { + _ = {|MA0179:member.GetCustomAttributes(typeof(ObsoleteAttribute), inherit).Length == 0|}; + } + } + """; + test.FixedCode = """ + using System; + using System.Reflection; + + class TestClass + { + void Test(MemberInfo member, bool inherit) + { + _ = !member.IsDefined(typeof(ObsoleteAttribute), inherit); + } + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task InstanceGetCustomAttributes_WithoutInherit_Property_UsesAttributeIsDefined() + { + var test = CreateTest(); + test.TestCode = """ + using System; + using System.Reflection; + + class TestClass + { + void Test(PropertyInfo member) + { + _ = {|MA0179:member.GetCustomAttributes(typeof(ObsoleteAttribute), inherit: false).Length > 0|}; + } + } + """; + test.FixedCode = """ + using System; + using System.Reflection; + + class TestClass + { + void Test(PropertyInfo member) + { + _ = Attribute.IsDefined(member, typeof(ObsoleteAttribute), inherit: false); + } + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("Type")] + [InlineData("MethodInfo")] + [InlineData("FieldInfo")] + public Task InstanceGetCustomAttributes_WithInherit_NotPropertyOrEvent_UsesAttributeIsDefined(string memberType) + { + var test = CreateTest(); + test.TestCode = $$""" + using System; + using System.Reflection; + + class TestClass + { + void Test({{memberType}} member) + { + _ = {|MA0179:member.GetCustomAttributes(typeof(ObsoleteAttribute), true).Length > 0|}; + } + } + """; + test.FixedCode = $$""" + using System; + using System.Reflection; + + class TestClass + { + void Test({{memberType}} member) + { + _ = Attribute.IsDefined(member, typeof(ObsoleteAttribute), true); + } + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task ExtensionGetCustomAttributes_Property_UsesAttributeIsDefined() + { + var test = CreateTest(); + test.TestCode = """ + using System; + using System.Linq; + using System.Reflection; + + class TestClass + { + void Test(PropertyInfo member) + { + _ = {|MA0179:member.GetCustomAttributes(typeof(ObsoleteAttribute)).Any()|}; + } + } + """; + test.FixedCode = """ + using System; + using System.Linq; + using System.Reflection; + + class TestClass + { + void Test(PropertyInfo member) + { + _ = Attribute.IsDefined(member, typeof(ObsoleteAttribute)); } } """;