[WIP] Add FxCop to the build #449
Conversation
|
Good initiative. I basically did this and turned it off again because of the spamming, but we should set up a ignore list, including CA1303 (until we get localization). |
|
Is it ok to use the editorconfig approach to control the analyzer settings? |
|
I take the way that the number of warnings in the CI build didn't go down after updating editorconfig to mean the answer to my previous question is 'no' :-( In any case, I sent #450 to fix some of the produced warnings. |
Numpsy
force-pushed
the
rw/fxcop
branch
2 times, most recently
from
1fa73b4
to
e882412
Compare
|
@piksel I left this using the 2.9 version of FxCop as the release notes for the v3 version say it requires VS 2019 and I thought that may not be wanted, but maybe that's not an issue now the tests are targetted at Core 3.1 and need that anyway? |
|
Yeah, I don't see any problems with requiring VS 2019 for FxCop. If a need for this to change emerges then we could just disable or change this later, but for now I don't see any reason to use anything earlier than 2019. |
|
hmm, I don't know how this change could cause that one test failure. |
|
No, that's probably due to something unexpected in the CI. Especially since it worked on the same platform, different config, and same config, different platform(s). |
Numpsy
force-pushed
the
rw/fxcop
branch
2 times, most recently
from
05966d2
to
fab58ac
Compare
Codecov Report
@@ Coverage Diff @@
## master #449 +/- ##
==========================================
- Coverage 73.35% 73.23% -0.12%
==========================================
Files 68 68
Lines 8721 8721
==========================================
- Hits 6397 6387 -10
- Misses 2324 2334 +10
Continue to review full report at Codecov.
|
|
I'm thinking about how to best get this merged. The best thing to do is probably to just create a bulk commit/PR with most of the fixes done. This will render other PRs in need of rebasing, but at least it would be more of an one-time event. |
|
I think a pretty large proportion of the remaining warnings were about checking public params for being null, and at the time I might have thought that #501 would deal with some of those and not looked in much detail. |
|
Theres also the question at this point on if it should use NetAnalyzers instead of FxCop (I can see about a PR that fixes many warnings at once in either case though)) |
Add FxCop to the build to see what issues it finds.
Marked as WIP because it creates over a thousand warnings in the build.
Some of them look worth looking at (e.g. warnings about not disposing of things before they go out of scope), but a lot of them are things like passing string literals to exceptions rather than using resources, which are possibly not really useful. (so could be turned off)
(Note: #445 is something that was found by the analyzer)
I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.