Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA1062 raised if Contract.Requires uses TException #3030

Closed
azegallo opened this issue Nov 9, 2019 · 5 comments
Closed

CA1062 raised if Contract.Requires uses TException #3030

azegallo opened this issue Nov 9, 2019 · 5 comments
Labels
False_Positive A diagnostic is reported for non-problematic case Needs-Review Resolution-Duplicate
Milestone

Comments

@azegallo
Copy link

azegallo commented Nov 9, 2019

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.7 (Latest)

Diagnostic ID

CA1062

Repro steps

  1. Inside a method use Contract.Requires<ArgumentNullException>(param != null)

Expected behavior

Nothing.

Actual behavior

CA1062 raised

CA1062 not raised if the TException is not used.

@Evangelink
Copy link
Member

@mavasani My understanding is that CodeContracts is deprecated so I am wondering whether we should add this exception.

@Evangelink
Copy link
Member

Is there something we want to do on this rule? I see it is disabled and meant to be replaced by a compiler warning.

Do we want to deprecate this rule and remove it on the release/6.0.1xx-preview1 branch?

cc @mavasani

@Evangelink Evangelink added False_Positive A diagnostic is reported for non-problematic case Needs-Review labels Dec 18, 2020
@mavasani
Copy link
Contributor

Do we want to deprecate this rule and remove it on the release/6.0.1xx-preview1 branch?

I do not think we want to deprecate CA1062. I am also not sure if it is planned to be replaced with a compiler warning. It is disabled by default as it uses dataflow analysis, and hence can be noisy + expensive to be enabled by default.

I think we certainly want to update CA1062 messaging to remove the words that recommend use of Code contracts.

@Evangelink
Copy link
Member

I do not think we want to deprecate CA1062. I am also not sure if it is planned to be replaced with a compiler warning. It is disabled by default as it uses dataflow analysis, and hence can be noisy + expensive to be enabled by default.

My comment was based on the comment left next to the Disabled state of the rule (see
https://github.com/dotnet/roslyn-analyzers/blob/master/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/ValidateArgumentsOfPublicMethods.cs#L27)

I think we certainly want to update CA1062 messaging to remove the words that recommend use of Code contracts.

Do we want to simply remove this mention or change it to recommend System.Diagnostics.Contracts.Contract?

@mavasani mavasani added this to the Unknown milestone May 6, 2021
@Youssef1313
Copy link
Member

I think we certainly want to update CA1062 messaging to remove the words that recommend use of Code contracts.

Duplicate of #3134

@Youssef1313 Youssef1313 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False_Positive A diagnostic is reported for non-problematic case Needs-Review Resolution-Duplicate
Projects
None yet
Development

No branches or pull requests

4 participants