Address a targeted set of analyzer warnings by DaveTryon · Pull Request #901 · microsoft/sbom-tool (original) (raw)
Issue #340 provides a long list of analyzer warnings that were added to prevent a build break. The vast majority of the warnings on this list are extraordinarily pedantic, and some of them actually go against what I would consider best practices. I grabbed a few of the rules that seemed worthwhile and didn't represent a huge impact, and made the appropriate changes. At some point, we may want to officially ignore analyzer warnings that we actively don't want. This PR adds enforcement of the following analyzer warnings:
| ID/Link | Description | Comment |
|---|---|---|
| IDE0005 | Using directive is unnecessary | Cleaned up the using blocks that the compiler says it's not using. I had to tell the test code to generate XML comments to enable this--that seems odd but not-problematic |
| CA1040 | Avoid empty interfaces | We only had one, and it exists for a reason, so I suppressed the warning on that one |
| CA1508 | Avoid dead conditional code | Nobody wants dead code |
| SA1207 | Member attributes should follow the order | OK, this is pedantic but it only changed 1 line of code |
| SA1633 | File should have header | This was probably fixed by #347 but the flag was never enforced |
| SA1626 | Single-line comments should not use documentation style slashes | Another pedantic one, but Visual Studio was whining and the fix was trivial |
| SA1625 | Element documentation should not be copied and pasted | Another pedantic one, but Visual Studio was whining and the fix was trivial |
Note: This will probably best be reviewed on a per-commit basis. It will flag the banner about changing the API, but it can be ignored because we're not actually changing any of the API surfaces.