Add lint check for C++ changes

[seanbudd]

Is your feature request related to a problem? Please describe.

Unlinted C++ code exists on the master branch of NVDA, and PRs to master can contain unlinted C++ code. Currently reviewers must manually check if a PR introduces unlinted C++ code.

Describe the solution you'd like

It would be nice to have a lint check similar to our flake8 python check that checks if a PR introduces unlinted C++ code.

It would also be good to lint our existing C++ code base, but this may run into problems as discussed on #12261

Potential tool to use: cpplint

Describe alternatives you've considered

Manual checking and linting, other tools

[feerrenrut]

I've been involved in the integration of cppcheck before, a static analysis tool.
We also have the option to run: /analyze via MSVC. see "Compiling NVDAHelper with Debugging Options" in the readme.

However, I agree it would be best to have something that runs as part of the build. Ideally it would address not only style, but also detect common errors via static analysis.

Some overviews of common tools:

• https://hownot2code.com/2017/06/29/review-of-contemporary-cc-static-code-analyzers/
• https://blog.kitware.com/static-checks-with-cmake-cdash-iwyu-clang-tidy-lwyu-cpplint-and-cppcheck/#:~:text=cpplint%20%3A%20a%20C%2B%2B%20style%20checker%20following%20Google%E2%80%99s,in%20the%20code%20%28i.e.%20have%20zero%20false%20positives%29.

[josephsl]

CC @LeonarddeR, I think this is something to be aware of in your C++/WinRT research. Thanks.

[feerrenrut]

Building nvda with scons source nvdaHelperDebugFlags=analyze will produce warnings for the code.
Many of these are highlighted in code when using Visual Studio (if paths and options are set reasonably well)

There are options to log these to an xml file. This file could then be used to add pragma directives to ignore the pre-existing warnings. Then we can set the build to run analyze for all PR builds and fail them if there are any new warnings. The in code pragmas will highlight the warnings for developers to watch out for problems / fix as we go.

[feerrenrut]

We might also want to consider what ruleset is used:
https://docs.microsoft.com/en-us/cpp/code-quality/using-the-cpp-core-guidelines-checkers?view=msvc-170

[LeonarddeR]

It would also be good to lint our existing C++ code base, but this may run into problems as discussed on #12261

I don't think #12261 is relevant to C++ code. With Python, auto formatting can theoretically result into parse errors. With C++, badly linted stuff simply won't build.
It makes sense to find out what are the defaults of VS Code as well. I assume Visual Studio and VS Code have similar defaults.

[feerrenrut]

badly linted stuff simply won't build.

There may be confusion of terms here. Formatting of C++ is one thing, and as you say low risk, should be easy to introduce (however git blame needs the ignored revision fix). Most C++ linters / checkers warn about more serious "correctness" issues. These warnings may highlight a bug, but fixing them isn't always straightforward, and may introduce other bugs, or snowball into a very large and hard to verify change.

Careful and incremental is my advice. I'd be happy for a reformat to be done automatically however, with something like ClangFormat

[LeonarddeR]

I forgot about this one. @seanbudd any plans to address it in the near future?

[seanbudd]

@LeonarddeR - it's on our roadmap on a very low priority and scheduled for late next year, so I wouldn't bet on it, but we'd like to tackle it eventually