Issue #12542: new TreeWalker property to skip exceptions #14779
+292
−14
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(); | ||
| DetailAST rootAST = null; | ||
| // whether skip the procedure after parsing Java files. | ||
| boolean skip = false; | ||
| try { | ||
| rootAST = JavaParser.parse(contents); | ||
rnveach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| catch (CheckstyleException ex) { | ||
|
There was a problem hiding this comment. This report shows we can get parse errors from more than just CheckstyleExceptions. It seems to me we need to expand it to all throwables for this to work properly. Edit: We will also need a test case to show this. Let me know what the new test name will be. There was a problem hiding this comment. @Lmh-java , please add this file to our Inputs https://github.com/openjdk/jdk/blob/master/test/langtools/tools/javac/diags/examples/VarargsAndReceiver.java without header comment. |
||
| if (skipFileOnJavaParseException) { | ||
| skip = true; | ||
| violations.add(new Violation(1, Definitions.CHECKSTYLE_BUNDLE, EXCEPTION_MSG, | ||
| new Object[] {ex.getMessage()}, javaParseExceptionSeverity, null, | ||
| getClass(), null)); | ||
| addViolations(violations); | ||
| } | ||
| else { | ||
| throw ex; | ||
| } | ||
rnveach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| 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(); | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| //non-compiled syntax: bad file for testing | ||
|
|
||
| public clazz InputTreeWalkerSkipParsingException {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /*xml | ||
| <module name="Checker"> | ||
| <module name="TreeWalker"> | ||
| <property name="skipFileOnJavaParseException" value="true"/> | ||
| <property name="javaParseExceptionSeverity" value="ignore"/> | ||
| <module name="ConstantName"/> | ||
romani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| </module> | ||
| </module> | ||
| */ | ||
|
|
||
| //non-compiled syntax: bad file for testing | ||
| public clazz InputTreeWalkerSkipParsingExceptionConfig {} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=>
Setter to specify the severity level...