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 ?
You need to be on the latest master - make sure to have your branch rebased on it.
@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 ?
No idea, really. Are you maybe using a JRE instead of a JDK? Check your JAVA_HOME setting.
I had the same problem and my JAVA_HOME indeed pointed to JRE instead of JDK. After changing that it worked.
…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.
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.
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 ?
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?
1 Like
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
It’s a great step forward to start using something like this 
Is there a specific reason for NewlineAtEndOfFileCheck: File does not end with a newline. I never worked at a company who enforced this and see no direct reason for it.
PS: I’ll make a PR for the PMD/checkstyle errors in RFXCom
Afaik, the NewlineAtEndOfFileCheck is useful for getting proper diffs for changes on Github, but I cannot tell you the exact problem, if this is missing (@maggu2810, afair, you know more about that?).
1 Like
POSIX defines a “Line” this way:
A sequence of zero or more non- characters plus a terminating character.
If you are using tools that rely on POSIX this tools could rely that a newline is the last character of a line and so the last character of a file.
If you use the diff function of Git, you will see that a missing newline at the end of a file (so a file that has an unfinished line) could result in a removed and added last “(incomplete) line” (also if all characters are equal).
Ok, thanks for the heads up.
Maybe I also have a suggestion for another rule, I once had the review feedback that I miss capitalised openHAB and saw it as feedback in another review request to0.
So we could validate it to be proper in:
- md files
- MANIFEST.MF
- in XML texts and comments
- in java comments and strings
If you find it useful I can try to work it out based on the existing code/configuration we have in place.
Would it possible to have a global report for all bindings that will include summary messayes and maybe summary files ?
It will help to check how we improve the code and have a global view on what rules are less respected.
You mean like the summary report, but with the difference that this should also have a table of how many checks have failed? I fear that this probably isn’t easily possible, but @svilenvul might the better person to comment this.
I was thinking about having the table named “Summary Messages” globally for all bindings. It will give us globally the number of failing checks by rule.
Currently, to see if we have globally a lot of failing check “AvoidInstantiatingObjectsInLoops”, we have to open the report of each binding and count manually.
Maybe we could consider using a tool like SonarQube, it’s better in providing overviews like this. And also allows for the additional rules to be used, because SonarQube combines results of checkstyle, PMD and others in clean browseable overview.
I ran a SonarQube scan and there appear some extra things which are not yet handled by our current analyzer:
- OverrideBothEqualsAndHashcode (originates from PMD) real bug which bites when you when you are working for example hashmaps [1 time only]
- Incorrect use of loggers (formatting manually or calling methods as argument to debug statement eg
logger.debug("Log this: {}", object.toString())) [263 times] - Commented out code [93 times]
- Magic numbers, what does this 5 mean number of attempts, is it an upper of lower boundary see
- Reorder the modifiers to comply with the Java Language Specification [1554 times]
- When handling a caught exception, the original exception’s message and stack trace should be logged or passed forward. [577 times]
- Strings literals should be placed on the left side when checking for equality (to overcome possible null pointers) [399 times]
- “InterruptedException” should not be ignored [66 times]
- Control flow statements “if”, “for”, “while”, “switch” and “try” should not be nested too deeply [330 times]
- The members of an interface declaration or class should appear in a pre-defined order constructor should come for other member methods and after the fields [124 times]
SonarQube finds:
2.1k (possible) bugs including things like null pointers likely to occur, if’s that are always true, method arguments that are overwritten in a method.
704 vulnerabilities things which could lead to certain security problems like possible hardcoded passwords (most likely hardcoded by the vendors). Classes with public accessible fields etc.
5.4k code smells, things which might suggest there is something wrong with the code. Like duplications, non-standard order of methods or declarations (eg. private static final) non standard naming of fields like starting with _ or having lowercase constants or uppercase fields.
See my comment above regarding the use of SonarQube.
The main reason of this tool here is to support (me) on code reviews. It should therefore focus on finding real issues and not list hundreds of potential problems.
So my intention is to start with a small set of checks and increase it over time with the things we think are useful for us.
Yes, I saw that was already mentioned. The reason I mentioned it (again) is that solves the overview problem.
Besides that I mentioned a list of things which I always (try to) address in a code review. I do however agree that some of them are more suited for a professional environment like the order of modifiers. But with an easy fix lowering the chance of a NPE to occur is imho always worth it.
Are you aware that you can also start out with sonar with just eg. 5 rules enabled (its called a quality profile) and still profit from the overview.
If you would like I can try to create a sonar quality profile which covers the things we cover right now. Working with really custom things we have right now might be some additional work but is not impossible.