Upgrade Checkstyle to 14.3.0, PMD to 7.28.0, SpotBugs to 4.10.5

custom-checks/checkstyle/src/main/java/org/openhab/tools/analysis/checkstyle/MissingJavadocFilterCheck.java

@@ -12,34 +12,143 @@
  */
 package org.openhab.tools.analysis.checkstyle;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import com.puppycrawl.tools.checkstyle.FileStatefulCheck;
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.Scope;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
-import com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck;
+import com.puppycrawl.tools.checkstyle.utils.AnnotationUtil;
+import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
+import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;
+import com.puppycrawl.tools.checkstyle.utils.ScopeUtil;
 
 /**
  * Provides a filter that determines whether to check the inner units for a
  * javadoc.
  *
+ * Based on Checkstyle's MissingJavadocTypeCheck, which cannot be subclassed
+ * anymore since Checkstyle 14.
+ *
  * @author Petar Valchev - Initial contribution
  */
-public class MissingJavadocFilterCheck extends MissingJavadocTypeCheck {
+@FileStatefulCheck
+public class MissingJavadocFilterCheck extends AbstractCheck {
+
+    public static final String MSG_JAVADOC_MISSING = "javadoc.missing";
+
+    private final List<DetailAST> javadocComments = new ArrayList<>();
+
     private boolean checkInnerUnits = false;
+    private Scope scope = Scope.PUBLIC;
+    private Scope excludeScope;
+    private Set<String> skipAnnotations = Set.of("Generated");
 
     // A configuration property that determines whether to check the inner units
     public void setCheckInnerUnits(boolean checkInnerUnits) {
         this.checkInnerUnits = checkInnerUnits;
     }
 
+    public void setScope(Scope scope) {
+        this.scope = scope;
+    }
+
+    public void setExcludeScope(Scope excludeScope) {
+        this.excludeScope = excludeScope;
+    }
+
+    public void setSkipAnnotations(String... userAnnotations) {
+        skipAnnotations = Set.of(userAnnotations);
+    }
+
+    @Override
+    public boolean isCommentNodesRequired() {
+        return true;
+    }
+
+    @Override
+    public int[] getDefaultTokens() {
+        return getAcceptableTokens();
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return new int[] { TokenTypes.INTERFACE_DEF, TokenTypes.CLASS_DEF, TokenTypes.ENUM_DEF,
+                TokenTypes.ANNOTATION_DEF, TokenTypes.RECORD_DEF };
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return CommonUtil.EMPTY_INT_ARRAY;
+    }
+
+    @Override
+    public void beginTree(DetailAST rootAST) {
+        javadocComments.clear();
+        collectJavadocComments(rootAST);
+    }
+
     @Override
     public void visitToken(DetailAST ast) {
         if (!checkInnerUnits) {
             DetailAST astParent = ast.getParent();
-            // if outer class/interface/enum
-            if (astParent == null || astParent.getType() == TokenTypes.COMPILATION_UNIT) {
-                super.visitToken(ast);
+            // if not outer class/interface/enum
+            if (astParent != null && astParent.getType() != TokenTypes.COMPILATION_UNIT) {
+                return;
+            }
+        }
+        if (shouldCheck(ast) && !hasJavadoc(ast)) {
+            log(ast, MSG_JAVADOC_MISSING);
+        }
+    }
+
+    private void collectJavadocComments(DetailAST rootAST) {
+        DetailAST current = rootAST;
+        while (current != null) {
+            if (current.getType() == TokenTypes.BLOCK_COMMENT_BEGIN && JavadocUtil.isJavadocComment(current)) {
+                javadocComments.add(current);
+            }
+            if (current.getFirstChild() != null) {
+                current = current.getFirstChild();
+            } else {
+                DetailAST parent = current;
+                while (parent != null && current.getNextSibling() == null) {
+                    current = parent;
+                    parent = parent.getParent();
+                }
+                current = current.getNextSibling();
+            }
+        }
+    }
+
+    private boolean shouldCheck(DetailAST ast) {
+        Scope surroundingScope = ScopeUtil.getSurroundingScope(ast).orElse(null);
+        return surroundingScope != null && surroundingScope.isIn(scope)
+                && (excludeScope == null || !surroundingScope.isIn(excludeScope))
+                && !AnnotationUtil.containsAnnotation(ast, skipAnnotations);
+    }
+
+    private boolean hasJavadoc(DetailAST ast) {
+        DetailAST closestJavadoc = null;
+        for (DetailAST comment : javadocComments) {
+            if (comment.getLineNo() <= ast.getLineNo()) {
+                closestJavadoc = comment;
+            }
+        }
+        return closestJavadoc != null && noInterveningCode(closestJavadoc, ast);
+    }
+
+    private static boolean noInterveningCode(DetailAST javadoc, DetailAST type) {
+        int typeStartLine = type.getLineNo();
+        for (DetailAST node = javadoc; node != null; node = node.getNextSibling()) {
+            if (node.getLineNo() < typeStartLine && node.getType() != TokenTypes.BLOCK_COMMENT_BEGIN
+                    && node.getType() != TokenTypes.SINGLE_LINE_COMMENT) {
+                return false;
             }
-        } else {
-            super.visitToken(ast);
         }
+        return true;
     }
 }

custom-checks/checkstyle/src/main/java/org/openhab/tools/analysis/checkstyle/NoEmptyLineSeparatorCheck.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.tools.analysis.checkstyle;
 
+import com.puppycrawl.tools.checkstyle.FileStatefulCheck;
 import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
@@ -68,6 +69,7 @@
  * @author Svilen Valkanov - Initial contribution
  * @author Tanya Georgieva - Added check for switch statement without braces
  */
+@FileStatefulCheck
 public class NoEmptyLineSeparatorCheck extends AbstractCheck {
 
     private static final String MSG_LINE_AFTER_OPENING_BRACE_EMPTY = "Remove empty line after opening brace";
@@ -117,6 +119,14 @@ public int[] getRequiredTokens() {
 
     @Override
     public void visitToken(DetailAST ast) {
+        // the case group flags are set in findRightCurly when visiting a case group, and are reused by its case and
+        // default labels, which are visited right after it. They must not leak into any other block.
+        if (ast.getType() != TokenTypes.LITERAL_CASE && ast.getType() != TokenTypes.LITERAL_DEFAULT) {
+            isCaseGroupWithoutBraces = false;
+            isCaseGroupWithoutListOfStatements = false;
+            isCaseGroupWithBraces = false;
+        }
+
         DetailAST leftCurly = findLeftCurly(ast);
         DetailAST rightCurly = findRightCurly(ast);
 
@@ -275,6 +285,7 @@ private DetailAST findRightCurly(DetailAST ast) {
                      * }
                      */
                     isCaseGroupWithoutListOfStatements = true;
+                    isCaseGroupWithoutBraces = true;
                     return findRightCurlyInBlock(ast);
                 }
             }

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/AuthorContributionDescriptionCheckTest.java

@@ -46,7 +46,7 @@ public class AuthorContributionDescriptionCheckTest extends AbstractStaticCheckT
     private DefaultConfiguration configuration;
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/authorContributionDescriptionCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/AuthorTagCheckTest.java

@@ -17,6 +17,7 @@
 import org.openhab.tools.analysis.checkstyle.api.AbstractStaticCheckTest;
 
 import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+import com.puppycrawl.tools.checkstyle.checks.javadoc.WriteTagCheck;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 
 /**
@@ -29,7 +30,7 @@ public class AuthorTagCheckTest extends AbstractStaticCheckTest {
     private static final String EXPECTED_WARNING_MESSAGE = "An author tag is missing";
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/authorTagCheckTest";
     }
 
@@ -40,9 +41,9 @@ public void testOuterClassWithNoAuthorTag() throws Exception {
          * an error is expected at the line where the outer class is declared
          * in the file
          */
-        int warningLine = 4;
+        String warningLocation = "4:1";
         boolean checkInnerClasses = false;
-        checkFileForAuthorTags(checkInnerClasses, fileName, warningLine);
+        checkFileForAuthorTags(checkInnerClasses, fileName, warningLocation);
     }
 
     @Test
@@ -52,10 +53,10 @@ public void testOuterAndInnerClassesWithNoAuthorTag() throws Exception {
          * errors are expected at the lines where the classes are declared in
          * the file
          */
-        int firstWarningLine = 4;
-        int secondWarningLine = 9;
+        String firstWarningLocation = "4:1";
+        String secondWarningLocation = "9:9";
         boolean checkInnerClasses = true;
-        checkFileForAuthorTags(checkInnerClasses, fileName, firstWarningLine, secondWarningLine);
+        checkFileForAuthorTags(checkInnerClasses, fileName, firstWarningLocation, secondWarningLocation);
     }
 
     @Test
@@ -92,14 +93,14 @@ public void testOuterAndInnerClassesWithPresentAuthorTag() throws Exception {
         checkFileForAuthorTags(checkInnerClasses, fileName);
     }
 
-    private void checkFileForAuthorTags(boolean checkInnerUnits, String fileName, Integer... warningLine)
+    private void checkFileForAuthorTags(boolean checkInnerUnits, String fileName, String... warningLocation)
             throws Exception {
         String filePath = getPath(fileName);
         String[] expected;
-        if (warningLine.length > 0) {
-            expected = new String[warningLine.length];
-            for (int i = 0; i < warningLine.length; i++) {
-                expected[i] = warningLine[i] + ": " + EXPECTED_WARNING_MESSAGE;
+        if (warningLocation.length > 0) {
+            expected = new String[warningLocation.length];
+            for (int i = 0; i < warningLocation.length; i++) {
+                expected[i] = warningLocation[i] + ": " + EXPECTED_WARNING_MESSAGE;
             }
         } else {
             expected = CommonUtil.EMPTY_STRING_ARRAY;
@@ -118,9 +119,8 @@ private DefaultConfiguration createConfiguration(boolean checkInnerUnits) {
          */
         configuration.addProperty("tag", "@author");
         configuration.addProperty("tagFormat", "\\S");
-        configuration.addProperty("tagSeverity", "ignore");
         configuration.addProperty("checkInnerUnits", String.valueOf(checkInnerUnits));
-        configuration.addMessage("type.missingTag", EXPECTED_WARNING_MESSAGE);
+        configuration.addMessage(WriteTagCheck.MSG_MISSING_TAG, EXPECTED_WARNING_MESSAGE);
 
         return configuration;
     }

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/AvoidScheduleAtFixedRateCheckTest.java

@@ -31,7 +31,7 @@ public class AvoidScheduleAtFixedRateCheckTest extends AbstractStaticCheckTest {
     private final Configuration config = createModuleConfig(AvoidScheduleAtFixedRateCheck.class);
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/avoidScheduleAtFixedRateCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/DeclarativeServicesDependencyInjectionCheckTest.java

@@ -29,7 +29,7 @@ public class DeclarativeServicesDependencyInjectionCheckTest extends AbstractSta
     private static final String SERVICE_CUSTOMIZER_IMPLEMENTED = "Avoid using ServiceTrackerCustomizer for dependency injection, consider using Declarative Services";
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/declarativeServicesDependencyInjectionTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/ForbiddenPackageUsageCheckTest.java

@@ -36,7 +36,7 @@ public void setUp() {
     }
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/ForbiddenPackageUsageCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/InheritDocCheckTest.java

@@ -30,7 +30,7 @@ public class InheritDocCheckTest extends AbstractStaticCheckTest {
     Configuration config = createModuleConfig(InheritDocCheck.class);
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/inheritDocCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/JavadocMethodStyleCheckTest.java

@@ -33,7 +33,7 @@ public class JavadocMethodStyleCheckTest extends AbstractStaticCheckTest {
     private DefaultConfiguration configuration = createModuleConfig(JavadocMethodStyleCheck.class);
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/javadocMethodStyleCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/KarafAddonFeatureCheckTest.java

@@ -19,7 +19,7 @@
 import java.text.MessageFormat;
 
 import org.apache.commons.lang3.ArrayUtils;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 import org.openhab.tools.analysis.checkstyle.KarafAddonFeatureCheck;
 import org.openhab.tools.analysis.checkstyle.api.AbstractStaticCheckTest;
 
@@ -36,7 +36,7 @@ public class KarafAddonFeatureCheckTest extends AbstractStaticCheckTest {
     private static final DefaultConfiguration CONFIGURATION = createModuleConfig(KarafAddonFeatureCheck.class);
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/karafAddonFeatureCheck";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/KarafFeatureCheckTest.java

@@ -48,7 +48,7 @@ public static void setUp() {
     }
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/karafFeatureCheck";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/MarkdownCheckTest.java

@@ -39,7 +39,7 @@ public void setUpClass() {
     }
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/markdownCheckTest";
     }
 

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/MissingJavadocFilterCheckTest.java

@@ -17,7 +17,6 @@
 import org.openhab.tools.analysis.checkstyle.api.AbstractStaticCheckTest;
 
 import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
-import com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 
 /**
@@ -28,7 +27,7 @@
 public class MissingJavadocFilterCheckTest extends AbstractStaticCheckTest {
 
     @Override
-    protected String getPackageLocation() {
+    public String getPackageLocation() {
         return "checkstyle/missingJavadocFilterCheckTest";
     }
 
@@ -65,7 +64,7 @@ private void verifyJavadoc(String testFileName, boolean checkInnerClasses, Strin
         if (locations != null) {
             expectedMessages = new String[locations.length];
             for (int i = 0; i < locations.length; i++) {
-                expectedMessages[i] = locations[i] + ": " + MissingJavadocTypeCheck.MSG_JAVADOC_MISSING;
+                expectedMessages[i] = locations[i] + ": " + MissingJavadocFilterCheck.MSG_JAVADOC_MISSING;
             }
         } else {
             expectedMessages = CommonUtil.EMPTY_STRING_ARRAY;