Pcl: [CI] Discussion: Adding a linter in the CI pipeline

[divmadan]

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

Linting issues, which can lead to build-fails, are currently not detected in our CI pipeline.

Context

While extending clang-format to the tracking directory, custom macro formatting gave rise to build errors which was solved by doing a linter check locally: https://github.com/PointCloudLibrary/pcl/pull/3814#issuecomment-616761450

Expected behaviour

A linting check before the formatting begins, and to abort the pipeline in case of errors.
It's lightweight, so there shouldn't be an overhead problem and will save time (early detection).

Current Behavior

No linting. It leads to build-fails which are difficult to debug because they do not indicate syntax issues.

---

_See thread: https://github.com/PointCloudLibrary/pcl/pull/3814#issuecomment-616846356_

[shrijitsingh99]

I like the idea of having a static analyser as part of the CI which runs maybe weekly.
Though there could be an argument tools like Codacy exists so there might be not benefit to this.

Integrating it to the current pipeline might be little redundant though I understand that running it before building it helps catch errors before spending time on a lengthy build, but as I understand most people will always build the project before pushing a commit so errors will get detected locally.

Clang-tidy and cppcheck are some well known tools and our configurable to our requirements should we need it.

Instead of using it to identify build issues, I think it would be better to use it to improve code quality this the suggestion of running it periodically.

[divmadan]

Instead of using it to identify build issues, I think it would be better to use it to improve code quality this the suggestion of running it periodically.

I think codacy works fine for static analysis.

[kunaltyagi]

clang-tidy is not a linter, but I propose adding it to the format stage to keep things tidy and not relapse on fixes already applied. @SunBlack can help a potential implementer know what those fixes are.

[shrijitsingh99]

clang-tidy is not a linter

What is the exact difference between a static analyzer and a linter? As per my current understanding linters contain a basic a static -analyzer and tools to resolve styling issues.

I propose adding it to the format stage to keep things tidy and not relapse on fixes already applied. @SunBlack can help a potential implementer know what those fixes are.

Sounds good. if no one is working on it I can go ahead and start this.

[kunaltyagi]

This SO question says that all linters are static-analyzers, but all static-analyzers aren't linters. Linters are focussed on the linting (finding issues in code) while static-analyzers produce a metric without compiling. Clang-tidy goes beyond a linter into a static-analyzer territory, and more since it can take compile_commands.json.

And example of something that's not a static analyzer or linter, yet is used in that way would be IWYU (Include What You Use).

[jasjuang]

Just in case if interested, this is how I usually run cpplint in CI on Ubuntu

cpplint --counting=detailed $(find io/ kdtree/ keypoints/ -type f \( -name '*.cpp' -o -name '*.h' -o -name '*.cu' -o -name '*.cuh' \) -print)

If success it returns 0, otherwise it will return 1.