Issue #12542: new TreeWalker property to skip exceptions

checkstyle · May 11, 2024 · f15fbed · f15fbed
1 parent c34d756
commit f15fbed
Show file tree

Hide file tree

Showing 7 changed files with 292 additions and 14 deletions.
diff --git a/config/ant-phase-verify.xml b/config/ant-phase-verify.xml
@@ -166,7 +166,8 @@
           <exclude name="**/InputMainFrameModelIncorrectClass.java"/>
           <exclude name="**/InputBeforeExecutionExclusionFileFilterIncorrectClass.java"/>
           <exclude name="**/InputJavaParser.java"/>
-
+          <exclude name="**/InputTreeWalkerSkipParsingException.java"/>
+          <exclude name="**/InputTreeWalkerSkipParsingExceptionConfig.java"/>
         </fileset>
       </path>
       <formatter type="plain"/>
@@ -203,6 +204,8 @@
           <exclude name="**/InputMainFrameModelIncorrectClass.java"/>
           <exclude name="**/InputBeforeExecutionExclusionFileFilterIncorrectClass.java"/>
           <exclude name="**/InputJavaParser.java"/>
+          <exclude name="**/InputTreeWalkerSkipParsingException.java"/>
+          <exclude name="**/InputTreeWalkerSkipParsingExceptionConfig.java"/>
           <!-- This check does not apply to grammar input files -->
           <exclude name="**/grammar/**/*"/>
           <!-- No need to check xpathmapper files -->

diff --git a/config/checker-framework-suppressions/checker-nullness-optional-interning-suppressions.xml b/config/checker-framework-suppressions/checker-nullness-optional-interning-suppressions.xml
@@ -851,6 +851,72 @@
     </details>
   </checkerFrameworkError>
 
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter args of Violation constructor.</message>
+    <lineContent>new Object[] {ex.getMessage()}, javaParseExceptionSeverity, null,</lineContent>
+    <details>
+      found   : @Initialized @Nullable Object @Initialized @NonNull []
+      required: @Initialized @NonNull Object @Initialized @NonNull []
+    </details>
+  </checkerFrameworkError>
+
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter ast of TreeWalker.walk.</message>
+    <lineContent>walk(rootAST, contents, AstState.ORDINARY);</lineContent>
+    <details>
+      found   : @Initialized @Nullable DetailAST
+      required: @Initialized @NonNull DetailAST
+    </details>
+  </checkerFrameworkError>
+
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter customMessage of Violation constructor.</message>
+    <lineContent>getClass(), getCustomMessages().get(EXCEPTION_MSG)));</lineContent>
+    <details>
+      found   : @Initialized @Nullable String
+      required: @Initialized @NonNull String
+    </details>
+  </checkerFrameworkError>
+
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter moduleId of Violation constructor.</message>
+    <lineContent>new Object[] {ex.getMessage()}, javaParseExceptionSeverity, null,</lineContent>
+    <details>
+      found   : null (NullType)
+      required: @Initialized @NonNull String
+    </details>
+  </checkerFrameworkError>
+
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter root of JavaParser.appendHiddenCommentNodes.</message>
+    <lineContent>final DetailAST astWithComments = JavaParser.appendHiddenCommentNodes(rootAST);</lineContent>
+    <details>
+      found   : @Initialized @Nullable DetailAST
+      required: @Initialized @NonNull DetailAST
+    </details>
+  </checkerFrameworkError>
+
+  <checkerFrameworkError unstable="false">
+    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter rootAST of TreeWalker.getFilteredViolations.</message>
+    <lineContent>getFilteredViolations(file.getAbsolutePath(), contents, rootAST);</lineContent>
+    <details>
+      found   : @Initialized @Nullable DetailAST
+      required: @Initialized @NonNull DetailAST
+    </details>
+  </checkerFrameworkError>
+
   <checkerFrameworkError unstable="false">
     <fileName>src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java</fileName>
     <specifier>initialization.fields.uninitialized</specifier>

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
@@ -42,6 +42,7 @@
 import com.puppycrawl.tools.checkstyle.api.ExternalResourceHolder;
 import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.FileText;
+import com.puppycrawl.tools.checkstyle.api.SeverityLevel;
 import com.puppycrawl.tools.checkstyle.api.Violation;
 import com.puppycrawl.tools.checkstyle.utils.TokenUtil;
 
@@ -53,6 +54,9 @@
 @FileStatefulCheck
 public final class TreeWalker extends AbstractFileSetCheck implements ExternalResourceHolder {
 
+    /** Message to use when an exception occurs and should be printed as a violation. */
+    public static final String EXCEPTION_MSG = "general.exception";
+
     /** Maps from token name to ordinary checks. */
     private final Map<Integer, Set<AbstractCheck>> tokenToOrdinaryChecks =
         new HashMap<>();
@@ -79,6 +83,12 @@ public final class TreeWalker extends AbstractFileSetCheck implements ExternalRe
     /** A factory for creating submodules (i.e. the Checks) */
     private ModuleFactory moduleFactory;
 
+    /** Control whether to skip files with Java parsing errors. */
+    private boolean skipFileOnJavaParseException;
+
+    /** Severity Level to log Java parsing exceptions when they are skipped. */
+    private SeverityLevel javaParseExceptionSeverity = SeverityLevel.ERROR;
+
     /**
      * Creates a new {@code TreeWalker} instance.
      */
@@ -95,6 +105,27 @@ public void setModuleFactory(ModuleFactory moduleFactory) {
         this.moduleFactory = moduleFactory;
     }
 
+    /**
+     * Setter to control whether to skip files with Java parsing errors.
+     *
+     *  @param skipFileOnJavaParseException whether to always check for a trailing comma.
+     *  @since 10.17.0
+     */
+    public void setSkipFileOnJavaParseException(boolean skipFileOnJavaParseException) {
+        this.skipFileOnJavaParseException = skipFileOnJavaParseException;
+    }
+
+    /**
+     * Sets severity level to log Java parsing exceptions when they are skipped.
+     *
+     *  @param javaParseExceptionSeverity severity level to log parsing exceptions
+     *                                when they are skipped.
+     *  @since 10.17.0
+     */
+    public void setJavaParseExceptionSeverity(SeverityLevel javaParseExceptionSeverity) {
+        this.javaParseExceptionSeverity = javaParseExceptionSeverity;
+    }
+
     @Override
     public void finishLocalSetup() {
         final DefaultContext checkContext = new DefaultContext();
@@ -149,21 +180,41 @@ protected void processFiltered(File file, FileText fileText) throws CheckstyleEx
         // check if already checked and passed the file
         if (!ordinaryChecks.isEmpty() || !commentChecks.isEmpty()) {
             final FileContents contents = getFileContents();
-            final DetailAST rootAST = JavaParser.parse(contents);
-            if (!ordinaryChecks.isEmpty()) {
-                walk(rootAST, contents, AstState.ORDINARY);
-            }
-            if (!commentChecks.isEmpty()) {
-                final DetailAST astWithComments = JavaParser.appendHiddenCommentNodes(rootAST);
-                walk(astWithComments, contents, AstState.WITH_COMMENTS);
+            DetailAST rootAST = null;
+            // whether skip the procedure after parsing Java files.
+            boolean skip = false;
+            try {
+                rootAST = JavaParser.parse(contents);
             }
-            if (filters.isEmpty()) {
-                addViolations(violations);
+            catch (CheckstyleException ex) {
+                if (skipFileOnJavaParseException) {
+                    skip = true;
+                    violations.add(new Violation(1, Definitions.CHECKSTYLE_BUNDLE, EXCEPTION_MSG,
+                            new Object[] {ex.getMessage()}, javaParseExceptionSeverity, null,
+                            getClass(), getCustomMessages().get(EXCEPTION_MSG)));
+                    addViolations(violations);
+                }
+                else {
+                    throw ex;
+                }
             }
-            else {
-                final SortedSet<Violation> filteredViolations =
-                    getFilteredViolations(file.getAbsolutePath(), contents, rootAST);
-                addViolations(filteredViolations);
+
+            if (!skip) {
+                if (!ordinaryChecks.isEmpty()) {
+                    walk(rootAST, contents, AstState.ORDINARY);
+                }
+                if (!commentChecks.isEmpty()) {
+                    final DetailAST astWithComments = JavaParser.appendHiddenCommentNodes(rootAST);
+                    walk(astWithComments, contents, AstState.WITH_COMMENTS);
+                }
+                if (filters.isEmpty()) {
+                    addViolations(violations);
+                }
+                else {
+                    final SortedSet<Violation> filteredViolations =
+                            getFilteredViolations(file.getAbsolutePath(), contents, rootAST);
+                    addViolations(filteredViolations);
+                }
             }
             violations.clear();
         }

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java
@@ -35,9 +35,12 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.UUID;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -56,7 +59,10 @@
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.FileText;
+import com.puppycrawl.tools.checkstyle.api.SeverityLevel;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import com.puppycrawl.tools.checkstyle.api.Violation;
+import com.puppycrawl.tools.checkstyle.checks.NoCodeInFileCheck;
 import com.puppycrawl.tools.checkstyle.checks.coding.EmptyStatementCheck;
 import com.puppycrawl.tools.checkstyle.checks.coding.HiddenFieldCheck;
 import com.puppycrawl.tools.checkstyle.checks.design.OneTopLevelClassCheck;
@@ -673,6 +679,129 @@ public void testOrderOfCheckExecution() throws Exception {
         }
     }
 
+    @Test
+    public void testSkipFileOnJavaParseExceptionTrue() throws Exception {
+        final DefaultConfiguration config = createModuleConfig(TreeWalker.class);
+        config.addProperty("skipFileOnJavaParseException", "true");
+        config.addProperty("javaParseExceptionSeverity", "ignore");
+        config.addChild(createModuleConfig(ConstantNameCheck.class));
+
+        final File[] files = {
+            new File(getNonCompilablePath("InputTreeWalkerSkipParsingException.java")),
+            new File(getPath("InputTreeWalkerProperFileExtension.java")),
+            new File(getNonCompilablePath("InputTreeWalkerSkipParsingException.java")),
+        };
+
+        final Checker checker = createChecker(config);
+        final Map<String, List<String>> expectedViolation = new HashMap<>();
+        expectedViolation.put(getPath("InputTreeWalkerProperFileExtension.java"),
+                Collections.singletonList(
+                        "10:27: " + getCheckMessage(ConstantNameCheck.class,
+                        MSG_INVALID_PATTERN, "k", "^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$")));
+        verify(checker, files, expectedViolation);
+    }
+
+    @Test
+    public void testSkipFileOnJavaParseExceptionFalse() throws Exception {
+        final DefaultConfiguration config = createModuleConfig(TreeWalker.class);
+        config.addProperty("skipFileOnJavaParseException", "false");
+        config.addChild(createModuleConfig(ConstantNameCheck.class));
+
+        final String[] files = {
+            getNonCompilablePath("InputTreeWalkerSkipParsingException.java"),
+            getPath("InputTreeWalkerProperFileExtension.java"),
+            getNonCompilablePath("InputTreeWalkerSkipParsingException.java"),
+        };
+        final Exception ex = TestUtil.getExpectedThrowable(CheckstyleException.class,
+                () -> execute(config, files),
+                "Exception is expected");
+        assertWithMessage("Error message is unexpected")
+                .that(ex.getMessage())
+                .contains("Exception was thrown while processing ");
+    }
+
+    @Test
+    public void testSkipFileOnJavaParseExceptionConfig() throws Exception {
+        final String path = getNonCompilablePath("InputTreeWalkerSkipParsingExceptionConfig.java");
+        final String[] expected = {};
+        verifyWithInlineXmlConfig(path, expected);
+    }
+
+    @Test
+    public void testSkipFileOnJavaParseExceptionSkipChecks() throws Exception {
+        final DefaultConfiguration config = createModuleConfig(TreeWalker.class);
+        config.addProperty("skipFileOnJavaParseException", "true");
+        config.addProperty("javaParseExceptionSeverity", "ignore");
+        config.addChild(createModuleConfig(NoCodeInFileCheck.class));
+
+        final Checker checker = createChecker(config);
+
+        final File[] files = {
+            new File(getNonCompilablePath("InputTreeWalkerSkipParsingException.java")),
+            new File(getPath("InputTreeWalkerProperFileExtension.java")),
+        };
+        final Map<String, List<String>> expectedViolation = new HashMap<>();
+        expectedViolation.put(getPath("InputTreeWalkerProperFileExtension.java"),
+                new ArrayList<>());
+
+        verify(checker, files, expectedViolation);
+    }
+
+    @Test
+    public void testJavaParseExceptionSeverityDefaultError() throws Exception {
+        final DefaultConfiguration config = createModuleConfig(TreeWalker.class);
+        config.addProperty("skipFileOnJavaParseException", "true");
+        config.addChild(createModuleConfig(NoCodeInFileCheck.class));
+
+        final Checker checker = createChecker(config);
+
+        final File[] files = {
+            new File(getNonCompilablePath("InputTreeWalkerSkipParsingException.java")),
+            new File(getPath("InputTreeWalkerProperFileExtension.java")),
+        };
+
+        final Map<String, List<String>> expectedViolation = new HashMap<>();
+        expectedViolation.put(getPath("InputTreeWalkerProperFileExtension.java"),
+                new ArrayList<>());
+        expectedViolation.put(getNonCompilablePath("InputTreeWalkerSkipParsingException.java"),
+                List.of("1: Got an exception - IllegalStateException occurred while parsing file "
+                        + getNonCompilablePath("InputTreeWalkerSkipParsingException.java") + "."));
+
+        verify(checker, files, expectedViolation);
+    }
+
+    @Test
+    public void testJavaParseExceptionSeverity() throws Exception {
+        final PackageObjectFactory factory = new PackageObjectFactory(
+                new HashSet<>(), Thread.currentThread().getContextClassLoader());
+        final File testFile = new File(
+                getNonCompilablePath("InputTreeWalkerSkipParsingException.java"));
+
+        for (SeverityLevel severityLevel : SeverityLevel.values()) {
+            final DefaultConfiguration config = createModuleConfig(TreeWalker.class);
+            config.addProperty("skipFileOnJavaParseException", "true");
+            config.addProperty("javaParseExceptionSeverity", severityLevel.getName());
+            config.addChild(createModuleConfig(NoCodeInFileCheck.class));
+
+            final TreeWalker treeWalker = new TreeWalker();
+            treeWalker.setModuleFactory(factory);
+            treeWalker.configure(config);
+
+            final SortedSet<Violation> violations = treeWalker.process(testFile,
+                    new FileText(testFile, StandardCharsets.UTF_8.name()));
+            assertWithMessage("Incorrect length of violation")
+                    .that(violations.size())
+                    .isEqualTo(1);
+            assertWithMessage("Incorrect severity of violation")
+                    .that(violations.first().getSeverityLevel())
+                    .isEqualTo(severityLevel);
+            assertWithMessage("Incorrect customized message")
+                    .that(violations.first().getViolation())
+                    .contains("Got an exception - "
+                            + "IllegalStateException occurred while parsing file ");
+        }
+    }
+
     public static class BadJavaDocCheck extends AbstractCheck {
 
         @Override

diff --git a/...lable/com/puppycrawl/tools/checkstyle/treewalker/InputTreeWalkerSkipParsingException.java b/...lable/com/puppycrawl/tools/checkstyle/treewalker/InputTreeWalkerSkipParsingException.java
@@ -0,0 +1,3 @@
+//non-compiled syntax: bad file for testing
+
+public clazz InputTreeWalkerSkipParsingException {}
diff --git a/...com/puppycrawl/tools/checkstyle/treewalker/InputTreeWalkerSkipParsingExceptionConfig.java b/...com/puppycrawl/tools/checkstyle/treewalker/InputTreeWalkerSkipParsingExceptionConfig.java
@@ -0,0 +1,12 @@
+/*xml
+<module name="Checker">
+  <module name="TreeWalker">
+    <property name="skipFileOnJavaParseException" value="true"/>
+    <property name="javaParseExceptionSeverity" value="ignore"/>
+    <module name="ConstantName"/>
+  </module>
+</module>
+*/
+
+//non-compiled syntax: bad file for testing
+public clazz InputTreeWalkerSkipParsingExceptionConfig {}
diff --git a/src/xdocs/config.xml b/src/xdocs/config.xml
@@ -539,6 +539,20 @@
               <td><code>.java</code></td>
               <td>3.0</td>
             </tr>
+            <tr>
+              <td>javaParseExceptionSeverity</td>
+              <td>Severity level to log Java parsing exceptions when they are skipped.</td>
+              <td><a href="property_types.html#SeverityLevel">SeverityLevel</a></td>
+              <td><code>error</code></td>
+              <td>10.17.0</td>
+            </tr>
+            <tr>
+              <td>skipFileOnJavaParseException</td>
+              <td>Control whether to skip files with Java parsing errors.</td>
+              <td><a href="property_types.html#boolean">boolean</a></td>
+              <td><code>false</code></td>
+              <td>10.17.0</td>
+            </tr>
           </table>
         </div>
       </subsection>