Skip to content

[Java.Interop] Replace Gendarme with Roslyn FxCop Analyzers. #523

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

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Nov 20, 2019

Today we use Gendarme for static code analysis on Java.Interop.dll. Unfortunately Gendarme is dated and additionally relies on .mdb files for analysis. This requires compiling with mcs which is deprecated and does not support modern C# features like NRT.

This PR replaces Gendarme with Roslyn FxCop Analyzers, which get around the above issues, as well as provide a few additional benefits:

  • They run on every compile so they will be seen as part of the normal development workflow instead of a separate step.
  • They do source code analysis instead of binary analysis which allows them to catch more issues.

This PR aims for parity with Gendarme so it configures every analyzer equivalent to Gendarme as severity error (by default these only cause warnings).

If we want to change the severity of a rule, that should be changed in the .editorconfig file.

If we want to ignore an instance of a violation, that goes in the suppression file. VS has a tooltip of "Suppress this error in suppression file" to do it for you, other editors may need to do it by hand, but the syntax is pretty similar to the Gendarme syntax.

@@ -0,0 +1,149 @@
# This files sets all rules from Gendarme as Code Analysis errors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the "nature" of this file? What causes errors to be generated? Any build, meaning we'd see future failures in PR builds? Or just builds within "some supported IDE"?

When an error is found, how is it reported?

What's the workflow?

Copy link
Contributor Author

@jpobst jpobst Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are understood by msbuild and are passed to csc.exe which actually does the source code analysis at compile time (this is source code analysis, not binary analysis like Gendarme or FxCop):

csc.exe ... /analyzerconfig:C:\code\xamarin-android\external\Java.Interop\.editorconfig

They are reported by the compiler just like any other compiler generated warning or error, so this will now fail builds from the msbuild command line, in IDEs, and in CI/PR builds without any additional steps.

Example:
https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3261455&view=results

If we want to ignore an instance, that goes in the suppression file. VS has a tooltip of "Suppress this error in suppression file" to do it for you, other editors may need to do it by hand, but the syntax is pretty similar to the Gendarme syntax.

@jpobst jpobst force-pushed the remove-gendarme branch 2 times, most recently from 331cd74 to 86a09ab Compare November 21, 2019 15:46
@jonpryor jonpryor merged commit 5fa8c18 into master Nov 22, 2019
@jonpryor jonpryor deleted the remove-gendarme branch November 22, 2019 00:15
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants