Investigate and fix issues reported by static code analyzers

[stweil]

Static code analyzers for Java report some issues in the current code: FindBugs reports more than 500, Coverity around 440. As there are real errors among those issues, they need more investigation.

FindBugs is Freeware and included in Linux Distributions, so it is easy to run those checks in your own build environment.

Coverity is closed source, but they offer a scan service for free software. They scanned Goobi.Production, and a summary is available here: https://scan.coverity.com/projects/4522. The full report is available on demand - just use the link on the website.

[claussni]

I agree that static analyzers should be used in the development process. However, the code base is old and mostly untested, but stable. Affected and erroneous parts are all over the class path. Some bugs might as well be features (and the other way around). Almost all parts of the application need a complete renovation, which is costly.

Introducing proper development mechanisms like code analysis, automated integration testing and style checking is common sense. But as long as there is only feature-driven funding we will need to take a pragmatic road.

[henning-gerhardt]

General notice for Coverity: You can use your GitHub account for access to it. Afterwards you should apply for more access. Without you see no Coverity reports / detailed information.

[matthias-ronge]

Hi @stweil, The Coverty report doesn't seem to have been updated for more than three years. If we still follow this approach, how will it be updated? I logged in, but I see no way. Is it possible at all?

[stweil]

Hi Matthias, yes, that's confusing. https://scan.coverity.com/projects/kitodo-kitodo-production is a dead end and won't be updated because it was superseded by https://scan.coverity.com/projects/kitodo-production-master-branch. @henning-gerhardt, are you planning an update now as 3.0.0 was released? If an update is desired for 2.x as well that would also have to be done by @henning-gerhardt as I no longer have the necessary rights.

[henning-gerhardt]

Coverity is up again? I did not check this tool anymore after the had a lot of security issues, a new owner and their command line tool works not anymore. I have no plan to re-activate it because there are many other tools in the wild with more features and a better UI. I used a long time SonarCloud as a cloud based solution or Vulas (currently named as steady as a local working solution for security checks). But no one was really working on the issues discovered by Coverity or SonarCloud :-(

If @Kathrin-Huber will use Coverity I can transfer it to her or we abandon it.

[stweil]

Issue #2349 has also be fixed before external code analyzers can access the whole Kitodo code base.

[stweil]

First results with 9 errors and 27 warnings are now also available from LGTM. The analyzer is still busy, so maybe more reports will get added.

[stweil]

@Kathrin-Huber, LGTM reports a Java code quality of C (= can be improved). A quick look on the detected issues shows some obvious bugs, for example in IllustratedSelectItemConverter.java:32 (|| instead of &&?).

[Kathrin-Huber]

It's a pretty generic issue. please open issues for specific enhancements.

[stweil]

@Kathrin-Huber, it's not about enhancements, it's about real or potential bugs or code which looks buggy although it is correct, for example the one mentioned above. LGTM lists 8 errors and 25 warnings. Ideally there should be none, either because the code was fixed or because it was marked that there is no problem. Do you think that opening up to 8 + 25 new issues would be easier to handle?

[stweil]

A quick look on the detected issues shows some obvious bugs, for example in IllustratedSelectItemConverter.java:32 (|| instead of &&?).

This bug was fixed in commit cc95e035362210d33c5c652ec2febf6ac79149ab.