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

Question: Why no code analyzers (or linters) for the code base? #1098

Closed
mrlacey opened this issue Jul 25, 2019 · 3 comments
Closed

Question: Why no code analyzers (or linters) for the code base? #1098

mrlacey opened this issue Jul 25, 2019 · 3 comments

Comments

@mrlacey
Copy link
Contributor

mrlacey commented Jul 25, 2019

Based on the number of comments on this PR that are about basic style and code formatting, I was surprised to see that there are no analyzers (or equivalent) set up to automatically check for issues like white-space use or naming conventions.

Is there a reason these can't be added?

There would be two immediate benefits:

  1. Code would be formatted consistently which helps with reading, understanding, and maintenance.
  2. It would mean code reviews aren't distracted by trivial issues and can focus on the code. This can reduce the number of code comments and stop valuable issues becoming lost among multiple discussions about blank lines.

The down-side of adding these automatic formatting checks may be that existing code doesn't meet any set of rules and so turning it on would flag a large number of issues.
If that's the case the rules could be turned on (or analyzers added) when a project is otherwise being changed (or added) so not everything is done at once.
Alternatively, formatting inconsistencies are normally easy to fix and so it may only take a few hours (based on my past experiences with other large projects) to address what's found as tooling can normally automatically fix most of them and so it's just exceptions that need manual intervention or review.
It's not the most exciting work to do but it brings value over time.
Existing code coverage should make this a low risk change.

@msft-github-bot msft-github-bot added this to Needs triage in Controls Triage Jul 25, 2019
@shaggygi
Copy link

shaggygi commented Jul 25, 2019

I guess the editorconfig could be improved or FxCopAnalyzers added maybe.

@mrlacey
Copy link
Contributor Author

mrlacey commented Jul 25, 2019

I was thinking StyleCop.Analyzers for the C# code. I'm not sure what's best for C++ though.

@jevansaks
Copy link
Member

I'm not sure what's best for C++ though.

@ranjeshj is working on getting clang-tidy and clang-format enabled to run on the codebase and I think that should help for C++. MSVC just doesn't have a good linter as far as we've seen.

For C# there's the new editorconfig stuff that we have set up and should be taking effect in C# files. If there's more stylistic stuff for C# that needs to be added there we can do it. And then if something like StyleCop would help go even further we can run that too. But I haven't seen as much nit-picking in C# so i think the editorconfig mostly is doing its job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

3 participants