Suggestion: add static code analysis

[escherstair]

I think that every project would benefit from some static code analysis (and probably stb already use some of them). If you think that Codacy can be an option, I configured it over my fork of stb project. Here you can see the reports.

[nothings]

Unfortunately, stb uses github in a weird way--I only push ready-to-go versions of the libraries, not in-development versions. This means it needs to already pass all tests that I care about before I push it, so tools that integrate with github et al aren't really that great. I.e. I can't use any online-only tools.

( I use Travis as a final sanity check, but it's just using normal compiler warnings that I can check locally so there's not a problem with me pushing stuff that doesn't pass it, unless I screw up.)

I do plan to work on making everything pass clang's static analyzer locally. Maybe once that happens we can add clang static analysis to the Travis makefile as well.

[rygorous]

That's a bad reason though. We should just set up test/analysis automation for the dev branch and merge PRs there (independent of the actual release cycle).

On Mon, Mar 25, 2019, 00:37 Sean Barrett notifications@github.com wrote:

Unfortunately, stb uses github in a weird way--I only push ready-to-go versions of the libraries, not in-development versions. This means it needs to already pass all tests that I care about before I push it, so tools that integrate with github aren't really that great. I use Travis as a final sanity check, because it doesn't warn on anything too extreme there's not a problem with me pushing stuff that doesn't build. So I can't use any online-only tools.

I do plan to work on making everything pass clang's static analyzer locally. Maybe once that happens we can add clang static analysis to the Travis makefile as well.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nothings/stb/issues/740#issuecomment-476085718, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBw-O39GCZVAFn1Ia9PiLSb67OSQsVjks5vaHy1gaJpZM4cGMRP .

[nothings]

(Note that I edited my comment while you were replying to it, but I doubt my changes make any difference to your reply.)

I'm concerned about changes I make myself, rather than PRs. I'd have to push them to dev to see if they're valid. I don't really want to pay that price on my workflow, at least not if clang -analyze gets us most of the value.

[jarikomppa]

Having static analysis doesn't really hurt anything. You can always just look at them and do possible fixes in the next version. Not having it doesn't make the (possible) problems go away.

Based on what I looked at the result, there was maybe one issue you might want to look at. I also set up the same for SoLoud, and out of the ~360 issues reported maybe one or two require action.

On Mon, Mar 25, 2019 at 9:50 AM Sean Barrett notifications@github.com wrote:

(Note that I edited my comment while you were replying to it, but I doubt my changes make any difference to your reply.)

I'm concerned about changes I make myself, rather than PRs. I'd have to push them to dev to see if they're valid. I don't really want to pay that price on my workflow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nothings/stb/issues/740#issuecomment-476088948, or mute the thread https://github.com/notifications/unsubscribe-auth/AEQ_Rye6MaXvfgV1I4vqcMOJ4Nn9VL_uks5vaH_igaJpZM4cGMRP .

[nothings]

Actually, I deleted that last reply because I realized I'm misunderstanding because I assumed it was intended for automated/CI usage. If there's 100s of false positives, then presumably the intent is to just run it once in a while and fix what comes up, not use it as CI? Or is there some mechanism to markup false positives so they stop showing?

[jarikomppa]

I wouldn't use it for CI, but to check it once in a while. You can run other static analyzers (clang, cppcheck, etc) offline in CI before releases. All static analyzers have some false positives, and all static analyzers find slightly different kinds of issues, so having several helps.

As far as I know there's no standard "stfu" clause for all static analyzers, but I know some have such things. I wouldn't start jumping through hoops to make all of them happy, though. Just skim through the report sometimes to see if something sounds scary.

On Mon, Mar 25, 2019 at 11:16 AM Sean Barrett notifications@github.com wrote:

Actually, I deleted that last reply because I realized I'm misunderstanding because I assumed it was intended for CI usage. If there's 100s of false positives, then presumably the intent is to just run it once in a while and fix what comes up, not use it as CI? Or is there some mechanism to markup false positives so they stop showing?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nothings/stb/issues/740#issuecomment-476112858, or mute the thread https://github.com/notifications/unsubscribe-auth/AEQ_RyDd-yI1PEwKB28WDS8LKPmjEo6Iks5vaJPtgaJpZM4cGMRP .