Making Static Code Analysis and Code Contracts work together, or the CA1062 problem
By terje
UPDATED: Feb 16th 2012: Workaround #1 and #2 only works when run-time checking is not enabled. Added workaround #2B which seems to work. Thanks to David S, Michael S and ZbynekZ for pointing this out, and sorry for not responding faster to this. Also added comments and overviews over what happens for different settings.
There have been several reports on problems with the Static Code Analysis (SCA) not adhering to the statements of the Code Contract tools. See these links: connect, forum1, forum2.
The problem can be shown in the following code snippet:
Code Problem:
public int FindLength1(string something) { Contract.Requires(something != null); return something.Length; }
Running a Code Analysis gives a CA1062 warning (note: use a ruleset which enables this warning) saying:
Warning 5 CA1062 : Microsoft.Design : In externally visible method 'ParseNumber.FindLength1(string)', validate parameter 'something' before using it. C:TFSDCodesamplesCodeContractAndCA1062ProblemCodeContractAndCA1062ProblemParseNumber.cs 133 CodeContractAndCA1062Problem
It complains that the parameter “something” can be null, but the Contract statement preceding this use says it can’t be null.
The problem is that the SCA doesn’t recognize the Contracts. As can be seen from the links above, this issue has proven to be a bit hard to fix. So, we have to resort to workarounds to get this to work – until the bright guys have found a way to make the SCA work with contracts.
Workarounds are never as pretty as a proper solution can be, so you should choose the one which most applies to you, also taking your scenario into account.
There is a problem with the binary rewriter and the code analysis. The code analysis is done after binary rewriting, and that means some of these workarounds will not work when you do run-time checking. All of the workarounds work for static code checking, but if you need to use runtime checking, you must enable assembly mode Custom Parameter Validation [CPV] . See page 20 in the user manual for information on when to use Standard Contract Require mode and Custom Parameter Validation mode.
The following two statements were assumed, but it doesn’t seem to hold anymore, so the situation may be better than expected. It awaits confirmation though:
If you enable run time checking with CPV, workaround 2B works.
If you can’t accept to use CPV – because it prevents inheritance of contracts – your only option is to use workaround 3.
The table below shows the effect of the workarounds and the different assembly mode and check setting combinations. The red text shows what fails.
Workaround #1 and #2B gives this warning message when run in SCR:
Method 'TestCA1062.A.FindLength1(System.String)' has custom parameter validation but assembly mode is not set to support this. It will be treated as Requires<E>.
The different workarounds also give different exceptions when using the runtime checking:
We want to have the ContractException, but as can be seen, the #2B gives its inner exception, the ArgumentNullException.
So, the conclusion from this is that none of the workarounds are all perfect, but #2B gives the best compromise, as you get rid of the static code warning CA1062, and it works with runtime checking.
Workaround #1
This workaround is useful where you have legacy code already in place, or if you don’t mind using old fashioned code checks.
public int FindLength2(string something) { if (something==null) throw new ArgumentNullException("something"); Contract.EndContractBlock(); return something.Length; }
There are four things to note about this:
- The use of the normal “if (something==null)” check is recognized by the SCA, thus eliminating the CA1062 warning.
- The Contract.EndContractBlock statement turns the lines ahead into contracts, so the lines above “is equal to” a Contract.Requires(something!=null) statement , but mind, it doesn’t give identical run time exceptions, the “equality” is only for static contract checking (Thanks for pointing this out Michael)
- Inheritance of contracts
- These contracts should not have been inheritable (as stated in #2 under here), but from testing now it seems they do. Awaiting confirmation on this one.
- This workaround is meant to run in Custom Parameter Validation (CPV) assembly mode. If you run in Standard Contract Require (SCR) mode, it reverts to a standard Requires<E> statement, and you get a warning about this. This warning should have been informational, as there is no need to do anything about it.
<li>(These contracts are not inherited to subtypes as a Contract.Requires is. If you make use of inheritance trees with contracts, this will force you to implement these checks manually throughout the inheritance tree, which might be a problem.) </li>
Workaround #2
This workaround requires a few other things to be put into place, and might be regarded as a bit strange code-wise, but does the job quite nicely. It also uses some extra features in the Code Contract suite. Thanks to Cameron Skinner and Mike Barnett for pointing me to this solution. This workaround only works when static contract checking is used without having runtime checking enabled.
public int FindLength3(string something) { CheckNotNull(something); return something.Length; } [ContractAbbreviator] internal void CheckNotNull<T>([ValidatedNotNull]T whatever) { Contract.Requires(whatever != null); }
There are a few things to note about this too:
- The contract in the FindLength method is replaced with a special method, CheckNotNull. This might feel unusual, but solves the problem rather elegant.
- The CheckNotNull method is a special method, it must do the following:
- Be decorated by the ContractAbbreviator attribute, which is used to tell that this method contains one or more contracts. This will satisfy the Contract, and the contracts within will appear as contracts for the FindLength method.
<li>Have its method parameter decorated with an attribute named ValidatedNotNull. This attribute name is picked up by the SCA (FxCop) and will silence the CA1062 warning. </li> </ol>
The two attributes here must be added to the program. The ContractAbbreviator is not part of the standard Contract assembly, but is available in source form from the folder %ProgramFiles%MicrosoftContractsLanguages....
or you can just add it yourself, it looks like this:
namespace System.Diagnostics.Contracts
/// <summary> /// Enables writing abbreviations for contracts that get copied to other methods /// </summary> [AttributeUsage(AttributeTargets.Method, AllowMultiple=false)] [Conditional("CONTRACTS_FULL")] internal sealed class ContractAbbreviatorAttribute : global::System.Attribute { }
The other attribute, ValidatedNotNull can be defined as follows:
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false)] internal sealed class ValidatedNotNullAttribute : global::System.Attribute { }
Namespace is not important for this attribute, place it wherever.
Workaround #2B
This is a variation of the pattern for #2, which works in all modes, although it is intended to work for the CPV only. It gives the annoying warning about wrong assembly mode, but otherwise works in all modes. Thanks again to Mike Barnett and his colleague Manuel Fahndrich for coming up with this alternative.
public int FindLength2B(string something) { CheckNotNullB(something); return something.Length; } [ContractArgumentValidator] internal void CheckNotNullB<T>([ValidatedNotNull]T whatever) { if (whatever == null) { throw new ArgumentNullException("whatever"); } Contract.EndContractBlock(); }
and it also requires you to define the attribute like this:
[AttributeUsageAttribute(AttributeTargets.Method, AllowMultiple = false)] [ConditionalAttribute("CONTRACTS_FULL")] public sealed class ContractArgumentValidatorAttribute : Attribute { }
Note that this attribute is defined in the upcoming .Net framework 4.5, so when you start using that one, you should delete your own definition.
Workaround #3
This is the simple way to do this, but it works too – suppress the warning. And, it is not as dumb as one could fear – because the Suppress attribute to be used here can be made rather narrow:
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "0")] public int FindLength7(string something) { Contract.Requires(something != null); return something.Length; }
The MessageId parameter states which of the parameters is to be suppressed in case of multiple parameters.
-----------------------
This workaround is more reactive than Workaround 1 and 2, which are more proactive. When would I choose what?
- Workaround 1: When I have legacy code, and just want to add contracts.
- Workaround 2: New code, including new methods on old code, where I want the code to be safe before I do more.
- Workaround 3: Legacy code, when adding SCA and contracts, and where no existing guards exist.