Checkstyle has recently started to update old checks from logging on the line/column, to log on the AST node. This is needed to support https://checkstyle.org/config_filters.html#SuppressionXpathFilter which needs the AST of the violation to match it to the user supplied XPath.
As PR is started for each check, links from master ( Ex: https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java#L201 ) must be posted to show each log call found in the check. An additional note must be made of the number of logs actually modified. This will greatly help admins ensure the full check is now xpath compliant and nothing is missed.
All checks need to be reviewed to ensure:
- All their calls to
log pass the AST and not the line/column
- Each log have a respective IT test to show that it supports xpath at https://github.com/checkstyle/checkstyle/tree/master/src/it/java/org/checkstyle/suppressionxpathfilter
- Documentation of https://checkstyle.org/config_filters.html#SuppressionXpathFilter_Description shows the check is now supported.
- Since this is modifying the functionality of a check, regression will be required for the PR. See https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-tester
1: We don't expect drastic change in current UTs when making the switch. If any items change the line/column drastically, please point them out to be clear to admins in the PR. Adding a column number when no was there before is ok and an expected change.
2: Must have a UT for each log in the check. If the check has 3 separate log calls, then it must have 3 separate UTs.
4: In report for each project, Number of unique base messages reported below and Number of unique path messages reported below should match otherwise it is saying there is an uneven amount of changes, either new violations or violations removed. We aren't expecting this type of change in these PRs unless you can account for it. One example where it would be acceptable is if duplicate violations on the same line were being dropped as we only report 1 violation per line/column/message/check.
Checkstyle has recently started to update old checks from logging on the line/column, to log on the AST node. This is needed to support https://checkstyle.org/config_filters.html#SuppressionXpathFilter which needs the AST of the violation to match it to the user supplied XPath.
As PR is started for each check, links from master ( Ex: https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java#L201 ) must be posted to show each log call found in the check. An additional note must be made of the number of logs actually modified. This will greatly help admins ensure the full check is now xpath compliant and nothing is missed.
All checks need to be reviewed to ensure:
logpass the AST and not the line/column1: We don't expect drastic change in current UTs when making the switch. If any items change the line/column drastically, please point them out to be clear to admins in the PR. Adding a column number when no was there before is ok and an expected change.
2: Must have a UT for each log in the check. If the check has 3 separate log calls, then it must have 3 separate UTs.
4: In report for each project,
Number of unique base messages reported belowandNumber of unique path messages reported belowshould match otherwise it is saying there is an uneven amount of changes, either new violations or violations removed. We aren't expecting this type of change in these PRs unless you can account for it. One example where it would be acceptable is if duplicate violations on the same line were being dropped as we only report 1 violation per line/column/message/check.