Roslyn-analyzers: CA1062 should no longer suggest to use Code Contracts.

[kmgallahan]

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.8 (Latest)

Diagnostic ID

Example: CA1062

Suggestion

The description currently includes:

If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.

The Code Contract project is no longer being developed, and this rule should not recommend that it be used.

[julealgon]

Isn't stuff like Contract.Assume(x != null) part of the standard framework now though?

I understand the assembly rewriter and realtime analyzer are completely deprecated, but these libraries that are part of the framework could still be seen as "Code Contracts", couldn't they?

[kmgallahan]

Without runtime checking, suggesting to -

• throw an ArgumentNullException
• add a Code Contract (contract from System.Diagnostics.Contracts, not the named Code Contracts project)

do not have the same result, which is to fix violations as mentioned in the CA1062 docs:

To fix a violation of this rule, validate each reference argument against null.

If the rule read "CA1062: Suggest to callers not to pass a null argument" then it would be fine.

The docs also don't mention Code Contracts at all.

[Evangelink]

@kmgallahan Just to confirm I got all of your suggestions right. You'd like to have the doc website updated to also mention System.Diagnostics.Contracts maybe also include an example and then you'd like the message and description of the rule to be updated.

@mavasani Are you fine with these changes?

[kmgallahan]

@Evangelink @mavasani There are 3 problems:

1. The rule states: CA1062: Validate arguments of public methods
2. The Code Contracts project is dead and doesn't work with VS 2017/2019
3. System.Diagnostics.Contracts requires using the binary rewriter of the Code Contracts project (as mentioned here) to enforce run-time contracts

In my opinion there should be zero suggestions to use a dead project that doesn't work with VS 2017/2019, therefore the rule as it appears when broken in VS should not read:

If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.

Further, suggesting to use System.Diagnostics.Contracts doesn't seem appropriate either, as it doesn't do anything during run-time.

In my opinion either one of these needs to be done:

A. If the rule documentation continues to mention System.Diagnostics.Contracts, then it needs to be "softened" to read more along the lines of: "CA1062: Suggest to callers not to pass a null argument" (since the contracts don't do anything during run-time)
B. The rule needs to be "hardened" to apply to run-time enforcement by removing all references to the Code Contracts project and System.Diagnostics.Contracts

Bottom line: if the goal is to enforce run-time null checking, then mentioning contracts in any form is not helpful.

[chriskuech]

+1, I don't think System.Diagnostics.Contracts work to resolve CA1062 (I'm using VS2019), but there isn't documentation for me to know whether this is an analyzer bug or user error. Documentation should be updated to explicitly mention if Contracts can be used, as well as if/how custom Contracts can be written.