Issue #12542: new TreeWalker property to skip java parsing exceptions#15204
Conversation
d0da0a3 to
fd95f44
Compare
fd95f44 to
67a474a
Compare
|
github, generate report |
|
Report generation failed on phase "make_report", |
7a71d89 to
8a12016
Compare
|
There is an issue with circleci failures is unrelated |
|
github, generate report |
|
github, generate site |
|
github, generate report |
|
property is on with default severity: |
|
github, generate report |
|
property is on with severity ignore: |
|
Site: Reports:
|
|
github, generate report |
|
Report generation failed on phase "make_report", |
|
I was trying to generate the report with HOE as true but the report generation failed
So I think there is no way to test HOE true with regression because it will halt will generating the report? |
Yes, this property was basically created for regression, so it should not be modified. |
|
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/8a12016_2024140116/reports/diff/project.html |
|
Regression provided looks good to me. |
I thought that one project for this kind of updates is enough. I chose jdk21 as it has all forms of non-compilable Java code legacy syntax and new syntax. |
8a12016 to
be5710d
Compare
| } | ||
|
|
||
| /** | ||
| * {@inheritDoc} Processes the file. |
There was a problem hiding this comment.
Why is more needed than the inherited doc ? I don't see us needing the sentence.
There was a problem hiding this comment.
there was a tool complained about {@inheritDoc} only. I don't remember what was it I think inspection
There was a problem hiding this comment.
This seems like a false violation since we added the suppressions. I don't know the intellij enough to know if we can suppress this without a javadoc. Please restore the sentence.
|
I am good with regression. |
2beb9f0 to
d7f01d4
Compare
| Please recheck that class name is specified as canonical name or read how to configure \ | ||
| short name usage https://checkstyle.org/config.html\#Packages. Please also \ | ||
| recheck that provided ClassLoader to Checker is configured correctly. | ||
| parse.exception=TreeWalker is skipped due to a parse exception - {0} |
There was a problem hiding this comment.
User set skipFileOnJavaParseException but we tell him about Treewalker.
Something is mismatching.
There was a problem hiding this comment.
We need something like:
Java specific module are skipped due to parse exception
In this case user clearly match what he activated with new behavior
d7f01d4 to
7f252e6
Compare
romani
left a comment
There was a problem hiding this comment.
Ok to merge.
@nrmancuso , please review message in case it should be improved
nrmancuso
left a comment
There was a problem hiding this comment.
Here’s my recommendation:
| Please recheck that class name is specified as canonical name or read how to configure \ | ||
| short name usage https://checkstyle.org/config.html\#Packages. Please also \ | ||
| recheck that provided ClassLoader to Checker is configured correctly. | ||
| parse.exception=Java specific modules are skipped due to a parse exception - {0} |
There was a problem hiding this comment.
| parse.exception=Java specific modules are skipped due to a parse exception - {0} | |
| parse.exception= Java specific (TreeWalker-based) modules are skipped due to an exception during parsing - {0} |
There was a problem hiding this comment.
We should keep some tie to TreeWalker in the message to make it clear that it happened in some child of TreeWalker. Also, I would prefer “during parsing” since this could also be an exception thrown in the JavaAstVisitor, right?
There was a problem hiding this comment.
we don't add a space after = for .properties file I think we should leave it as it is for consistency
There was a problem hiding this comment.
I am good.
I just need keep few words: Java, Parse, Exception . To let users easily match it to skipFileOnJavaParseException
eb6b022 to
781b0b1
Compare
|
can you help me with this I don't understand this failure it didn't happen before https://www.jetbrains.com/help/inspectopedia/InconsistentResourceBundle.html looks like a false positive |
781b0b1 to
84c2f2d
Compare
closes #12542:
Diff Regression config: https://gist.githubusercontent.com/mahfouz72/afd57aa6e51ca161b7927acd959e5211/raw/a0240a31e088cbf55633a71dca58a46fb44c85e4/check.xml
Diff Regression patch config: https://gist.githubusercontent.com/mahfouz72/33967bf86e4a29cafa429771b0e95808/raw/eeb0cbe545525dc5fd04943dcb137d7f175838e4/check_patch.xml
Diff Regression projects: https://gist.githubusercontent.com/mahfouz72/a3d0af030c8f5efd0d8a39f2c14750bc/raw/a3424ad9b6f722e5de1a927e3d85e383101b2b93/projects-to-test-on.properties
Report label: property is on with severity ignore