Tmpe: Linter & Code Quality

[aubergine10]

stub

This is in reference to #211

I have some stuff in mind, will add it later.

[aubergine10]

These three:

• StyleCop - focuses mainly on the layout and formatting of your code.
• Codemaid - focuses on organising your code (like order by member type, removing unused using declarations, etc)
• Roslynator - offers lots of code fixes and suggestions, in addition to those native to VS2017.
• Speaking of which, we should also customise the settings of the M$ stuff that's bundled with VS2017. From memory most of the rules are turned off by default.

From vague memory dymanoid used StyleCop in the Real Time mod to ensure consistent code comments, etc. This would be particularly useful for the planned TM:PE API (couldn't find issue tracking that) as we could use the proper code comments to auto-generate API docs to github.io, not to mention just making the source far more maintainable to anyone who hasn't been working on it the past 3+ years ;)

There is also a non-open source, but free, U2U Consult Performance Code Analyzers which might be worth investigating. I don't know if it's usable with the version of .Net Framework that C:SL limits us to?

All of the above work on VS2017 and would thus be suitable for open source contributions.

I don't use the JetBrains IDE (Riser?) so can't comment on that. @krzychu124 might have some ideas in that respect.

[aubergine10]

@VictorPhilipp We should ideally make a decision (well, really a decision between you and krzychu) on these things prior to any major code refactorings (such as move to harmony)?

[VictorPhilipp]

There is also SonarLint which, according to https://stackoverflow.com/questions/748631/lint-for-c-sharp also uses Roslyn under the hood. It can installed directly from VS marketplace: https://marketplace.visualstudio.com/items?itemName=SonarSource.SonarLintforVisualStudio2017
We should also consider to implement a build pipeline (e.g. Travis CI) that runs for each pull request and merge. The linter and (later) tests will have to run successfully before a PR can be accepted.

An important aspect is integratability (into build pipeline) and ease of use. I have just downloaded all four tools and try them for a while.

[VictorPhilipp]

Links to marketplace:

• StyleCop: https://marketplace.visualstudio.com/items?itemName=ChrisDahlberg.StyleCop
• SonarLint: https://marketplace.visualstudio.com/items?itemName=SonarSource.SonarLintforVisualStudio2017
• Roslynator 2017: https://marketplace.visualstudio.com/items?itemName=josefpihrt.Roslynator2017
• CodeMaid: https://marketplace.visualstudio.com/items?itemName=SteveCadwallader.CodeMaid

[VictorPhilipp]

@krzychu124 Maybe you could set up Travis CI for this repo? https://travis-ci.org/krzychu124/Cities-Skylines-Traffic-Manager-President-Edition

[VictorPhilipp]

I checked StyleCop:

• 👍 It works
  

• 👍 It is quite easy to configure (see screenshot above: "StyleCop Settings")

• 👎 You cannot set severity by issue type, only for all issues in general

• 👎 Build integration does not work. The project is broken after enabling and I had to manually repair the .csproj file. It is missing some StyleCop project
  
  

• 👎 You cannot group findings because StyleCop does not fill the respective fields in the error log (e.g. "Code" and "Tool")
  

Conclusion: 👎

[VictorPhilipp]

I checked SonarLint:

• 👎 Does not support .NET 3.5
• 👎 "As of release 3.0, analyzing projects built with MSBuild 12 is no longer supported." (https://docs.sonarqube.org/display/SCAN/Compatibility) - we could build with MSBuild 14 for 12

[VictorPhilipp]

I checked Roslynator:

• 👍 Seamless integration in VS
  
• 👍 Supports setting the severity for every issue type
  
• 👎 Does not work. No issues are shown in the error list, although I have set every severity to "Error". I followed the setup instructions from https://github.com/JosefPihrt/Roslynator/blob/master/docs/HowToConfigureAnalyzers.md

Maybe the problem is caused by the old csproj format we use (it's from VS 2015)? I tried to update it to the new format (https://natemcmaster.com/blog/2017/03/09/vs2015-to-vs2017-upgrade/) but that did not make any difference.

I found a GitHub issue (https://github.com/JosefPihrt/Roslynator/issues/198) but setting the dropdown inside the error list to "Build + IntelliSense" did nothing.

It's sad. Roslynator looks promising but I am unable to get it running.

[VictorPhilipp]

Regarding CodeMaid: This tool does not provide any static code analysis rules

[VictorPhilipp]

@krzychu124 was magically able to create a build pipeline which facilitates StyleCop. The VS project file was also fixed.

Next steps:

• Select which rules we want to use
• Fix all Stylecop reports
• Handle detected issues as errors

[dymanoid]

Do not use that StyleCop version, it's obsolete.

Use:
StyleCop.Analyzers
FxCop Analyzers
Roslynator Analyzers

They are implemented as Roslyn analyzers and can be easily integrated into any build environment, the severity of each particular issue can be configured (info/warning/error/ignore), etc etc

See the Real Time repo for examples.

[krzychu124]

Yeah, I know, that's why we are using StyleCop.Analyzers. Great extension.

Thanks for links to other code analyzers :wink:

[aubergine10]

Will we add any more of the analysers listed above or can this issue be closed?

[aubergine10]

Some analysers were added to solution around v10.21 so closing this.

Also, @kvakvs added brief style guide to wiki (also linked from contributing guide) here: https://github.com/CitiesSkylinesMods/TMPE/wiki/Code-style-and-naming