Merge pull request #1 from wborn/fix-UseSLF4JLoggerRule

Fix UseSLF4JLoggerRule
openhab · Mar 1, 2024 · 00b8771 · 00b8771
2 parents ae5f15d + 2e254de
commit 00b8771
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 19 deletions.
diff --git a/custom-checks/pmd/src/main/java/org/openhab/tools/analysis/pmd/UseSLF4JLoggerRule.java b/custom-checks/pmd/src/main/java/org/openhab/tools/analysis/pmd/UseSLF4JLoggerRule.java
@@ -15,13 +15,11 @@
 import java.util.HashSet;
 import java.util.Set;
 
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
 import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
 import net.sourceforge.pmd.lang.java.ast.ASTType;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol;
 
 /**
  * Checks if a logger other than the one provided by slf4j is used
@@ -41,6 +39,11 @@ public UseSLF4JLoggerRule() {
         forbiddenLoggers.add("org.apache.commons.logging.Log");
     }
 
+    @Override
+    public String getMessage() {
+        return "The org.slf4j Logger should be used";
+    }
+
     @Override
     public Object visit(ASTImportDeclaration node, Object data) {
         String fullImportName = node.getImportedName();
@@ -56,19 +59,12 @@ public Object visit(ASTImportDeclaration node, Object data) {
     @Override
     public Object visit(ASTVariableDeclarator node, Object data) {
         ASTType typeNode = node.getParent().firstChild(ASTType.class);
-        if (typeNode != null) {
-            // getChild(0) returns out of bounds if no child exists, getFirstChild returns null
-            Node reftypeNode = typeNode.getFirstChild();
-            if (reftypeNode instanceof ASTReferenceType) {
-                ASTClassOrInterfaceType classOrInterfaceType = reftypeNode.firstChild(ASTClassOrInterfaceType.class);
-                if (classOrInterfaceType != null) {
-                    // getImage will now return null, not sure if getSimpleName is the correct replacement
-                    String className = classOrInterfaceType.getSimpleName();
-
-                    if (isClassNameForbidden(className)) {
-                        asCtx(data).addViolation(typeNode);
-                    }
-                }
+        if (typeNode != null && typeNode.getTypeMirror().isClassOrInterface()) {
+            JTypeDeclSymbol symbol = typeNode.getTypeMirror().getSymbol();
+            String className = symbol.getPackageName().equals(symbol.getSimpleName()) ? symbol.getSimpleName()
+                    : symbol.getPackageName() + "." + symbol.getSimpleName();
+            if (isClassNameForbidden(className)) {
+                asCtx(data).addViolation(typeNode);
             }
         }
         return super.visit(node, data);
@@ -79,7 +75,7 @@ private boolean isClassNameForbidden(String className) {
             return true;
         }
         // If the className is Logger but org.slf4j is not in the imports,
-        // that means the current Logger literal is not a sfl4j.Logger
+        // that means the current Logger literal is not an org.slf4j.Logger
         return LOGGER_LITERAL.equals(className) && !isSlf4jPackageImported;
     }
 }
diff --git a/sat-plugin/src/main/resources/rulesets/pmd/customrules.xml b/sat-plugin/src/main/resources/rulesets/pmd/customrules.xml
@@ -2,8 +2,7 @@
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.net/ruleset_2_0_0.xsd">
 	<description>Rule set that contains custom rules created for checking all bundles</description>
-	<rule class="org.openhab.tools.analysis.pmd.UseSLF4JLoggerRule" name="UseSLF4J" language="java"
-		message="The org.slf4j Logger should be used">
+	<rule class="org.openhab.tools.analysis.pmd.UseSLF4JLoggerRule" name="UseSLF4J" language="java">
 		<priority>3</priority>
 	</rule>
 </ruleset>