From f2a7179af0c575c223b302c1db7683f91ea9ee8f Mon Sep 17 00:00:00 2001 From: Svilen Valkanov Date: Mon, 4 Sep 2017 13:39:51 +0300 Subject: [PATCH 1/5] Static analysis added to ESH build Signed-off-by: Svilen Valkanov --- .travis.yml | 2 +- .../smarthome/core/items/ItemUtil.java | 2 +- docs/documentation/development/guidelines.md | 8 ++ pom.xml | 115 ++++++++++++++++++ .../checkstyle/ruleset.properties | 3 + .../checkstyle/suppressions.xml | 22 ++++ .../findbugs/suppressions.xml | 32 +++++ 7 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 tools/static-code-analysis/checkstyle/ruleset.properties create mode 100644 tools/static-code-analysis/checkstyle/suppressions.xml create mode 100644 tools/static-code-analysis/findbugs/suppressions.xml diff --git a/.travis.yml b/.travis.yml index 000752c488a..d12511dcbde 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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 diff --git a/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/ItemUtil.java b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/ItemUtil.java index 64332e56413..d2598cf122d 100644 --- a/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/ItemUtil.java +++ b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/ItemUtil.java @@ -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; } diff --git a/docs/documentation/development/guidelines.md b/docs/documentation/development/guidelines.md index 0d88525a3f5..a0e93c79d15 100644 --- a/docs/documentation/development/guidelines.md +++ b/docs/documentation/development/guidelines.md @@ -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 idividual report for each binding that you can find in `../your_binding_directory/target/code-analysis/report.html` file and a report for the whole build that contains links to the individual reports in the `../smarthome/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 Prioriry 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. diff --git a/pom.xml b/pom.xml index 5ab6b721958..93cc2ce5c77 100644 --- a/pom.xml +++ b/pom.xml @@ -53,6 +53,7 @@ 2.1.0 1.8 UTF-8 + 0.3.0 pom @@ -469,6 +470,120 @@ + + + check + + + !skipChecks + + + + + + + org.openhab.tools + static-code-analysis + ${sat.version} + + + verify + + checkstyle + pmd + findbugs + report + + + + + tools/static-code-analysis/checkstyle/ruleset.properties + tools/static-code-analysis/checkstyle/suppressions.xml + tools/static-code-analysis/findbugs/suppressions.xml + + + + + + + + check-bundles + + + META-INF/MANIFEST.MF + + + + + + org.openhab.tools + static-code-analysis + ${sat.version} + + + + + + check-bindings + + + ESH-INF/binding/binding.xml + + + + + + org.openhab.tools + static-code-analysis + ${sat.version} + + + + + + + tycho-snapshots + https://oss.sonatype.org/content/groups/public/ + + + cbi-releases + https://repo.eclipse.org/content/repositories/cbi-releases/ + + + buchen-maven-repo + http://buchen.github.io/maven-repo + default + + + openhab-artifactory-release + https://openhab.jfrog.io/openhab/libs-release + + + + + + eclipse + Eclipse Snapshot Repository + default + https://repo.eclipse.org/content/repositories/snapshots/ + + true + + + + openhab-artifactory-release + JFrog Artifactory Repository + https://openhab.jfrog.io/openhab/libs-release + + true + never + + + false + + + + diff --git a/tools/static-code-analysis/checkstyle/ruleset.properties b/tools/static-code-analysis/checkstyle/ruleset.properties new file mode 100644 index 00000000000..22398810e74 --- /dev/null +++ b/tools/static-code-analysis/checkstyle/ruleset.properties @@ -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 diff --git a/tools/static-code-analysis/checkstyle/suppressions.xml b/tools/static-code-analysis/checkstyle/suppressions.xml new file mode 100644 index 00000000000..9c3da8ae5c6 --- /dev/null +++ b/tools/static-code-analysis/checkstyle/suppressions.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tools/static-code-analysis/findbugs/suppressions.xml b/tools/static-code-analysis/findbugs/suppressions.xml new file mode 100644 index 00000000000..9c30533da52 --- /dev/null +++ b/tools/static-code-analysis/findbugs/suppressions.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 56385669017416168ea0cd46e670905f3ee9711c Mon Sep 17 00:00:00 2001 From: Svilen Valkanov Date: Mon, 4 Sep 2017 16:17:12 +0300 Subject: [PATCH 2/5] Remove unneeded repositories Signed-off-by: Svilen Valkanov --- pom.xml | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/pom.xml b/pom.xml index 93cc2ce5c77..a320ee87e6e 100644 --- a/pom.xml +++ b/pom.xml @@ -543,19 +543,6 @@ - - tycho-snapshots - https://oss.sonatype.org/content/groups/public/ - - - cbi-releases - https://repo.eclipse.org/content/repositories/cbi-releases/ - - - buchen-maven-repo - http://buchen.github.io/maven-repo - default - openhab-artifactory-release https://openhab.jfrog.io/openhab/libs-release @@ -563,15 +550,6 @@ - - eclipse - Eclipse Snapshot Repository - default - https://repo.eclipse.org/content/repositories/snapshots/ - - true - - openhab-artifactory-release JFrog Artifactory Repository From b2a8293fb9499e77871b5d23c6f5f18d6ce654b9 Mon Sep 17 00:00:00 2001 From: Svilen Valkanov Date: Mon, 4 Sep 2017 16:24:12 +0300 Subject: [PATCH 3/5] Remove unused profile Signed-off-by: Svilen Valkanov --- pom.xml | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/pom.xml b/pom.xml index a320ee87e6e..ad4274350c0 100644 --- a/pom.xml +++ b/pom.xml @@ -523,23 +523,6 @@ - - check-bindings - - - ESH-INF/binding/binding.xml - - - - - - org.openhab.tools - static-code-analysis - ${sat.version} - - - - From 8636a8ea2a7f16c13520917cc03d6c4a289ff101 Mon Sep 17 00:00:00 2001 From: Svilen Valkanov Date: Mon, 4 Sep 2017 16:29:10 +0300 Subject: [PATCH 4/5] Refine the documentation about the tool Signed-off-by: Svilen Valkanov --- docs/documentation/development/guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation/development/guidelines.md b/docs/documentation/development/guidelines.md index a0e93c79d15..ab8745449a8 100644 --- a/docs/documentation/development/guidelines.md +++ b/docs/documentation/development/guidelines.md @@ -73,6 +73,6 @@ Note that this list also serves as a checklist for code reviews on pull requests 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 idividual report for each binding that you can find in `../your_binding_directory/target/code-analysis/report.html` file and a report for the whole build that contains links to the individual reports in the `../smarthome/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 Prioriry 1 on the console. +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. From 12c5234c2facd58708adbf52998b48636bb2c5b6 Mon Sep 17 00:00:00 2001 From: Svilen Valkanov Date: Tue, 5 Sep 2017 14:48:10 +0300 Subject: [PATCH 5/5] Change to JCenter repository Signed-off-by: Svilen Valkanov --- pom.xml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index ad4274350c0..965927ee485 100644 --- a/pom.xml +++ b/pom.xml @@ -527,16 +527,16 @@ - openhab-artifactory-release - https://openhab.jfrog.io/openhab/libs-release + jcenter + https://jcenter.bintray.com - openhab-artifactory-release - JFrog Artifactory Repository - https://openhab.jfrog.io/openhab/libs-release + jcenter + JCenter Repository + https://jcenter.bintray.com true never