Static analysis

[wolf99]

Should we add static analysis to the CI tasks to analyse each implementation?

As it is all the exercise implementations work just fine -- according to the tests that have been written. Static analysis would add a second layer of "correctness" checking, for the current exercises and also for any exercises added into the future. So far I don't think there is any specific need for this it just occurred to me that it might be a nice-to-have thing. Downsides might be potentially increased maintenance in the case of any major change to the track structure.

Thoughts?

[wolf99]

FYI I have no previous experience with explicit static analysis but I'm willing to work on implementing this if it people think it's worthwhile. I currently know of two open source static analysers, Splint and Infer.

• Splint is kind of old at this stage and the development seems to have died some time ago
• Infer is from Facebook so possibly has their weird license. It's based on Clang but can be made to work with GCC builds

Other suggestions welcome 😃

[ryanplusplus]

I'm not opposed to adding some static analysis. I don't have any experience with any FOSS linters for C, though.

[wolf99]

The Infer license is actually plain BSD (https://github.com/facebook/infer/blob/master/LICENSE) so should be OK in that regard I think.

[Gamecock]

I don't know how valuable it is to confirm the example. The goal of exercism is to help people write better code, and most people never see the example. Would it be more useful to tie into make like we are planning on doing for memory analysis #169 .

[wolf99]

@Gamecock That could be a possibility... An optional, additional, make target for static analysis then?

Seems there are lots of other FOSS C analysers to pick from: https://github.com/mre/awesome-static-analysis#cc, such as CppCheck

[arcuru]

I think that this is somewhat valuable. Not because static analyzing the code here is terribly useful (though I guess we could leverage it to also have another 'extra credit' checker alongside Valgrind), but because I want to have a simple to steal setup for Travis.

I've been meaning to do that at some point (not necessarily here, but setting up a Travis setup with Cppcheck, the Clang tools (all their sanitizers, which is a superset of Valgrind AFAIK, clang-tidy for static analysis / style, clang-format for formatting, etc), maybe Valgrind if that's not covered by Clang, etc. I haven't heard of Infer, but at a glance it looks like it may be worth looking into.

[wolf99]

Looking at CppCheck a simple implementaion for CI would add something like the following to the #Make it! section

if cppcheck --std=c99 --enable=portability,warning,performance,style ./src/example.* | grep error ;  then
  exit 1
fi

As an addition to the makefile it should be something similarly simple

CPPCFLAGS  = --std=c99
CPPCFLAGS += --enable=portability,warning,performance,style

cppcheck:
    @cppcheck $(CPPCFLAGS) src/$(EXERCISE_NAME).c src/$(EXERCISE_NAME).h

[wolf99]

SonarQube might also prove interesting - they are free for open source too 😃

[antony-jr]

you can use codacy to automatically check the entire repo , this also includes checking new pull request automatically and thus notifies everyone about the code quality 🍰 🐱 🐶

EDIT : codacy is also a part of github so integration is simple!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[wolf99]

I've recently started a new job and not had as much time to look at this as I'd like. It may be some time but I would still like to play with this and get it working at some stage. If anyone else is interested feel free to contribute suggestions or PRs!

[DrJPizzle]

+1. I think this is a great idea.

In addition to the obvious "It checks the code", even if it's not exposed straight away it think it likely to be useful for running on student's submissions as a starting point for feedback.

I think, especially as there is the potential for learners to want to copy the behaviour, having something stand-alone and mature would be a good thing.
Also, for the same reason, having it as an additional step is a good thing.
As:
a) It may reduce frustration if the code works fine in theory but the make file spits out "redundant initialisation of ctr on line 45" or build failed as can't find cppcheck.
b) It makes it conceptually clearer what parts of the process do what. Which is useful to a complete newcomer and someone looking to scavenge the cppcheck addition to cmake.
It also:
c) Gets in the way less if for some reason you don't want the checks.
d) Gives sightly more obvious high fidelity feedback. I.e.: it passes the functional side of things but the linter is not happy, is easier to digest at a glance than "exit status 1" underneath a cppcheck output.

Putting this into make is as you describe, however, I would use:

--error-exitcode=1

instead of piping to grep for error. If you have a perf warning in a function handles

error_code

...

Depending on the version of cmake (3.10 or above) in use, this is really simple in cmake.
I know that's not how the c track works, but it might be worth thinking about:

• generating the make files.
• using this in the cpp track.

See:
https://blog.kitware.com/static-checks-with-cmake-cdash-iwyu-clang-tidy-lwyu-cpplint-and-cppcheck/
but effectively it boils down to:

set(CMAKE_CXX_CPPCHECK "cppcheck")

Final note: overzealous linters put good developers off, I would be reluctant to make PRs an automatic no because the linter isn't happy until you are sure it's making good decisions.