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

Code Analysis misses many issues .net framework CA catches #1948

Closed
Jm-Fox opened this issue Dec 26, 2018 · 8 comments
Closed

Code Analysis misses many issues .net framework CA catches #1948

Jm-Fox opened this issue Dec 26, 2018 · 8 comments
Assignees
Labels
Bug The product is not behaving according to its current intended design Urgency-Soon

Comments

@Jm-Fox
Copy link

Jm-Fox commented Dec 26, 2018

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.6.2

Diagnostic ID

Only in .net framework:

Only in .net core*:

*I do not mind new CA issues being introduced to .net core; in fact I support it. However, old CA issues being lost is not acceptable to my employer.

Repro steps

  1. Clone https://github.com/Jm-Fox/NetCoreCodeAnalysis
  2. Open CodeAnalysis.sln (I used VS 2017.9)
  3. In the solution explorer, right click on Solution 'CodeAnalysis' (2 projects) and select Analyze > Run Code Analysis on Solution
  4. Compare CA results for each project (they have identical code)

Expected behavior

Each CA warning should appear twice: once in the .net framework 4.6.1 project and once in the .net core project.

Actual behavior

Many CA warnings appear only in the .net framework 4.6.1 project.

Other notes

I don't have a proper repro for CA1062, but this is another one that appears to be missing in .net core that is fairly important.

I apologize if this is documented elsewhere or is expected behavior. In my research so far I have not been able to find anything that explains why CA for .net core isn't 1:1 with CA for .net framework.

These issues not showing up in CA for .net core is very important to both me and my employer. Not having automated checks like disposing of IDisposables along all paths can lead to bugs in production if not caught in code review, and this is not something we can reasonably ask our code reviewers to catch 100% of the time without CA assistance.
Also the repro I have given is just a sample of CA inconsistencies I found by accident. I have no reason to believe there aren't other CA issues that are missing in .net core, and given what I've seen I expect that I'm missing some.

Other questions

  1. Is the nuget package supposed to be a 1:1 replacement for CA in .net framework?
  2. If so, when can one expect these inconsistencies to be ironed out?
@sharwell
Copy link
Member

sharwell commented Jan 2, 2019

@sharwell
Copy link
Member

sharwell commented Jan 2, 2019

@mavasani I summarized the state of impacted rules above.

@Jm-Fox
Copy link
Author

Jm-Fox commented Jan 3, 2019

Thanks for the summary. Is there any action required from me, such as filing individual bugs?

@sharwell
Copy link
Member

sharwell commented Jan 3, 2019

@Jm-Fox from the steps above, it sounds like the reproducer case (run against a reference repository) would be enough to show the various differences you found, but it doesn't give a good way to know which ones are the most important to you. It might take us a while to get these differences itemized since the process for filing bugs like this is not automated. If you find specific cases that are especially relevant to your projects, it would be good to go ahead and file individual issues for those.

@mavasani
Copy link
Contributor

mavasani commented Jan 3, 2019

Thanks @sharwell @Jm-Fox for all the investigations/reports here. We are striving very hard to ensure compatibility between legacy FxCop and FxCop analyzers, especially for rules which are deemed as important to port based on the current dotnet framework guidelines. We are going to schedule testing cycles in the coming months to run both legacy FxCop and FxCop analyzers over real world code and identify differences and work towards fixing them. As @sharwell mentioned above, if you do find any differences that are super important to you OR block migration to FxCop analyzers, kindly file individual issues, with a note on its priority.

Tagging @vatsalyaagrawal @jinujoseph as well so we can cost/schedule this testing effort.

@Jm-Fox
Copy link
Author

Jm-Fox commented Jan 11, 2019

Thanks for the responses; I apologize for the delay. I have created individual git issues for the CA issues that are most important to me / my employer.

I will add that CA1031 is of moderate importance to my employer. Should I comment on the existing issue, or should I leave it be?

The remaining issues I / my employer are not worried about. While we'd like to see a one-for-one replacement of the FxCop nuget package for the old code analysis, the remaining issues are not reasons to avoid .net core.

@mavasani mavasani added Bug The product is not behaving according to its current intended design Urgency-Soon labels Feb 6, 2019
@mavasani mavasani self-assigned this Feb 6, 2019
@MelbourneDeveloper
Copy link

Yep, many of the fundamental issues are missing.

@mavasani
Copy link
Contributor

Closing this issue as the recommendation is to file separate issues for each rule OR comment on existing issues for a rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug The product is not behaving according to its current intended design Urgency-Soon
Projects
None yet
Development

No branches or pull requests

4 participants