openhab / static-code-analysis

Maven tooling for static code analysis

Static Code Analysis Tool

The Static Code Analysis Tools is a Maven plugin that executes the Maven plugins for SpotBugs, Checkstyle and PMD and generates a merged .html report. It is especially designed for openHAB to respect the defined coding guidelines.

This project contains:

• properties files for the PMD, Checkstyle and SpotBugs Maven plugins configuration in the sat-plugin/src/main/resources/configuration folder;
• rule sets for the plugins in the sat-plugin/src/main/resources/rulesets folder;
• custom rules for PMD, CheckStyle and SpotBugs and unit tests for the rules;
• tool that merges the reports from the individual plugins in a summary report.

Essentials

1. A list of included checks.
2. How to use and configure the Static Analysis Tool.
3. How to integrate a new check into the tool.

3rd Party

• The example checks provided in the static-code-analysis-config (MethodLimitCheck, CustomClassNameLengthDetector, WhileLoopsMustUseBracesRule) are based on tutorials how to use the API of Checkstyle, SpotBugs and PMD. For more info, see javadoc;
• The tool that merges the individual reports is based completely on source files from the https://github.com/MarkusSprunck/static-code-analysis-report that are distributed under a custom license. More information can be found in the LICENSE file.

static-code-analysis's Issues

RequiredFilesCheck might report wrong directory

During a test execution the following tests failed:

Failed tests:   testMissingReadmeMdFile(org.openhab.tools.analysis.checkstyle.test.RequiredFilesCheckTest): error message 0 expected:<...readme_md_directory/[]README.md:0: Missing...> but was:<...readme_md_directory/[META-INF/]README.md:0: Missing...>
  testMissingPomXmlFile(org.openhab.tools.analysis.checkstyle.test.RequiredFilesCheckTest): error message 0 expected:<...g_pom_xml_directory/[]pom.xml:0: Missing p...> but was:<...g_pom_xml_directory/[META-INF/]pom.xml:0: Missing p...>
  testMissingAboutHtmlFile(org.openhab.tools.analysis.checkstyle.test.RequiredFilesCheckTest): error message 0 expected:<...bout_html_directory/[]about.html:0: Missin...> but was:<...bout_html_directory/[META-INF/]about.html:0: Missin...>
  testMissingBuildPropertiesFile(org.openhab.tools.analysis.checkstyle.test.RequiredFilesCheckTest): error message 0 expected:<...roperties_directory/[]build.properties:0: ...> but was:<...roperties_directory/[META-INF/]build.properties:0: ...>

I have looked at the code and found a problem on this line . The directory of the first visited file is used for the report generation, but different files might be in different directories. This is why the about.html file is expected to be in the META-INF folder in the log above.

Improve the documentation

I think the SAT documentation has grown too much, so it can be split it several separate files.

It would be great if there are more detailed instruction how to implement a new check and test for it, include it in the configuration and execute it on the openHAB code base.

Question: manifest wildcard

While working on a binding I got the following error:

[ERROR] ERROR found by checkstyle: .binding.windcentrale/META-INF/MANIFEST.MF:21 A good approach is to use OSGI-INF/*.xml instead of including the services metadata files separately or using common wildcard. 

But the developer used:

source.. = src/main/java/
output.. = target/classes
bin.includes = META-INF/,\
               .,\
               OSGI-INF/,\
               ESH-INF/,\
               about.html

Is there a reason why that is wrong?

Update checktyle

The version used in the SAT is 7.2 and currently 7.6.1. is released.

It should include a fix for the flooding Maven log in debug mode - https://github.com/openhab/static-code-analysis#known-problems.

Remove AvoidInstantiatingObjectsInLoops check

Discussion started in the openHAB forum .

Lolodomo

I have several AvoidInstantiatingObjectsInLoops
https://pmd.github.io/pmd-5.5.1/pmd-java/rules/java/optimizations.html#AvoidInstantiatingObjectsInLoops1
This rule looks weird to me. I don't see what is wrong in my code. Ok, I instantiate an object inside a loop but I need to do that !

I have not yet run the analysis on all bindings but isn't it something that comes back in all bindings very often ?

Kai Kreuzer

I am also not sure whether we should keep this check - ok, there might be situations, where it can be avoided, but in openHAB we probably are also not talking about loops with million iterations... @svilenvul, what do you think?

Add check for tabs in java files

Our code formatter is using spaces and no tabs in java files.
I'd therefore claim that any tab within a java file is not correct and should be marked as an error.

Could such a check be added?

RequireBundleCheck

I have started implementing a check that verifies if the MANIFEST.MF file contains a Require-Bundle header.

PackageExportsNameCheck - severity error

@kaikreuzer

We are writing a check that will cover this guideline:

10. Packages that contain classes that are not meant to be used by other bundles should have “internal” in their package name.

What severity should be used ? Warning or maybe just info ?

Many bundles (according to my estimation around 40-50 percent of the bundles ) in the OH2 addons repo contain packages that are not exported but are not "internal". Most often that are packages x.y.z.config and x.y.z.discovery. How important is this naming convention - should we proceed with some clean up and how ?

Tool causes Maven Warnings

When this tool is activated in a build, there are many warnings like

[WARNING] Unable to locate Source XRef to link to - DISABLED

in the Maven build log.

It seems that PMD requires some specific reporting plugin - could that be made part of the static analysis tool?

Add some documentation about covered guidelines

@svilenvul I am personally not having the overview about which bullets from our guidelines are already covered by the tool and which aren't yet.

Would you be able to provide a short info about that in the tool's documentation / README?

This would also be very helpful to track, which new checks might still need to be implemented.

Findbugs can't load jars if they contain special german characters in the path name

Started in the openHAB community forum.

The problem is that FindBugs can't find jars that are in directories containing special german characters. I couldn't find an open issue in the FindBugs repository or in the FindBugs Maven plugin repository, I guess it might be some configuration issue.

FindBugs loads all classes that are referenced in the code that is being analysed. When the path to the file contain special german characters, FindBugs is adding a path containing ? to the class path and later throws FileNotFoundException.

I have successfully reproduced the issue by checking out the OH2 repository to a subfolder containing special german characters. The Maven log shows the following:

....
[DEBUG]   Adding to AuxClasspath ->C:\prj\openHAB\EclipseIDE\git\löwe\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\jna-4.2.1.jar
[DEBUG]   Adding to AuxClasspath ->C:\prj\openHAB\EclipseIDE\git\löwe\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\pcap4j-core-1.6.6.
jar
[DEBUG]   Adding to AuxClasspath ->C:\prj\openHAB\EclipseIDE\git\löwe\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\pcap4j-packetfacto
ry-static-1.6.6.jar
...
...
...
     [java] JVM args ignored when same JVM is used.
     [java] The following errors occurred during analysis:
     [java]   File not found: filesystem:C:\prj\openHAB\EclipseIDE\git\l?we\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\jna-4.2.1.ja
r
     [java]   File not found: filesystem:C:\prj\openHAB\EclipseIDE\git\l?we\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\pcap4j-core-
1.6.6.jar
     [java]   File not found: filesystem:C:\prj\openHAB\EclipseIDE\git\l?we\openhab2-addons\addons\binding\org.openhab.binding.amazondashbutton\lib\pcap4j-packe
tfactory-static-1.6.6.jar
     [java] The following classes needed for analysis were missing:
     [java]   org.pcap4j.core.PacketListener
     [java]   org.pcap4j.core.PcapHandle
     [java]   org.pcap4j.packet.Packet
     [java]   org.pcap4j.packet.EthernetPacket
     [java]   org.pcap4j.packet.EthernetPacket$EthernetHeader
     [java]   org.pcap4j.core.PcapNetworkInterface$PromiscuousMode
     [java]   org.pcap4j.core.BpfProgram$BpfCompileMode
     [java]   org.pcap4j.packet.namednumber.ArpOperation
     [java]   org.pcap4j.packet.namednumber.UdpPort
     [java]   org.pcap4j.packet.ArpPacket
     [java]   org.pcap4j.packet.ArpPacket$ArpHeader
     [java]   org.pcap4j.packet.UdpPacket
     [java]   org.pcap4j.packet.UdpPacket$UdpHeader
     [java]   org.pcap4j.core.PcapAddress
     [java]   org.pcap4j.core.PcapNetworkInterface
     [java]   org.pcap4j.core.Pcaps
     [java]   org.pcap4j.util.MacAddress
     [java]   org.pcap4j.core.PcapNativeException
     [java]   org.pcap4j.core.NotOpenException
     [java]   org.pcap4j.core.BpfProgram
     [java] Missing classes: 18
     [java] Analysis errors: 3
...

Add codestyle checks: empty line

There are often empty lines in the code which might be detected automatically:

• after method declaration and before the method-closing-curly-braces:

private void doSomething() {

    String myString....

}

• after if, for, switch, case, break statements:

if (true) {

    for (;true;) {
    
        switch (value) {

            case 1:

                doSomething();
                break;

        }
    }
}

RequiredFilesCheck

I saw that there is an open discussion for a check that verifies whether a binding contains a README.md file: #13.

I have started implementing a check that will be able to verify not only the existence of a README.md, but the existence of any file, that is included in the configuration of the check (e.g. another files that we could check for existence are about.html, pom.xml, build.properties, MANIFEST.MF).

Add checks for README.md

A very common mistake in PRs is that there are no empty lines in READMEs to make the Jekyll-rendering correct.

On https://www.eclipse.org/smarthome/documentation/development/bindings/docs.html, we have documented the rules:

There must be an empty line after every header (#...).
There must be an empty line before and after every list.
There must be an empty line before and after every code section.

It would be great if this could be checked automatically on any README.md file.

Additionally, for bindings, the existence of a README.md file should be checked (and the build should fail, if none is there).

Make a warning check for scheduleAtFixedRate.

The coding guidelines state the following:

For periodically executed jobs that do not require a fixed rate scheduleWithFixedDelay should be preferred over scheduleAtFixedRate.

It sounds like an easy static check, could it be an idea to add an info / warning when you use scheduleAtFixedRate?

FindBugs slf4j bug pattern

IIRC I comment this already on the ESH repo some time ago before the MOJO moved.
Shouldn't the slf4j bug pattern for FindBugs be a mandatory check?
Currently it seems we need to add the pattern in the plugin configuration:
https://github.com/openhab/static-code-analysis/blame/dd38ca38b698189a7e118e6c0adca894b00cedf4/README.md#L41

I assume this should be configured by the plugin itself similar to the FindBugs, Checkstyle, PMD, ... dependencies, too.

Exception if MANIFEST contains "Bundle-Description"

I had a "Bundle-Description" entry in a manifest of a bundle, which made the checks fail with an exception (similar one as mentioned in https://issues.apache.org/jira/browse/IVY-1438).

Removing this entry made the checks pass correctly.

Maven compatibility issues

The ESH code formatter sets the max line width to 120 while the LineLengthCheck configured in this tool for CheckStyle flags lines over 80 characters as priority 1.

• What is the desired max line length for ESH & openHAB?
• The formatter and the CheckStyle rule should be aligned.

Check for the Service-Component header in the MANIFEST.MF

My colleague @AleksandarKovachev started implementing a check, that verifies if all the services in the OSGI-INF/ directory are included in the Service-Component header of the MANIFEST.MF file. I will soon finish the check and the tests for it and make a public PR about it.

@author tag only necessary once per file

I just checked a PR in ESH with the static code analyser and found this "bug":

Here it complains about a missing author tag:

ERROR] ERROR found by checkstyle: org.eclipse.smarthome.binding.dmx.internal.action.FadeAction.java:149 An author tag is missing

But the file FadeAction.java does have an author tag set, just not in the extra enum defined at the end of the file.

External Libraries Check

Hello,

I am working on a check that verifies that all external libraries in a bundle (if they exist) are added properly:
The external libraries are packaged as a .jar file and usually located in a "lib" folder at the root of a bundle. The check should:

• assert that all jar files or the lib folder is added to the build.properties file;

• assert that all jar files are added to the "Bundle-Classpath:" MANIFEST.MF header.

I have a few questions:
Is it mandatory for the folder to be named "lib"?
Could there be more than 1 folder, containing external dependencies?

@kaikreuzer WDYT

Checks not working on Travis

As The Jenkins PR builder has memory issues with activated checks, I tried out Travis instead.

There, the builds run through nicely, without any memory issues.
Unfortunately, they seem to be "too green" - almost every PR has succeeded and if I look at the logs, it seems as if the check result files are missing:

[INFO] --- static-code-analysis:0.0.3:report (default) @ org.openhab.binding.rme ---
Error 
  java.io.FileNotFoundException:
  /home/travis/build/openhab/openhab2-addons/addons/binding/org.openhab.binding.rme/target/code-analysis/checkstyle-result.xml (No such file or directory)
[ERROR] java.io.FileNotFoundException: /home/travis/build/openhab/openhab2-addons/addons/binding/org.openhab.binding.rme/target/code-analysis/checkstyle-result.xml (No such file or directory)
Error 
  SXXP0003: Error reported by XML parser: Premature end of file.
[ERROR] org.xml.sax.SAXParseException; Premature end of file.
Error 
  SXXP0003: Error reported by XML parser: Premature end of file.
[ERROR] org.xml.sax.SAXParseException; Premature end of file.
Error 
  SXXP0003: Error reported by XML parser: Premature end of file.
[ERROR] org.xml.sax.SAXParseException; Premature end of file.
[Fatal Error] :-1:-1: Premature end of file.
[WARNING] Cann't select /sca/file/message nodes from /home/travis/build/openhab/openhab2-addons/addons/binding/org.openhab.binding.rme/target/code-analysis/20946993_SecondMerge.xml. Empty NodeList will be returned.
...

Furthermore, it seems to miss some other of our recent changes as I see things like

[WARNING] Unable to locate Source XRef to link to - DISABLED

and

[INFO] Fork Value is true

Any idea what might be wrong on Travis?

Should every binding be mentioned in features\openhab-addons\src\main\feature\feature.xml

Since this is a file which is not changed, so no incorrect change but a missing change, its harder to detect in a PR. If it is required we could consider adding a rule for this?

Exception in ExportInternalPackageCheck

See openhab/openhab1-addons#5268.

The stack trace:

Caused by: com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing C:\prj\openHAB\EclipseIDE\git\openhab1-addons\bundles\binding\org
binding.homematic\META-INF\MANIFEST.MF
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:293)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:211)
        at org.apache.maven.plugin.checkstyle.exec.DefaultCheckstyleExecutor.executeCheckstyle(DefaultCheckstyleExecutor.java:252)
        at org.apache.maven.plugin.checkstyle.AbstractCheckstyleReport.executeReport(AbstractCheckstyleReport.java:473)
        ... 29 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: -1
        at com.puppycrawl.tools.checkstyle.api.FileText.get(FileText.java:326)
        at com.puppycrawl.tools.checkstyle.api.FileText.get(FileText.java:55)
        at org.openhab.tools.analysis.checkstyle.api.AbstractStaticCheck.findLineNumber(AbstractStaticCheck.java:69)
        at org.openhab.tools.analysis.checkstyle.ExportInternalPackageCheck.processFiltered(ExportInternalPackageCheck.java:47)
        at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:78)
        at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:314)
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:284)
        ... 32 more

Check that loggers are non-static loggers and have the correct name

The overview page suggests that somebody might be working on the following rule:

As we are in a dynamic OSGi environment, loggers should be non-static, when ever possible and have the name logger.

Is that case and if yes could we make it more visible by assigning this task :-)

XML validation check

Currently we have a check that validates thing types, channel types and configurations against the xsd schemas and logs an error if there is a violation.

This check finds a problem in the following code:

....
 <properties>
        	<property name="vendor">Philips</property>
        </properties>
        <representation-property>serialNumber</representation-property>
        
        <config-description>
            <parameter name="ipAddress" type="text">
...

The error message is:

[ERROR] ERROR found by checkstyle: C:\prj\openHAB\EclipseIDE\git\smarthome\extensions\binding\org.eclipse.smarthome.binding.hue\ESH-INF\thing\bridge.xml:15 Invalid content was foun
d starting with element representation-property. One of {config-description, config-description-ref} is expected.

Eclipse as well marks the xml as invalid.

How should we proceed in this case ?

• ignore the file ;
• update the XSD schema ;
• reduce the priority of the check;
• consider it as really an error and change the file;
• some other option, maybe ..

@kaikreuzer, please take a look.

False positive in EshInfXmlCheck

On core.karaf bundle, I get the prio 1 error

Unused configuration reference with uri - system:addons

I have no clue what should be wrong and how I could workaround that. @svilenvul can you help? This is rather critical for me :-)

Summary Report uses absolut urls

I activated the analysis tool on Jenkins and kept the reports as a build artifact.
Unfortunately, the summary report contains absolute file:// urls, so that the links do not work when being used directly on Jenkins, see https://openhab.ci.cloudbees.com/view/Pull%20Request%20Builds/job/openHAB2-CodeAnalysis/lastSuccessfulBuild/artifact/target/summary_report.html.

Reduce memory allocation for findbugs

I am currently trying to run the checks within the openHAB PR builder.

This has only 1GB of total memory available and the Maven build exists with error 137, which seems to be due to a lack of available native memory.
I am therefore looking into reducing the memory consumption of the build.

What I have noticed is that the FindBugs plugins spawns a separate Java process and uses -Xmx=512m as a JVM parameter. I assume that this actually leads to the issue and wonder, if there is a way to reduce this to 256m (which I would hope should be enough for Findbugs to do its job).

Add a warning for JavaDocs that only use "inheritDoc"

I often see these in reviews:

   /**
    * {@inheritDoc}
    */
   @Override

e.g. here.

As Javadoc is inherited by default from an interface definition if nothing else is specified, this is redundant and should be avoided. I would therefore suggest to add a warning in such a case.

inheritDoc should imho only be used if some additional documentation is added to it on a specific method.

Eclipse project - test build problems

While the tests are compiled and run correctly in the Maven build, the Eclipse project has some problems.

Eclipse project files are generated by the Maven Eclipse Plugin. The .classpath file looks the following way:

<?xml version="1.0" encoding="UTF-8"?>
<classpath>
  <classpathentry kind="src" path="src/test/java" output="target/test-classes" including="**/*.java"/>
  <classpathentry kind="src" path="src/test/resources" output="target/test-classes" excluding="**/*.java"/>
  <classpathentry kind="src" path="src/main/java" including="**/*.java"/>
  <classpathentry kind="src" path="src/main/resources" excluding="**/*.java"/>
  <classpathentry kind="output" path="target/classes"/>
   ............
<classpath/>

Java test files (located in "src/test/java") are excluded from the build. I have tried to change excluding="**/*.java" to including="**/*.java", but then the java files are compiled instead of just copied.

The Maven Eclipse Plugin is simple and easy to use but doesn't offer much configuration options and is deprecated. As far as I remember the project was initially m2e, but after moving to this repo, we converted it to simple java project.

@kaikreuzer do you think it is a good idea to convert the project to Eclipse Maven Project (the openHAB IDE seems to have support for m2e) ?

Check for the Java version in the MANIFEST

Started here : https://github.com/openhab/openhab2-addons/pull/2087#issuecomment-289412622

FYI: We could add a check for the Java version in the MANIFEST now as well - it is now the very same in ESH and OH2!

See also:

#30 (comment)

Report mojo can't handle special characters in the path name

During the searching of the cause for the #91, I have found issues with the ReportUtility mojo in the static analysis tool.

It gets some path names as Strings and later throws FileNotFoundException when trying to to open these files. Soon I will be ready with a PR that removes the usage for Strings for path names.

Static loggers in (util) classes

The build fails locally at (among other warnings (#47 (comment)))

[ERROR] ERROR found by findbugs: org.openhab.binding.homematic.type.MetadataUtils.java:
[ERROR] Logger should be non-static field. Change this field (logger) to non-static field.

I think we should allow at least classes which are named util to have static loggers.

Imho it has never been a bad thing to use static methods (even in normal classes) so there also some occurrences where we have other static methods which use logging. It feels strange and bad to refactor those static methods away because of this rule, can we make it a warning instead of an error.

False positive in BuildPropertiesCheck

On a test bundle project, I get the following (prio 1) message:

Property output.. in the build.properties file is missing value: target/classes

This project does not define target/classes as an output folder, but only target/test-classes, which is correctly specified in build.properties.

I therefore consider this a bug in the checks, would you agree?

SAT build fails with error in findBugs.xml

Log:

[java] Missing classes: 24
[java] Analysis errors: 182
[INFO] Done FindBugs Analysis....
[Fatal Error] findbugsXml.xml:2:145: Invalid byte 2 of 4-byte UTF-8 sequence.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 39.539 s
[INFO] Finished at: 2017-06-09T23:31:55+02:00
[INFO] Final Memory: 64M/578M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.openhab.tools:static-code-analysis:0.0.6:findbugs (default) on project org.openhab.binding.tankerkoenig: Unable to execute mojo: Invalid byte 2 of 4-byte UTF-8 sequence. -> [Help 1]

martinvw commented

It was caused by the umlaute in the pom.xml which ended up in the findBugs.XML but it might also just be a problem with Findbugs itself.

Add static analysis tool verifications to the build lifecycle

I think it is time to run the code checks on our own codebase as well.

We will use the latest release version of the tool (the current one is 0.1.0) for the checks.

I have created a few PRs that realize this:

• #125
• #126
• #127

@kaikreuzer I think it will be great if we include it in the next release.

Check that parametrized logging is used

A common mistake is to use string concatenation instead of parametrized logging in slf4j.
Would be great if there could be a check for that!

JavaDoc generation shows errors

With 55a2a1b I have added JavaDoc generation to the build, but I had to set failOnError=false as otherwise the build didn't succeed due to errors.
This should be cleaned up, so that this option can be removed again.

Move common constants and methods to the base classes

During PR reviews we found that several constants and methods are duplicated across different checks. This includes:

• logMessage();
• constants for different file extensions;
• constants for different properties;
• names of some files (build.properties, pom.xml, etc..).

The existing checks have to be refactored.

Content of default ruleset files

When we started the discussion for the tool, one of the requirements was the option to have different rule sets for different kinds of projects (bundle, binding, ui,...).

The tool contains a set of default ruleset files that will be used when executing the Maven plugin without additional configuration.

Optionally in another Maven profile we can configure the tool to use another set of .xml files as it is described here. So far so good.

In the openHAB addons build we are using currently only the default rulesets. What should these rulesets contain ? Options are :

• checks that apply only for binding;
• checks that apply for bundles (this the case now);
• some other set of checks.

This question is related to #25 - #25 (comment).

If we keep the current defaults, we should add another set of rules in the OH2 adddons repository only for bindings. (where the check for README.MD can be included) and add another Maven profile. I am not sure if it is worth it, as only few checks are not applicable for both bindings and bundles... and we should maintain another set of .xml files.

@kaikreuzer, what do you think - should we change the default ruleset files to include checks only for bindings (and skip checking 'none-bindnigs' in OH2 for now) or we should start using separate profiles for bindings and for other type of projects?

Travis build and local build report differ with version 0.1.0

[ERROR] ERROR found by checkstyle: .binding.avmfritz.test/META-INF/MANIFEST.MF:25 The MANIFEST.MF file must not contain any Require-Bundle entries. Instead, Import-Package must be used.

The "Require-Bundle" entries in the test fragment (for org.junit, org.mockito, org.hamcrest) are the only exception from this rule. We have to adopt this in the checkstyle configuration.

See openhab/openhab2-addons#2516 .

FindBugs memory leak when same JVM is used

We have noticed that the static analysis tool is consuming a lot of memory when executed on a multi module build (as the openHAB build :)) and I made some research. I will try to summarize the results here.

I remember that we had a discussion if we should disable the fork option (which spawns a separate Java process for each execution of the FindBugs plugin), but this doesn't seem to be the best option now.

I have used the jconsole memory profiling tool and below can be seen the most critical leak that I have found (1 GB of non-heap memory usage):

The memory seems to be consumed by 100 000 classes that are being loaded (the lower left window). I suspect that the problem is related to the fact that the same JVM is used (see below for more details):

I have customized the build and excluding the FindBugs maven plugin the problem disappears.

I have experimented again with a fork set to true and the results are good (non-heap memory usage is stable around 200 MB and 20 000 classes are loaded), the separate process consumes around 300 MB before dying:

@kaikreuzer, I think the best option will be to use a separate thread for FindBugs, but this change could directly affect the openHAB build. Please share your opinion on the topic :)

Is it possible to skip a check in external repository?

Since the update to openHAB 2.2, I'm getting the following build error for my binding (https://github.com/pfink/openhab2-flicbutton):

Wrong /project/parent/version in the pom.xml file. The version should match the one in the MANIFEST.MF file.

I think this check makes sense for the addons which are part of the official openhab2-addons repository, but it does not fit for addons outside this repo as those addons may not have the same release cycle than openHAB and may don't want to use the same versions. From my point of view there should be a possibility to skip this rule without having to skip the whole static code analysis.

Move the configuration of the checks out of the Maven plugin

The configuration of the checks shouldn't be included in the Maven plugin. At the moment it is stored in the ''src/main/resources/" folder, but should be moved to the repository that is using the plugin.

This will allow having a different configurations for the same check, when using the tool in different builds.

Switch from FindBugs to SpotBugs

Migrated from #15 (comment):

kaikreuzer wrote :
Btw (sorry, maybe slightly off-topic): Did you already consider switching to https://github.com/spotbugs/spotbugs?

SpotBugs project description:

SpotBugs is the spiritual successor of FindBugs, carrying on from the point where it left off with support of its community.

Requirements:

To use SpotBugs, you need a runtime environment compatible with Java version 1.8 or later. SpotBugs is platform independent, and is known to run on GNU/Linux, Windows, and MacOS X platforms.

You should have at least 512 MB of memory to use SpotBugs. To analyze very large projects, more memory may be needed.

There is a guide for migration from FindBugs to SpotBugs.

Create check that validates files in ESH-INF directory

The check will validate the .xml files against the ESH thing description XSD, config description XSD and binding XSD.

It will include as well checks for missing or unused:

• config-description.uri ;
• bridge-type-ref.id ;

See: http://www.eclipse.org/smarthome/documentation/development/bindings/xml-reference.html.

Checkstyle Header check does not work as expected

On almost all (openHAB2) files, I see the checkstyle error:

Line does not match expected header line of ' * All rights reserved. This program and the accompanying materials'.

although the files have a correct license header, see e.g. https://openhab.ci.cloudbees.com/view/Pull%20Request%20Builds/job/openHAB2-CodeAnalysis/lastSuccessfulBuild/artifact/addons/binding/org.openhab.binding.feed/target/code-analysis/report.html.

Typos in readme

Singular vs. plural: a user properties
https://github.com/MusalaSoft/static-code-analysis/blame/8ea229cdc77f1ba07e86aa2ec845ef03e2de4729/README.md#L174

Change the priority of HeaderCheck from info to error

kaikreuzer 16 hours ago Owner

Hm, I found out why: Because it is considered prio 3.
@svilenvul Did we do that by intention? I would actually hope that a missing header breaks the build - and where we have exceptions because we need different headers, it would be nice if we could deactivate this check in the pom of the appropriate bundle.

See https://github.com/openhab/openhab2-addons/pull/1711#discussion_r131113839