-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FUTURE: Gather ideas for column strategy #336
Comments
Here're few more unverified ideas to check and play with. Try to detect location of changed code more accurateGiven a patch we can try to obtain previous code version by reverting that patch. In git it can be done via In the
This information can potentially be used in detecting start / end We can also try to configure diff algorithms mentioned above to skip whitespace characters. However, whitespace related checks may be affected. We can also consider a size of changed code block, if it's one line or few character change it probably makes sense to just skip such outliers and ignore new violations. Use AST trees to accept only new violationsGiven AST tree of the current code we can try to obtain an AST tree for the code before patch. Having old and new trees we can try to accept only new violations by comparing both trees. Another option to investigate is to run Checkstyle second time on the old AST tree and just get a diff between two runs. This option won't work on input files which are not represented as AST trees. Do we need this in Checkstyle?Considering Checkstyle is just a library one may write a plugin to gradle / maven / etc. or just a CI script to run Checkstyle twice (on code before and after patch) and fail if diff is not empty. May not be a viable option if codebase is huge, however, it may still be faster than running all the tests. Another option is to cache Checkstyle execution on master / main and compare with new execution on pathed code in a different branch. |
Existing attempts to solve the problem:
|
Checking the following idea. Do we need this in Checkstyle?
Using
Todo
|
Only comparison of xpath on violation , can help with detection of new or very different by structure same violation ( ok to keep as new). Comparison by line and column will be too unreliable. We might end up in state to figure out algorithm how to compare two xpath and consider them the same. |
Column strategy should be able to distinguish between existing and new violations on the changed code lines and show new violations only. Current
patchedline
strategy can't understand what exactly was changed in the line (e.g. a whitespace was added between words), so it may show old violations as well.Investigate ways to implement a column strategy. One of them can be usage of https://github.com/google/diff-match-patch to compare an old and a new line and get the columns within those lines with a new text.
The text was updated successfully, but these errors were encountered: