Skip to content

bumpt checkstyle to 8.24; move checkstyle's LineLenght Check to Checker#245

Merged
jqno merged 1 commit into
jqno:masterfrom
checkstyle:issue_2116
Sep 15, 2019
Merged

bumpt checkstyle to 8.24; move checkstyle's LineLenght Check to Checker#245
jqno merged 1 commit into
jqno:masterfrom
checkstyle:issue_2116

Conversation

@romani

@romani romani commented Sep 15, 2019

Copy link
Copy Markdown
Contributor

What problem does this pull request solve?

upgrade to latest version of checkstyle to let keep using of this project in regression testing.

Please provide any additional information below.

migration of checkstyle config is provided.
unfortunate side effect of previous suppresion stop to work due to our redesign of Filters.
new SuppressWithPlainTextCommentFilter is provided to some sort of substitution of previous functionality.

@jqno jqno merged commit 92ad83c into jqno:master Sep 15, 2019
@jqno

jqno commented Sep 15, 2019

Copy link
Copy Markdown
Owner

Thank you for this!

Out of curiosity, why does the suppression not work the same way anymore? I like that the suppression works for each check in the exact same way, now I have to remember that it works differently for this specific check. Or is the style of suppressing that I'm using going to be deprecated?

@romani

romani commented Sep 15, 2019

Copy link
Copy Markdown
Contributor Author

LineLength changed his parent from TreeWalker to Checker. This allows to use this Check on non java files( all Checks that located under Checker are not aware of AST of file they validate), as validation is super easy, just count character in line.
But there was a side effect: as Check change its parent module, filters/suppression module are changing also. Now LineLength can use filters only of Checker, so you see in diff new filter SuppressWithPlainTextCommentFilter. Unfortunately for now such filter does not support influenceFormat.
SuppressWithNearbyCommentFilter is filter for TreeWalker, that is AST aware filter, and it has influenceFormat.

We need to extend SuppressWithPlainTextCommentFilter to support influenceFormat checkstyle/checkstyle#7064

@jqno

jqno commented Sep 16, 2019

Copy link
Copy Markdown
Owner

So if I understand you correctly, SuppressWithNearbyCommentFilter already didn't work for many of the checks? And the new issue you opened will add the ability to SuppressWithPlainTextCommentFilter to have (more or less) the same behavior as SuppressWithNearbyCommentFilter?

@romani

romani commented Sep 16, 2019

Copy link
Copy Markdown
Contributor Author

SuppressWithNearbyCommentFilter already didn't work for many of the checks?

Yes, as it works for Checks that validate any text file type ( not only java). Check was moved to more general area of validation, and unfortunately corresponding filter in such area has less functionality.

And the new issue you opened will add the ability

Yes

@jqno

jqno commented Sep 16, 2019

Copy link
Copy Markdown
Owner

OK, I understand now. Thanks for taking the time to explain it to me, and also thanks for making the PR in the first place! I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants