Skip to content

Commit

Permalink
Merge pull request #44 from ulrichb/CanBeNullOnMethodBug
Browse files Browse the repository at this point in the history 
Fixed wrong detection of CanBeNull attribute for method results
  • Loading branch information
@distantcam
distantcam committed Mar 21, 2015
2 parents c0a9181 + 2816fb8 commit 9411f39
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 68 deletions.
1 change: 1 addition & 0 deletions AssemblyToProcess/AssemblyToProcess.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<Reference Include="System.Xml" />
</ItemGroup>
<ItemGroup>
<Compile Include="ReSharperAnnotations.cs" />
<Compile Include="ClassWithBadAttributes.cs">
<SubType>Code</SubType>
</Compile>
Expand Down
8 changes: 8 additions & 0 deletions AssemblyToProcess/ReSharperAnnotations.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System;

[AttributeUsage (
AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Delegate,
AllowMultiple = false, Inherited = true)]
public sealed class CanBeNullAttribute : Attribute
{
}
6 changes: 6 additions & 0 deletions AssemblyToProcess/SimpleClass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ public string MethodAllowsNullReturnValue()
return null;
}

[CanBeNull]
public string MethodWithCanBeNullResult()
{
return null;
}

public void MethodWithOutValue(out string nonNullOutArg)
{
nonNullOutArg = null;
Expand Down
14 changes: 12 additions & 2 deletions Fody/CecilExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

public static class CecilExtensions
{
private const string AllowNullAttributeTypeName = "AllowNullAttribute";
private const string CanBeNullAttributeTypeName = "CanBeNullAttribute";

public static bool HasInterface(this TypeDefinition type, string interfaceFullName)
{
return (type.Interfaces.Any(i => i.FullName.Equals(interfaceFullName))
Expand Down Expand Up @@ -46,14 +49,21 @@ public static ParameterDefinition GetPropertySetterValueParameter(this MethodDef

public static bool AllowsNull(this ICustomAttributeProvider value)
{
return value.CustomAttributes.Any(a => a.AttributeType.Name == "AllowNullAttribute" || a.AttributeType.Name == "CanBeNullAttribute");
return value.CustomAttributes.Any(a => a.AttributeType.Name == AllowNullAttributeTypeName || a.AttributeType.Name == CanBeNullAttributeTypeName);
}

public static bool AllowsNullReturnValue (this MethodDefinition methodDefinition)
{
return methodDefinition.MethodReturnType.CustomAttributes.Any(a => a.AttributeType.Name == AllowNullAttributeTypeName) ||
// ReSharper uses a *method* attribute for CanBeNull for the return value
methodDefinition.CustomAttributes.Any(a => a.AttributeType.Name == CanBeNullAttributeTypeName);
}

public static bool ContainsAllowNullAttribute(this ICustomAttributeProvider definition)
{
var customAttributes = definition.CustomAttributes;

return customAttributes.Any(x => x.AttributeType.Name == "AllowNullAttribute");
return customAttributes.Any(x => x.AttributeType.Name == AllowNullAttributeTypeName);
}

public static void RemoveAllNullGuardAttributes(this ICustomAttributeProvider definition)
Expand Down
4 changes: 2 additions & 2 deletions Fody/MethodProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private void InnerProcess(MethodDefinition method)
returnType = genericReturnType.GenericArguments[0];

if (localValidationFlags.HasFlag(ValidationFlags.ReturnValues) &&
!method.MethodReturnType.AllowsNull() &&
!method.AllowsNullReturnValue() &&
returnType.IsRefType() &&
returnType.FullName != typeof(void).FullName)
{
Expand Down Expand Up @@ -146,7 +146,7 @@ private void InjectMethodReturnGuard(ValidationFlags localValidationFlags, Metho
foreach (var ret in returnPoints)
{
if (localValidationFlags.HasFlag(ValidationFlags.ReturnValues) &&
!method.MethodReturnType.AllowsNull() &&
!method.AllowsNullReturnValue() &&
method.ReturnType.IsRefType() &&
method.ReturnType.FullName != typeof(void).FullName &&
!method.IsGetter)
Expand Down
74 changes: 42 additions & 32 deletions Tests/approvals/ApprovedTests.SimpleClass.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,22 @@
IL_0000: ldnull
IL_0001: ret
} // end of method SimpleClass::MethodAllowsNullReturnValue
.method public hidebysig instance string
MethodWithCanBeNullResult() cil managed
{
.custom instance void CanBeNullAttribute::.ctor() = ( 01 00 00 00 )
// Code size 2 (0x2)
.maxstack 1
.line 63,63 : 9,21 ''
IL_0000: ldnull
IL_0001: ret
} // end of method SimpleClass::MethodWithCanBeNullResult
.method public hidebysig instance void
MethodWithOutValue([out] string& nonNullOutArg) cil managed
{
// Code size 37 (0x25)
.maxstack 2
.line 62,62 : 9,30 ''
.line 68,68 : 9,30 ''
IL_0000: ldarg.1
IL_0001: ldnull
IL_0002: stind.ref
Expand All @@ -452,42 +462,42 @@
IL_0019: ldstr "[NullGuard] Out parameter 'nonNullOutArg' is null."
IL_001e: newobj instance void [mscorlib]System.InvalidOperationException::.ctor(string)
IL_0023: throw
.line 63,63 : 5,6 ''
.line 69,69 : 5,6 ''
IL_0024: ret
} // end of method SimpleClass::MethodWithOutValue
.method public hidebysig instance void
MethodWithAllowedNullOutValue([out] string& nonNullOutArg) cil managed
{
// Code size 4 (0x4)
.maxstack 2
.line 67,67 : 9,30 ''
.line 73,73 : 9,30 ''
IL_0000: ldarg.1
IL_0001: ldnull
IL_0002: stind.ref
.line 68,68 : 5,6 ''
.line 74,74 : 5,6 ''
IL_0003: ret
} // end of method SimpleClass::MethodWithAllowedNullOutValue
.method public hidebysig instance void
PublicWrapperOfPrivateMethod() cil managed
{
// Code size 8 (0x8)
.maxstack 2
.line 72,72 : 9,33 ''
.line 78,78 : 9,33 ''
IL_0000: ldarg.0
IL_0001: ldnull
IL_0002: call instance void SimpleClass::SomePrivateMethod(string)
.line 73,73 : 5,6 ''
.line 79,79 : 5,6 ''
IL_0007: ret
} // end of method SimpleClass::PublicWrapperOfPrivateMethod
.method private hidebysig instance void
SomePrivateMethod(string x) cil managed
{
// Code size 7 (0x7)
.maxstack 8
.line 77,77 : 9,30 ''
.line 83,83 : 9,30 ''
IL_0000: ldarg.1
IL_0001: call void [mscorlib]System.Console::WriteLine(string)
.line 78,78 : 5,6 ''
.line 84,84 : 5,6 ''
IL_0006: ret
} // end of method SimpleClass::SomePrivateMethod
.method public hidebysig instance void
Expand Down Expand Up @@ -532,7 +542,7 @@
IL_0046: newobj instance void [mscorlib]System.ArgumentNullException::.ctor(string,
string)
IL_004b: throw
.line 82,82 : 5,6 ''
.line 88,88 : 5,6 ''
IL_004c: ret
} // end of method SimpleClass::MethodWithTwoRefs
.method public hidebysig instance void
Expand All @@ -541,11 +551,11 @@
{
// Code size 73 (0x49)
.maxstack 2
.line 86,86 : 9,22 ''
.line 92,92 : 9,22 ''
IL_0000: ldarg.1
IL_0001: ldnull
IL_0002: stind.ref
.line 87,87 : 9,23 ''
.line 93,93 : 9,23 ''
IL_0003: ldarg.2
IL_0004: ldnull
IL_0005: stind.ref
Expand Down Expand Up @@ -581,7 +591,7 @@
IL_003d: ldstr "[NullGuard] Out parameter 'second' is null."
IL_0042: newobj instance void [mscorlib]System.InvalidOperationException::.ctor(string)
IL_0047: throw
.line 88,88 : 5,6 ''
.line 94,94 : 5,6 ''
IL_0048: ret
} // end of method SimpleClass::MethodWithTwoOuts
.method public hidebysig instance void
Expand All @@ -590,7 +600,7 @@
.param [1] = nullref
// Code size 1 (0x1)
.maxstack 0
.line 92,92 : 5,6 ''
.line 98,98 : 5,6 ''
IL_0000: ret
} // end of method SimpleClass::MethodWithOptionalParameter
.method public hidebysig instance void
Expand All @@ -615,7 +625,7 @@
IL_001e: newobj instance void [mscorlib]System.ArgumentNullException::.ctor(string,
string)
IL_0023: throw
.line 96,96 : 5,6 ''
.line 102,102 : 5,6 ''
IL_0024: ret
} // end of method SimpleClass::MethodWithOptionalParameterWithNonNullDefaultValue
.method public hidebysig instance void
Expand All @@ -624,15 +634,15 @@
.param [1] = "default"
// Code size 1 (0x1)
.maxstack 0
.line 100,100 : 5,6 ''
.line 106,106 : 5,6 ''
IL_0000: ret
} // end of method SimpleClass::MethodWithOptionalParameterWithNonNullDefaultValueButAllowNullAttribute
.method public hidebysig instance void
MethodWithGenericOut<T>([out] !!T& item) cil managed
{
// Code size 59 (0x3b)
.maxstack 2
.line 104,104 : 9,27 ''
.line 110,110 : 9,27 ''
IL_0000: ldarg.1
IL_0001: initobj !!T
.line 16707566,16707566 : 0,0 ''
Expand All @@ -653,15 +663,15 @@
IL_002f: ldstr "[NullGuard] Out parameter 'item' is null."
IL_0034: newobj instance void [mscorlib]System.InvalidOperationException::.ctor(string)
IL_0039: throw
.line 105,105 : 5,6 ''
.line 111,111 : 5,6 ''
IL_003a: ret
} // end of method SimpleClass::MethodWithGenericOut
.method public hidebysig instance !!T MethodWithGenericReturn<T>(bool returnNull) cil managed
{
// Code size 103 (0x67)
.maxstack 3
.locals init ([0] !!T CS$0$0000)
.line 109,109 : 9,72 ''
.line 115,115 : 9,72 ''
IL_0000: ldarg.1
IL_0001: brtrue.s IL_0033
IL_0003: call !!0 [mscorlib]System.Activator::CreateInstance<!!0>()
Expand Down Expand Up @@ -714,11 +724,11 @@
{
// Code size 70 (0x46)
.maxstack 3
.line 114,114 : 9,23 ''
.line 120,120 : 9,23 ''
IL_0000: ldarg.1
IL_0001: ldnull
IL_0002: stind.ref
.line 115,115 : 9,21 ''
.line 121,121 : 9,21 ''
IL_0003: ldnull
.line 16707566,16707566 : 0,0 ''
IL_0004: ldarg.1
Expand Down Expand Up @@ -776,60 +786,60 @@
IL_001e: newobj instance void [mscorlib]System.ArgumentNullException::.ctor(string,
string)
IL_0023: throw
.line 120,120 : 9,37 ''
.line 126,126 : 9,37 ''
IL_0024: ldarg.1
IL_0025: call bool [mscorlib]System.String::IsNullOrEmpty(string)
IL_002a: brfalse.s IL_003c
.line 121,121 : 13,69 ''
.line 127,127 : 13,69 ''
IL_002c: ldstr "x is null or empty."
IL_0031: ldstr "x"
IL_0036: newobj instance void [mscorlib]System.ArgumentException::.ctor(string,
string)
IL_003b: throw
.line 123,123 : 9,30 ''
.line 129,129 : 9,30 ''
IL_003c: ldarg.1
IL_003d: call void [mscorlib]System.Console::WriteLine(string)
.line 124,124 : 5,6 ''
.line 130,130 : 5,6 ''
IL_0042: ret
} // end of method SimpleClass::MethodWithExistingArgumentGuard
.method public hidebysig instance void
MethodWithExistingArgumentNullGuard(string x) cil managed
{
// Code size 26 (0x1a)
.maxstack 1
.line 128,128 : 9,37 ''
.line 134,134 : 9,37 ''
IL_0000: ldarg.1
IL_0001: call bool [mscorlib]System.String::IsNullOrEmpty(string)
IL_0006: brfalse.s IL_0013
.line 129,129 : 13,50 ''
.line 135,135 : 13,50 ''
IL_0008: ldstr "x"
IL_000d: newobj instance void [mscorlib]System.ArgumentNullException::.ctor(string)
IL_0012: throw
.line 131,131 : 9,30 ''
.line 137,137 : 9,30 ''
IL_0013: ldarg.1
IL_0014: call void [mscorlib]System.Console::WriteLine(string)
.line 132,132 : 5,6 ''
.line 138,138 : 5,6 ''
IL_0019: ret
} // end of method SimpleClass::MethodWithExistingArgumentNullGuard
.method public hidebysig instance void
MethodWithExistingArgumentNullGuardWithMessage(string x) cil managed
{
// Code size 31 (0x1f)
.maxstack 2
.line 136,136 : 9,37 ''
.line 142,142 : 9,37 ''
IL_0000: ldarg.1
IL_0001: call bool [mscorlib]System.String::IsNullOrEmpty(string)
IL_0006: brfalse.s IL_0018
.line 137,137 : 13,73 ''
.line 143,143 : 13,73 ''
IL_0008: ldstr "x"
IL_000d: ldstr "x is null or empty."
IL_0012: newobj instance void [mscorlib]System.ArgumentNullException::.ctor(string,
string)
IL_0017: throw
.line 139,139 : 9,30 ''
.line 145,145 : 9,30 ''
IL_0018: ldarg.1
IL_0019: call void [mscorlib]System.Console::WriteLine(string)
.line 140,140 : 5,6 ''
.line 146,146 : 5,6 ''
IL_001e: ret
} // end of method SimpleClass::MethodWithExistingArgumentNullGuardWithMessage
.property instance string NonNullProperty()
Expand Down
Loading

0 comments on commit 9411f39

Please sign in to comment.