Skip to content
Permalink
Browse files

Add Static code analysis tool to ESH build (#3995)

* Static analysis added to ESH build

Signed-off-by: Svilen Valkanov <svilen.valkanov@musala.com>
  • Loading branch information...
svilenvul authored and kaikreuzer committed Sep 5, 2017
1 parent 89a78cb commit b0812c2e760d9b25533c55d38c0f14dd1c6cab3f
@@ -9,7 +9,7 @@ before_install: echo "MAVEN_OPTS='-Xms1g -Xmx2g -XX:PermSize=512m -XX:MaxPermSiz
#$ mvn test -B

install:
- echo 'mvn clean install -B -V 1> .build.stdout 2> .build.stderr' > .build.sh
- echo 'mvn clean install -B -V -Dfindbugs.skip=true 1> .build.stdout 2> .build.stderr' > .build.sh
- chmod 0755 .build.sh
script:
- travis_wait 60 ./.build.sh
@@ -78,7 +78,7 @@ public static void assertValidItemName(String itemName) throws IllegalArgumentEx

public static State convertToAcceptedState(State state, Item item) {
if (state == null) {
LOGGER.error("A conversion of null was requested", new NullPointerException("state should not be null"));
LOGGER.error("A conversion of null was requested", new NullPointerException("state should not be null")); // NOPMD
return UnDefType.NULL;
}

@@ -68,3 +68,11 @@ Note that this list also serves as a checklist for code reviews on pull requests
- ```error``` logging should only be used to inform the user that something is tremendously wrong in his setup, the system cannot function normally anymore, and there is a need for immediate action. It should also be used if some code fails irrecoverably and the user should report it as a severe bug.
1. For bindings, you should NOT log errors, if e.g. connections are dropped - this is considered to be an external problem and from a system perspective to be a normal and expected situation. The correct way to inform users about such events is to update the Thing status accordingly. Note that all events (including Thing status events) are anyhow already logged.
1. Likewise, bundles that accept external requests (such as servlets) must not log errors or warnings if incoming requests are incorrect. Instead, appropriate error responses should be returned to the caller.

## Static code analysis

The Eclipse SmartHome Maven build includes [tooling for static code analysis](https://github.com/openhab/static-code-analysis) that will validate your code against the Coding Guidelines and some additional best practices. Information about the checks can be found [here](https://github.com/openhab/static-code-analysis/blob/master/docs/included-checks.md).

The tool will generate an individual report for each bundle that you can find in `path/to/bundle/target/code-analysis/report.html` file and a report for the whole build that contains links to the individual reports in the `target/summary_report.html`. The tool categorizes the found issues by priority: 1(error),2(warning) or 3(info). If any error is found within your code the Maven build will end with failure. You will receive detailed information (path to the file, line and message) listing all problems with Priority 1 on the console.

Please fix all the Priority 1 issues and all issues with Priority 2 and 3 that are relevant (if you have any doubt don't hesitate to ask). Re-run the build to confirm that the checks are passing.
76 pom.xml
@@ -53,6 +53,7 @@
<jdt-annotations.version>2.1.0</jdt-annotations.version>
<build.helper.maven.plugin.version>1.8</build.helper.maven.plugin.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<sat.version>0.3.0</sat.version>
</properties>

<packaging>pom</packaging>
@@ -469,6 +470,81 @@
</plugins>
</build>
</profile>
<!-- static code analysis profiles -->
<profile>
<id>check</id>
<activation>
<property>
<name>!skipChecks</name>
</property>
</activation>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.openhab.tools</groupId>
<artifactId>static-code-analysis</artifactId>
<version>${sat.version}</version>
<executions>
<execution>
<phase>verify</phase>
<goals>
<goal>checkstyle</goal>
<goal>pmd</goal>
<goal>findbugs</goal>
<goal>report</goal>
</goals>
</execution>
</executions>
<configuration>
<checkstyleProperties>tools/static-code-analysis/checkstyle/ruleset.properties</checkstyleProperties>
<checkstyleFilter>tools/static-code-analysis/checkstyle/suppressions.xml</checkstyleFilter>
<findbugsExclude>tools/static-code-analysis/findbugs/suppressions.xml</findbugsExclude>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
<profile>
<id>check-bundles</id>
<activation>
<file>
<exists>META-INF/MANIFEST.MF</exists>
</file>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.openhab.tools</groupId>
<artifactId>static-code-analysis</artifactId>
<version>${sat.version}</version>
</plugin>
</plugins>
</build>
</profile>
</profiles>

<pluginRepositories>
<pluginRepository>
<id>jcenter</id>
<url>https://jcenter.bintray.com</url>
</pluginRepository>
</pluginRepositories>

<repositories>
<repository>
<id>jcenter</id>
<name>JCenter Repository</name>
<url>https://jcenter.bintray.com</url>
<releases>
<enabled>true</enabled>
<updatePolicy>never</updatePolicy>
</releases>
<snapshots>
<enabled>false</enabled>
</snapshots>
</repository>
</repositories>

</project>
@@ -0,0 +1,3 @@
checkstyle.aboutHtmlCheck.url=https://eclipse.org/legal/epl/about.html
checkstyle.headerCheck.content=^/\\*\\*$\\n^ \\* Copyright \\(c\\) \\d\\d\\d\\d-\\d\\d\\d\\d by the respective copyright holders\\.$\\n^ \\* All rights reserved\\. This program and the accompanying materials$\\n^ \\* are made available under the terms of the Eclipse Public License v1\\.0$\\n^ \\* which accompanies this distribution, and is available at$\\n^ \\* http://www.eclipse.org/legal/epl\\-v10\\.html$
checkstyle.bundleVendorCheck.allowedValues=Eclipse.org/SmartHome
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suppressions PUBLIC
"-//Puppy Crawl//DTD Suppressions 1.1//EN"
"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">

<suppressions>
<!-- These suppressions define which files to be suppressed for which checks. -->
<suppress files=".+[\\/]internal[\\/].+\.java" checks="JavadocType|JavadocVariable|JavadocMethod|JavadocFilterCheck"/>
<suppress files=".+DTO\.java" checks="JavadocType|JavadocVariable|JavadocMethod|JavadocFilterCheck" />
<suppress files=".+Impl\.java" checks="JavadocType|JavadocVariable|JavadocMethod|JavadocFilterCheck"/>
<!-- All generated files will skip the author tag check -->
<suppress files=".+[\\/]gen[\\/].+\.java" checks="AuthorTagCheck"/>
<!-- Some checks will be supressed for test bundles -->
<suppress files=".+.test[\\/].+" checks="RequireBundleCheck|OutsideOfLibExternalLibrariesCheck|ManifestExternalLibrariesCheck|BuildPropertiesExternalLibrariesCheck"/>

<!-- Eclipse SmartHome specific suppressions-->
<!-- These bundles are generated trough XText -->
<suppress files=".+org.eclipse.smarthome.model.+|.+org.eclipse.smarthome.designer.+" checks="RequireBundleCheck|ExportInternalPackageCheck|ManifestPackageVersionCheck|ImportExportedPackagesCheck"/>
<!-- Some source files have different headers -->
<suppress files=".+org.eclipse.smarthome.automation.+" checks="HeaderCheck"/>
<suppress files=".+org.eclipse.smarthome.config.dispatch.test.+" checks="ServiceComponentManifestCheck"/>
</suppressions>
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<FindBugsFilter>
<!-- Groovy files produce a lot of warnings and will be ignored -->
<Match>
<Source name="~.*\.groovy" />
</Match>
<!-- Excludes all bugs with priority higher than 4 -->
<Match>
<Rank value="4"/>
<Not>
<Bug pattern="SLF4J_LOGGER_SHOULD_BE_NON_STATIC"/>
</Not>
</Match>
<!-- This pattern is not wanted as it reports usage of Throwable.getMessage() as argument to SLF4G logger -->
<Match>
<Bug pattern="SLF4J_MANUALLY_PROVIDED_MESSAGE"/>
</Match>
<!-- Allow util classes to have static loggers -->
<Match>
<Class name="~.*Utils"/>
<Bug pattern="SLF4J_LOGGER_SHOULD_BE_NON_STATIC"/>
</Match>
<Match>
<Class name="~.*Util"/>
<Bug pattern="SLF4J_LOGGER_SHOULD_BE_NON_STATIC"/>
</Match>
<!-- The format string is parameter, it can't be constant -->
<Match>
<Class name="org.eclipse.smarthome.model.script.actions.LogAction"/>
<Bug pattern="SLF4J_FORMAT_SHOULD_BE_CONST"/>
</Match>
</FindBugsFilter>

0 comments on commit b0812c2

Please sign in to comment.
You can’t perform that action at this time.