Static Code Analysis Tool

J-N-K · March 7, 2017, 6:46pm

Ok, got it working. Now it fails with bugs outside my binding and requests those to be fixed. Is it possible to configure the build to not fail on errors? If I add -fae the build runs on, but most of the targets are skipped because smarthome.core failed.

svilenvul · March 8, 2017, 12:27pm

Hi @J-N-K, thanks for testing it with the latest ESH build.

Yes, it is possible to configure the build to not fail on errors - try it out with -Dreport.fail.on.error=false.

mammuth · March 11, 2017, 10:42am

@Kai running the code analysis could be automated as well. For example like this workflow:

Run a Travis build (free for open source projects) on new PR’s or new commits on a PR
Perform a sonar-scanner analysis (free sonarqube.com server can be used). Sonar has support/plugins for PMD, FindBugs and Checkstyle
Make use of the Sonar GitHub Plugin to have a Bot automatically comment the found issues to the PR (check out the screenshots here or see the bot in action on this repo: https://github.com/SonarSource/sonarqube/pulls)
Make the bot add a label like has-code-smell or something similar which indicates that no core dev needs to further take a review at the current time. The author of the PR has all needed information on how to fix the issues by the Bot. Otherwise, a label like ready-for-review might be added.

This should be pretty easy to setup. The most complicated step might be to get sonar-scanner running with PMD, FindBugs and Checkstyle (at least I have no experience with that).

However, it would improve a) the time a PR’s author waits for feedback and b) reviewers can focus on PR’s that have no “boring” code issues. If you guys have questions I’m happy to answer or help out.

J-N-K · March 11, 2017, 1:21pm

That works fine (besides some tests e.g. hue still fail, but I got around that by -fae). I have a question regarding the report.

I have a lot of

pmd   3   78   optimization   AvoidInstantiatingObjectsInLoops   Avoid instantiating new objects inside loops

However, I don’t see how this should be avoided if I create objects which I all use and that need to be valid after the loop finishes (e.g. I create a object for each DMX channel during thing creation. These objects are added to a List, so they can be accessed later). Is this merely a warning that can be ignored if I check that is done intentionally?

Regards,

-jnk

Kai · March 12, 2017, 10:18am

See here. So in general, it should be avoided, but in practice it seems to be necessary. As we are on Java8 by now, populating lists might be more elegantly done through streams now, which should not show such warnings.
As it is just a warning and not an error, I think it is ok to keep it in the report.

Kai · March 13, 2017, 11:09am

Yes, that’s what I want to do as I have mentioned above (“Once we have good results from this tool, I would plan to activate it on PR builds”).
Note that we already have PR builds in place with Jenkins on Cloudbees. I would not want to have addional PR builds on Travis, if this can be avoided as it might rather confuse people.
Issue with SonarQube is that afaik, you cannot use your personal set of checks, so this won’t help us, if I do not miss anything?

Kai · March 13, 2017, 1:58pm

FTR, we now have a public report available of the current sources in the repo here:
https://openhab.ci.cloudbees.com/job/openHAB2-CodeAnalysis/lastSuccessfulBuild/artifact/target/summary_report.html

Feel free to browse through it and identify anything that is falsely reported as well as anything that you think is covered in our coding guidelines, but not being mentioned in the report. You can directly add an issue here with your suggestions.

opus · March 14, 2017, 7:15pm

I’m doing something wrong, but what?
I tried to run “mvn clean install -P check” after having added the profiles to the pom.xml in rot directory of my binding.
I’m getting those errors:

[ERROR] [ERROR] Some problems were encountered while processing the POMs:
[ERROR] ‘build.plugins.plugin.version’ for org.openhab.tools:static-code-analysis must be a valid version but is ‘${sat.version}’. @ org.openhab:pom:2.0.0-SNAPSHOT, D:\OpenHABEntwicklung\openhab2-master\git\MyTankerkoenig\pom.xml, line 319, column 22
@
[ERROR] The build could not read 1 project → [Help 1]
…

What am I missing?

Kai · March 15, 2017, 6:39am

You should check, why you have 2.0.0-SNAPSHOT references in your build. Clearly, this this new profile only exists in 2.1.0-SNAPSHOT.

opus · March 15, 2017, 9:37am

OK.
I’m actually working in support to the author of a binding So I forked from him and he in turn has forked 2.0.0
I’ll try to fork from the actual Addons snapshot and put the binding-code in there.
Will inform the author as well.
@dolic

[Edit;] Worked!

Lolodomo · March 18, 2017, 10:22pm

I ran “mvn clean install -P check” in my openhab2-addons directory but I can’t find any HTML report What is exactly the absolute path of the report ?
Do I have to install something before ?

Kai · March 19, 2017, 6:33am

You need to be on the latest master - make sure to have your branch rebased on it.

Lolodomo · March 20, 2017, 7:16pm

@Kai: with the latest master, I got this error:

 [ERROR] Failed to execute goal org.openhab.tools:static-code-analysis:0.0.2:checkstyle (default) on project org.openhab.binding.powermax: Execution default of goal org.openhab.tools:static-code-analysis:0.0.2:checkstyle failed: Plugin org.openhab.tools:static-code-analysis:0.0.2 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar -> [Help 1]

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.openhab.tools:static-code-analysis:0.0.2:checkstyle (default) on project org.openhab.binding.powermax: Execution default of goal org.openhab.tools:static-code-analysis:0.0.2:checkstyle failed: Plugin org.openhab.tools:static-code-analysis:0.0.2 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
        at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
        at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
        at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
        at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
        at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
        at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
        at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
        at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
        at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
        at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default of goal org.openhab.tools:static-code-analysis:0.0.2:checkstyle failed: Plugin org.openhab.tools:static-code-analysis:0.0.2 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar
        at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:106)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
        ... 20 more
Caused by: org.apache.maven.plugin.PluginResolutionException: Plugin org.openhab.tools:static-code-analysis:0.0.2 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar
        at org.apache.maven.plugin.internal.DefaultPluginDependenciesResolver.resolveInternal(DefaultPluginDependenciesResolver.java:218)
        at org.apache.maven.plugin.internal.DefaultPluginDependenciesResolver.resolve(DefaultPluginDependenciesResolver.java:149)
        at org.apache.maven.plugin.internal.DefaultMavenPluginManager.createPluginRealm(DefaultMavenPluginManager.java:400)
        at org.apache.maven.plugin.internal.DefaultMavenPluginManager.setupPluginRealm(DefaultMavenPluginManager.java:372)
        at org.apache.maven.plugin.DefaultBuildPluginManager.getPluginRealm(DefaultBuildPluginManager.java:231)
        at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:102)
        ... 21 more
Caused by: org.eclipse.aether.resolution.ArtifactResolutionException: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:444)
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:246)
        at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies(DefaultRepositorySystem.java:367)
        at org.apache.maven.plugin.internal.DefaultPluginDependenciesResolver.resolveInternal(DefaultPluginDependenciesResolver.java:210)
        ... 26 more
Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact com.sun:tools:jar:1.8.0 at specified path C:\jre1.8.0_92/../lib/tools.jar
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:286)
        ... 29 more

Any idea ?

Kai · March 20, 2017, 7:30pm

No idea, really. Are you maybe using a JRE instead of a JDK? Check your JAVA_HOME setting.

J-N-K · March 20, 2017, 7:55pm

I had the same problem and my JAVA_HOME indeed pointed to JRE instead of JDK. After changing that it worked.

opus · March 20, 2017, 9:31pm

…and if you have it running and the tool doesn’t find a problem, the html isn’t written to the root directory of the binding.

Lolodomo · March 22, 2017, 6:56pm

Thank you guys, as you guessed, my problem was that JAVA_HOME was pointing to JRE and not JDK.
Running the static code analysis on my powermax binding, I got 31 remarks; 0 high and only 3 medium. 12 of 31 are “AvoidInstantiatingObjectsInLoops”.
Will try to fix that.

Lolodomo · March 23, 2017, 12:14am

I have several AvoidInstantiatingObjectsInLoops
https://pmd.github.io/pmd-5.5.1/pmd-java/rules/java/optimizations.html#AvoidInstantiatingObjectsInLoops
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 · March 23, 2017, 1:37am

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?

svilenvul · March 23, 2017, 8:16am

Yes, I think we can exclude it as well.
I have opened an issue for the removal - https://github.com/openhab/static-code-analysis/issues/29