checkstyle / patch-filters Goto Github PK
View Code? Open in Web Editor NEWSuppression Filter for Checkstyle that is based on patch file
License: GNU Lesser General Public License v2.1
Suppression Filter for Checkstyle that is based on patch file
License: GNU Lesser General Public License v2.1
Related comment: https://github.com/checkstyle/patch-filters/pull/96/files#r456180795
There are a bunch of ignored tests in SuppressionPatchFilterTest.java
. They need to be fixed, @Ignore
s should be removed.
There are two of them:
for UniquePropertiesCheck, will be resolved in #108 or we need to update Check to report violation on all duplicates (I agree, but filter should still expect Check that place violation in unrelated to change place). There should be few UTs (without new property (mostly suppress all violations) and with property)
for RegexpMultilineCheck - same as above. Violation placed at line where pattern starts, but due to multiline nature, update is done NOT on first line, so Filter should suppress violation if no property from #108 (attention that id
might be used in such property if user wants).
There is problem:
GeneratePatchFileLauncher
against a checked out projectExpected:
Project's HEAD is reset to initial state which had been before running the generator
Detected:
Project's HEAD points to the last commit GeneratePatchFileLauncher
checked out and built report for
This is annoying because you need to manually reset project's HEAD back to initial state after each launch of generator.
GeneratePatchFileLauncher
's last parameter is the number of reports to generate for each commit starting from HEAD
. If you give value 4
there will be 3 reports generated between each pear of commits.
The problem with this approach is that usually latest commits contain very few changes. It would be good to specify a list of commit hashes and generate reports for those commits only (the diff between the specified hash and the previous one is a patch). In this way we are free to choose any commits with a lot of code changes and use them in report analysis.
remove Translation Check .... it is very special check let not support it for now.
remove:
<module name="RegexpSingleline">
<property name="format" value="^(?!(.*http|import)).{101,}$"/>
<property name="fileExtensions" value="g, g4"/>
<property name="message" value="Line should not be longer than 100 symbols"/>
</module>
remove:
<module name="RegexpSingleline">
<property name="format" value="href="(?!\/|https?:\/\/).*?\.dtd""/>
<property name="fileExtensions" value="xml, vm"/>
<property name="message"
value="Relative links to DTD files are prohibited. Please use absolute path or uri instead."/>
</module>
remove all checks that have messages Old site links should not be used,
remove Check with message All files in the ''src/xdocs'' folder should h...
when I try to test and get violations through patch filter, I find a bug about name, reason is that patch filter will decide to accept an AuditEvent event according to isFileNameMatching(event) && isLineMatching(event);
diff
or git format
or similar command, it is revenant path, and a subset of event's absolute path.update some old UT in SuppressionPatchFilterTest because of using new JGit's parsing patch file function.
try LineLength
check which mentioned in #21.
report link is here: https://huganghui.github.io/generatePatchReport/DiffReport/
but I am confused with that guava-bfc1cce does not have any violations, because I use LineLength
check:
<module name="LineLength">
<property name="max" value="20"/>
</module>
We need to know how users can generate diff for our filter by some git commands:
git show
git patch-format
We need to make few new patch methods in generator that use shell execution to get patch.
from Gdoc please move all usecases to inputs (input file and input patch file).
Even they are not executed by UTs please move them to sources.
Create UT that take input file and input patch and execute checkstyle validation the same as checkstyle do in his UTs (it is ok for config to be created in runtime).
if some usecase is not ready for execution (for example it dependes on Treewalker filter), inputs should still be hosted in github surces, UT method is created and compile-able (it could be empty and have commented out code for future) but marked with @Ignored
annotation.
MethodLine folder should MethodLength,
make junit test that use https://github.com/checkstyle/patch-filters/blob/a93897014ce3953ccd636f7d4733f1d436f85db0/src/test/resources/com/puppycrawl/tools/checkstyle/filters/MethodLine
if for some reason it is not passing, mark it as Ignore
and mention issue is what it will be resolved.
Please make sure that all folders in test resources are used in Junit.
com.puppycrawl.tools.checkstyle.filters.suppressionpatchfilter
com.puppycrawl.tools.checkstyle.filters.suppressionpatchxpathfilter
Some unit tests fail on Windows (in cmd, git bash). Works fine on Linux though.
Failed tests: testFileTabCharacterMoveCodeInSameFileWithMinorChanges(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\FileTabCharacter\MoveCodeInSameFileWithMinorChanges\newline/Test.java:19:25: Line contains a tab character].> but was:<[Audit done].>
testLineLength(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\LineLength\newline/Test.java:10: Line is longer than 80 characters (found 163)].> but was:<[Audit done].>
testOrderedProperties(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\OrderedProperties\newline/test.properties:6: Property key 'key.pag' is not in the right order with previous property 'key.png'].> but was:<[Audit done].>
testFileTabCharacterSingleChangedLine(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\FileTabCharacter\SingleChangedLine\patchedline/Test.java:19:25: Line contains a tab character].> but was:<[Audit done].>
testRegexpSingleline(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<...xpSingleline\newline[/]Input.java:12: Line ...> but was:<...xpSingleline\newline[\]Input.java:12: Line ...>
testFileTabCharacterMoveCodeInSameFileWithoutChanges(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\FileTabCharacter\MoveCodeInSameFileWithoutChanges\newline/Test.java:19:25: Line contains a tab character].> but was:<[Audit done].>
testFileTabCharacter(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\FileTabCharacter\newline/Test.java:21:25: Line contains a tab character].> but was:<[Audit done].>
testFileTabCharacterSingleNewLine(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchfilter\FileTabCharacter\SingleNewLine\newline/Test.java:20:25: Line contains a tab character].> but was:<[Audit done].>
testMethodName(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchXpathFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchxpathfilter\MethodName\newline/Test.java:4:17: Name 'MyMethod' must match pattern '^[a-z][a-zA-Z0-9]*$'].> but was:<[Audit done].>
testAvoidNestedBlocks(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchXpathFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchxpathfilter\AvoidNestedBlocks\patchedline/Test.java:5:9: Avoid nested blocks].> but was:<[Audit done].>
testMagicNumber(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchXpathFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchxpathfilter\MagicNumber\patchedline/Test.java:5:20: '256' is a magic number].> but was:<[Audit done].>
testIllegalToken(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchXpathFilterTest): error message 0 expected:<[C:\Users\Baratali\git\patch-filters\src\test\resources\com\puppycrawl\tools\checkstyle\filters\suppressionpatchxpathfilter\IllegalToken\newline/Test.java:15:14: Using 'outer:' is not allowed].> but was:<[Audit done].>
testStrategyOptionOnEclipse(com.puppycrawl.tools.checkstyle.jgit.GitDiffOnOpenSourceTest): Audit event should be rejected when there are no matching patch filters ==> expected: <true> but was: <false>
testInsertDeleteReplaceEditTypesWithinHunk(com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeDefaultTest): expected:<3> but was:<0>
testMultopleFilesWithMultipleHunks(com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeDefaultTest): expected:<2> but was:<1>
Tests in error:
testNewlineAtEndOfFile(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): Invalid id: 0047b33
testFileLength(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): Invalid id: 67782f7
testRegexpOnFilename(com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilterTest): Invalid id: 86748d4
testHaveRemovedFile(com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeDefaultTest): Invalid id: 0000000
testHaveRemovedFile(com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeZeroTest): Invalid id: 0000000
Tests run: 42, Failures: 15, Errors: 5, Skipped: 6
Interesting error on parsing a patch file:
testHaveRemovedFile(com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeZeroTest) Time elapsed: 0 sec <<< ERROR!
org.eclipse.jgit.errors.InvalidObjectIdException: Invalid id: 0000000
at org.eclipse.jgit.lib.AbbreviatedObjectId.fromHexString(AbbreviatedObjectId.java:151)
at org.eclipse.jgit.lib.AbbreviatedObjectId.fromString(AbbreviatedObjectId.java:108)
at org.eclipse.jgit.patch.FileHeader.parseIndexLine(FileHeader.java:607)
at org.eclipse.jgit.patch.FileHeader.parseGitHeaders(FileHeader.java:507)
at org.eclipse.jgit.patch.Patch.parseDiffGit(Patch.java:243)
at org.eclipse.jgit.patch.Patch.parseFile(Patch.java:196)
at org.eclipse.jgit.patch.Patch.parse(Patch.java:178)
at org.eclipse.jgit.patch.Patch.parse(Patch.java:151)
at com.puppycrawl.tools.checkstyle.jgit.AbstractJgitPatchParserEvaluationTest.loadPatch(AbstractJgitPatchParserEvaluationTest.java:35)
at com.puppycrawl.tools.checkstyle.jgit.GitDiffWithContextSizeZeroTest.testHaveRemovedFile(GitDiffWithContextSizeZeroTest.java:111)
Looks like problem is related to line separators. Ubuntu WSL detects diff in all files because of windows line separators, and if I reset git repo git reset --hard
all tests pass (in ubuntu wsl, not in cmd).
Steps to reproduce:
$ git clone [email protected]:checkstyle/patch-filters.git
$ cd patch-filters
$ mvn clean install
Detected:
Results :
Failed tests: testBoundaryOne(com.github.checkstyle.patchfilter.SuppressionPatchFilterTest): Audit event should be rejected when there are no matching patch filters ==> expected: <true> but was: <false>
testMultiChangedFilesOnOnePatch(com.github.checkstyle.patchfilter.SuppressionPatchFilterTest): Audit event should be rejected when there are no matching patch filters ==> expected: <true> but was: <false>
Tests run: 14, Failures: 2, Errors: 0, Skipped: 3
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.495 s
[INFO] Finished at: 2020-06-25T20:16:36+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project patch-filter: There are test failures.
Expected:
Build success
Notes:
$ java -version
java version "1.8.0_201"
Java(TM) SE Runtime Environment (build 1.8.0_201-b09)
Java HotSpot(TM) 64-Bit Server VM (build 25.201-b09, mixed mode)
Test this filter on a Guava project, we need to use checkstyle config that use only Checker Checks and path filter. We need to make some sort of script that iterate over git history and execute checkstyle, if any violation detected store report and diff aside for review later on. Result of such script is list of (diff, violations, full copy of files). Report should be shared with mentors to analyze.
During testing analyze violations which are skipped by the "line" strategy but can be covered by a "context" strategy
make a config, share it with mentors.
script should take SHA from where to start, make patch, execute checkstyle, store patch, files with violations in some report folder under subfolder that is named by commit. So result of execution is folder with files that can be shared and browsed in github.io .
All interesting cases should be moved to our test base (inputs should be minimized ).
We need pack of violations from each Check , we can modify Checks settings in config to be more aggressive and prone to violations.
We can take any range of commits to catch violations.
We can take any other opensource project for testing.
Checks:
FileTabCharacter
LineLength
FileLength
RegexpOnFilename
RegexpSingleline
RegexpMultiline
OrderedProperties
UniqueProperties
NewlineAtEndOfFile
we should not use filer.accept()
in test,
we should use same approach as in main library - full execution, check for messages.
we have tests like:
public void testAccept() throws Exception {
final String fileName = getPath("MethodCount/MethodCountPatch.txt");
final SuppressionPatchFilter filter = createSuppressionPatchFilter(fileName);
final LocalizedMessage message = new LocalizedMessage(7, 1, null, "msg", null,
SeverityLevel.ERROR, null, getClass(), null);
final AuditEvent ev = new AuditEvent(this, "MethodCountUpdate.java", message);
assertTrue(filter.accept(ev),
"Audit event should be rejected when there are no matching patch filters");
}
we need to have test to be like:
public void testAccept() throws Exception {
final Configuration checkConfig = getConfig(getPath("MethodCount/MethodCountConfig.xml"));
// update checkConfig to add filter or place filter config in xml
final String filePath = getPath("InputFileTabCharacter.java");
final String[] expected = {
"8:25: Line is longer than 2 characters (found 20)",
// ..... all other violations.
};
verify(checkConfig, filePath, expected);
}
We need to load whole config, run whole checkstyle on single file and have exact list of violations.
Please investigate how we do this in main project:
https://github.com/checkstyle/checkstyle/blob/ae9edbd4a8645c8cc1a99ad6890dd0006700d1cb/src/it/java/com/google/checkstyle/test/chapter2filebasic/rule231filetab/FileTabCharacterTest.java#L36
https://github.com/checkstyle/checkstyle/blob/739cf8e48e912fbd8ef633171a5a95e9d36e8a36/src/test/java/com/puppycrawl/tools/checkstyle/checks/sizes/LineLengthCheckTest.java#L39
Right now I see no UTs for Checker filter that you created. Please create such tests(patch, Input java file, config) for all Checks of Checker.
In #74 you will need to create manul tests, but in this issue we need to automate it.
As mentioned in #21 (comment), we need to update README.md——modifying projects-to-test-on.properties, remove specific version in project(for example in guava, remove v28.2
, change guava|git|https://github.com/google/guava|v28.2||
to guava|git|https://github.com/google/guava||
) to avoid Hidden bug about commitID
context - consider a wider context when new code introduces violations outside of new/changed lines
As we moved all content of test to files, except for expected values
it is still hard to do review, as we always need to jump between file and tests.
only expected is left in test method, if we move expected content to special file in folder - our test will be simple
@Test
public void testFileTabCharacterMoveCodeInSameFileWithMinorChanges() throws Exception {
testByConfig("strategy/FileTabCharacter/MoveCodeInSameFileWithMinorChanges/newline");
testByConfig("strategy/FileTabCharacter/MoveCodeInSameFileWithMinorChanges/patchedline");
}
with strategy/FileTabCharacter/expected.txt
19:25: Line contains a tab character.
instead:
@Test
public void testFileTabCharacterMoveCodeInSameFileWithMinorChanges() throws Exception {
final String inputFile =
"strategy/FileTabCharacter/MoveCodeInSameFileWithMinorChanges/Test.java";
final String defaultContextConfigPathOne =
"strategy/FileTabCharacter/"
+ "MoveCodeInSameFileWithMinorChanges/newline/defaultContextConfig.xml";
final String zeroContextConfigPathOne =
"strategy/FileTabCharacter/"
+ "MoveCodeInSameFileWithMinorChanges/newline/zeroContextConfig.xml";
final String[] expectedOne = {
"19:25: Line contains a tab character.",
};
testByConfig(defaultContextConfigPathOne, inputFile, expectedOne);
testByConfig(zeroContextConfigPathOne, inputFile, expectedOne);
...
ATTENTION: when we start to run tests on few files/folders we still can make standard name folder to let test to run on that folder.
https://github.com/java-diff-utils/java-diff-utils has a lot of restrictions and has a very poor support of loading a patch from a text file. There is another option we need to try - JGit. It is a more mature and reliable tool.
In scope of this issue add unit tests to evaluate JGit Patch parser abilities - https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/patch/Patch.java. JGit is already included as a dependency for a patch-filters project:
<dependency>
<groupId>org.eclipse.jgit</groupId>
<artifactId>org.eclipse.jgit</artifactId>
<version>5.2.1.201812262042-r</version>
</dependency>
Each unit test should have a patch file as an input and a bunch of assertions of a resulted Patch java object.
The following cases have to be covered:
The following sets of unit tests are expected for each of the above point:
git diff <commit1> <commit2>
(using default context size) - Pull Request 1git diff -U0 <commit1> <commit2>
(using context size = 0) - Pull Request 2each group of Inputs:
Origin
update
patch
should be placed in special folder, to be isolated from all other files.
As discussed in #84 (comment).
Violation on line 18 should be suppressed, because it was not editted/added in patch (line 19 changed).
This expected message should be removed:
And please remove @ignore from this unit test:
as identified at #13 (comment)
patch file on multiple files could be different from different tools.
library have limited support for this - java-diff-utils/java-diff-utils#82
We have to have this functionality, so we need to make support for us first and then contribute to that library.
We need UT that that proves that we can correctly load patch files with multiple files in it.
We need to check which transitive maven dependencies JGit brings when included in the project. The less, the better. Having a list of those dependencies we will decide whether to include JGit into the project or just copy needed classes from it and put them into a patch-filters.
java.io.FileNotFoundException: /xxx/workspace/patch-filters/DiffReport/index.html (No such file or directory)
at java.base/java.io.FileOutputStream.open0(Native Method)
at java.base/java.io.FileOutputStream.open(FileOutputStream.java:298)
at java.base/java.io.FileOutputStream.<init>(FileOutputStream.java:237)
at java.base/java.io.FileOutputStream.<init>(FileOutputStream.java:187)
at java.base/java.io.FileWriter.<init>(FileWriter.java:96)
at com.github.checkstyle.generatepatchfile.GeneratePatchFile.generateSummaryIndexHtml(GeneratePatchFile.java:318)
at com.github.checkstyle.generatepatchfile.GeneratePatchFile.generatePatch(GeneratePatchFile.java:164)
at com.github.checkstyle.generatepatchfile.GeneratePatchFileLauncher.main(GeneratePatchFileLauncher.java:48)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at com.intellij.rt.execution.application.AppMainV2.main(AppMainV2.java:128)
After creating DiffReport
folder manually the generator starts working.
If there is no such folder it should be created automatically without giving the above exception.
"Line" strategy in SuppresionPatchFilter should pass violations only for new lines based on patch/diff file. This means that violations for the changed lines should be suppressed (filter should be able to distinguish a new line from a changed line based on a patch file).
Example:
Input file before changes:
$ cat Input_original.java
class Input {
public void a() {
System.out.print("a");
}
public boolean b() {
return false;
}
}
Input file after changes:
$ cat Input.java
class Input {
public void a() {
System.out.print("changed"); // no violation, because it's not a new line
}
public boolean b() {
System.out.print("b"); // violation
return false;
}
public void c() {
System.out.print("c"); // violation
}
}
Diff:
$ diff -Naru Input_original.java Input.java > patch.diff
$ cat patch.diff
--- Input_original.java 2020-07-12 17:01:49.228134700 +0100
+++ Input.java 2020-07-12 16:48:53.828844600 +0100
@@ -1,9 +1,14 @@
class Input {
public void a() {
- System.out.print("a");
+ System.out.print("changed"); // no violation, because it's not a new line
}
public boolean b() {
+ System.out.print("b"); // violation
return false;
}
+
+ public void c() {
+ System.out.print("c"); // violation
+ }
}
Checkstyle config:
$ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<module name="RegexpSingleline">
<property name="format" value="System.out.print"/>
</module>
</module>
Expected checkstyle output would be following. Violation from line 3 should be suppressed:
Starting audit...
[ERROR] C:\Users\Baratali\git\test_checkstyle\line-strategy\Input.java:7: Line matches the illegal pattern 'System.out.print'. [RegexpSingleline]
[ERROR] C:\Users\Baratali\git\test_checkstyle\line-strategy\Input.java:12: Line matches the illegal pattern 'System.out.print'. [RegexpSingleline]
Audit done.
We need to create unit tests to test line strategy for each Checker check. Since line strategy is not implemented yet in SuppressionPatchFilter, please start creating input files, configs and expected results similar to the example above. Then we can easily create actual unit tests based on configs and expected results.
At least one test is required for each non-TreeWalker Check. Also while you generate diff reports for eclipse-cs project, please look for non-trivial cases and add them here as tests.
We need to test all possible diff cases like:
As mentioned in #21 (comment), we need to extend AutomaticBean
in SuppressionPatchFilter
When we using generatePatch(Set<String> commits)
, one weird bug happen when I plan to using 468a63a54a5343f8e09bcab7824c82eb5661a88d in eclipse-cs
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (default-site) on project sample: Error during page generation: Error rendering Maven report: Failed during checkstyle configuration: cannot initialize module com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter - an error occurred when load patch file: Input length = 1 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (default-site) on project sample: Error during page generation
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:213)
Now show violation on all lines shown in patch( changed and context) but we should do only only on lines with +; so we need provide functionality and test that we can take exact changed lines. We do not care about context lines, we need only changed lines.
explain violations at https://github.com/checkstyle/patch-filters/pull/95/files#r456474455
there is not violations in newline strategy , but patchedline raise violation on non changed line
https://checkstyle.org/config_misc.html#Translation
create UT to show how filter can work with this Check when pached files are affected and not.
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 14.205 s
[INFO] Finished at: 2020-06-25T22:44:55+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (default-site) on project sample: Error during page generation: Error rendering Maven report: Failed during checkstyle configuration: cannot initialize module com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter - Unable to instantiate 'com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter' class, it is also not possible to instantiate it as null. 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. -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (default-site) on project sample: Error during page generation
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
SuppressionPatchFilter
is located under com.github.checkstyle.patchfilter
which causes the above error. Fix checkstyle config used for testing.
when only one line was changed in patch file, origin patch filter can not solve it correctly. for example:
BoundaryTestPatchOne.txt
--- Origin1.java 2020-06-14 18:41:56.000000000 +0800
+++ Update1.java 2020-06-14 18:41:56.000000000 +0800
@@ -5 +4,0 @@
-// test
detected at #7 (comment)
We need to make new property in Check like neverSuppressedChecks
String[]
that will have user defined list of Checks to NEVER suppress if files are touched.
If there are violations but patch file is not changing them - we suppress violation.
if there are violations and violation affected files are referenced by any means in patch - no suppress.
It is required as each module is not aware about other modules.
In Addition to Check short name we should consider Check id, to let user differentiate by id
also.
context - consider a wider context when new code introduces violations outside of new/changed lines, but let's presume it is on the same branch in AST but on upper level.
While mentors are busy to analyze report and make decision on what to do. Student should implement the same filter(line >only) but for Treewalker and generate report on guava for google-checks.xml config. - https://github.com/checkstyle/patch->filters/issues/6
as mentioned in #6 (comment), according to doc, the same filter(line only)
, do we need to use current patch filter (line) to test checks belong to Treewalker and generate diff reports? Because many TreeWalker's checks also can be correctly filtered by current patch filter (line), although now patch-filter is only line strategy, and during this process, we can find all checks that can not be correctly filtered by line strategy (doc has part of them, but maybe not full).
testByConfig merged in 646103d should be able to process not only single file, but also directory, because some test input is a directory contains more than one file.
patch-filters/src/main/resources/patchConfig.xml
Lines 20 to 22 in 952acf1
please use system example:
<property name="headerFile" value="${checkstyle.header.file}"/>
you define system variables in IDE Run/Debug configuration of comamndline -Dcheckstyle.header.file=....
As we are moving to JGit we don't need java-diff-utils anymore. Please do cleanup.
all test now
has value like <property name="file" value="src/test/resources/com/puppycrawl/tools/checkstyle/filters/strategy/FileTabCharacter/MoveCodeInSameFileWithMinorChanges/zeroContext.patch" />
it is too long and become hard to review.
We need to make system variable tp
(very cryptic on reason to be very short of "testpath") with value
src/test/resources/com/puppycrawl/tools/checkstyle/filters
so our test will looks like
<property name="file"
value="${tp}/strategy/FileTabCharacter/MoveCodeInSameFileWithMinorChanges/zeroContext.patch" />
ATTENTION: I wrapped tag in two lines to make it unlikely to limit 100 character line length size.
PLEASE update all xml files that are already merged to use variable.
In All hanging PRs please do line wrap for now, to quickly search-replace to tp
ones fix is merged.
Use cases:
JGit interprets the above cases as DELETE and INSERT. So, if there was a violation in that code block and it was moved, newline strategy will show this violation in any case.
Think about how to handle new lines only and don't show violations from old lines.
before test script/app creation we need to make sure we can work with patch that contains information for several files.
We need to update Filter and prove that it works by UT.
implementation could be imperfect by all means, if few test cases are passing it is ok to merge and shift to test script creation.
All known items to improve in Filter should be moved to separate issue.
provide instruction(list of comands) to clone all required repositories, and how to execute report and where to get results.
Generate report that have some violations in it, we need to make sure it works. Please take more commits in generation, think about changing of Checks to produce more violations (for example: confgure LineLength to 80 symbols).
There is an example that only one line changed, so the startLine and endLine is same, when we change isLineMatching to
result = currentLine > startLine && currentLine <= endLine;
boundary test will fail.
and after filename match condition updating, some test should be updated too.
Please add the following use cases:
Tests should be added using the same approach as here #73
For a line
strategy we need to give violations on new (not changed) lines only. Rationale - https://docs.google.com/document/d/1isikemGXUXPOPIX2YnFc8_BaQYYPGSM1vgHuZLBQUIo/edit#heading=h.4j4y4dnetoyf
At the moment we don't distinguish between new and changed lines. Find a way how to do it and make SuppressionPatchFilter
give violations for new lines only.
similar #4 but for new Filter.
let make font if size same as "Checkstyle configuration report" at https://huganghui.github.io/generatePatchReport/DiffReport/guava-fb6ef19/diff/guava/index.html
All non-TreeWalker checks has been analyzed in #21 (comment) and #7 (comment). So we also need to find all TreeWalker Checks that need to use context strategy.
italics text means that this check has been considered and has context strategy UT.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.