checkstyle/patch-filters

Suppression Filter for Checkstyle that is based on patch file

Checkstyle is a tool for checking Java source code for adherence to a Code Standard or set of validation rules (best practices).

Contributors chat:

The latest release version can be found at GitHub releases or at Maven repo.

Each-commit builds of maven artifacts can be found at Maven Snapshot repository.

Documentation is available in HTML format, see https://checkstyle.org/checks.html.

Build instructions and Contribution

Build instructions

Setup IDE for development

Explanation on how to create your own module

Verification of code quality

Sending Pull Request

Report Issue

Continuous integration and Quality reports

See our CIs statuses.

Quality reports: https://checkstyle.org/project-reports.html

JavaScript, CSS and Java source file analysis on Codacy:

Feedback/Support

Please send any feedback to https://groups.google.com/forum/?hl=en#!forum/checkstyle

Questions and Answers from community:

Bugs and Feature requests (not the questions): https://github.com/checkstyle/checkstyle/issues

Support/Sponsor checkstyle

If you want to speed up fixing of issue and want to encourage somebody in internet to resolve any issue:

Licensing

This software is licensed under the terms in the file named "LICENSE" in this directory.

The software uses the ANTLR package (https://www.antlr.org/). Its license terms are in the file named "RIGHTS.antlr" in this directory.

This product includes software developed by The Apache Software Foundation (https://www.apache.org/).

The software uses the Logging and Beanutils packages from the Apache Commons project (https://commons.apache.org/). The license terms of these packages are in the file named "LICENSE.apache20" in this directory.

The software uses the Google Guava Libraries (https://github.com/google/guava/). The license terms of these packages are in the file named "LICENSE.apache20" in this directory.

The software uses the Picocli Library (https://github.com/remkop/picocli/). Its license terms are in the file named "LICENSE.apache20" in this directory.

patch-filters's Issues

Add unit tests to evaluate jgit patch parser

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:

1. Patch file contains a single file and multiple hunks within that file
2. Patch file contains multiple files and multiple hunks withing each file
3. Patch file contains file which was renamed without any changes
4. Patch file contains file which was removed
5. Patch file contains a block of code which was moved into another place within the same file without changes
6. Patch file contains a hunk which contains deletions only
7. Patch file contains a hunk which contains additions only
8. Patch file contains a hunk which contains replacements only
9. Patch file contains a hunk which contains deletions and additions and replacements within a single hunk

The following sets of unit tests are expected for each of the above point:

1. Patch is generated via git diff <commit1> <commit2> (using default context size) - Pull Request 1
2. Patch is generated via git diff -U0 <commit1> <commit2> (using context size = 0) - Pull Request 2

testByConfig should be able to process not only single file, but also directory

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.

resolve review item

#28 (comment)

fix bugs about filename and boundary

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.

patch-filters/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilterElement.java

Lines 62 to 66 in 27fb266

	private boolean isFileNameMatching(AuditEvent event) {
	return event.getFileName() != null
	&& ((event.getFileName()).equals(fileName)
	|| event.getFileName().contains(fileName));
	}

minors to improve in tests

1. MethodLine folder should MethodLength,
   

   patch-filters/src/test/resources/com/puppycrawl/tools/checkstyle/filters/MethodLine/MethodLineConfig.xml

   Line 9 in a938970

   <property name="tokens" value="METHOD_DEF"/>

   
   all files should be renamed.

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.

3. please make two separate folders in test resources to keep separate tests for SuppressionPatchFilter and SuppressionPatchXpathFilter
   com.puppycrawl.tools.checkstyle.filters.suppressionpatchfilter
com.puppycrawl.tools.checkstyle.filters.suppressionpatchxpathfilter
   to match https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/filters
   please move all tests in proper folder.

Add Non-Treewalker filter tests to cover more cases

Please add the following use cases:

1. A block of code contains violations and was moved from one file to another file without changes
2. A block of code contains violations and was moved within the same file without changes
3. A block of code contains violations and was moved from one file to another file with minor changes
4. A block of code contains violations and was moved within the same file with minor changes
5. A single new line of code contains violations
6. A single changed line of code contains violations

Tests should be added using the same approach as here #73

Script to iterate over git history and apply certain config to code.

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 ).

Fix Name bug in PatchFilter

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);

patch-filters/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilterElement.java

Lines 51 to 54 in ebd74e0

	@Override
	public boolean accept(AuditEvent event) {
	return isFileNameMatching(event) && isLineMatching(event);
	}

but I find that event.getFileName() is an absolute path, but now filename that get from patch file may not be consistent with event's absolute path, it decide to where you to do diff or git format or similar command, it is revenant path, and a subset of event's absolute path.
So the following FileNameMatching condition need to be modified.

patch-filters/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilterElement.java

Lines 62 to 65 in ebd74e0

	private boolean isFileNameMatching(AuditEvent event) {
	return event.getFileName() != null
	&& (event.getFileName()).equals(fileName);
	}

SuppressionPatchFilter: mention Translation as non supported Check in javadoc

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.

Reset git repo to the initial state after report generation completed

There is problem:

1. Checkout project and note current HEAD's state
2. Launch GeneratePatchFileLauncher against a checked out project
3. Check HEAD's state

Expected:
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.

Create Checkers filter SuppressionPatchFilter that suppress all violations on non affected lines

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.

Generate report that have violations from each Check in config

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

No such file or directory when trying to build a report

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.

Find all TreeWalker Checks that need to use context strategy

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.

update some old UT in SuppressionPatchFilterTest because of using new JGit's parsing patch file function

update some old UT in SuppressionPatchFilterTest because of using new JGit's parsing patch file function.

Resolve all ignored tests in SuppressionPatchFilterTest

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, @Ignores should be removed.

There are two of them:

1. 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)

2. 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).

Movement of code is counted as a new change

Use cases:

1. The code block of 10 lines was moved from one place to another without changes
2. The code block of 10 lines was moved from one place to another with a minor change in one line
3. The code block of 10 lines was moved from one place to another with an addition of a new line within the block
4. The code block of 10 lines was moved from one place to another with a deletion of one line within the block

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.

provide UT for Filter with patch on multiple files

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.

use system variable to customize path patch

patch-filters/src/main/resources/patchConfig.xml

Lines 20 to 22 in 952acf1

	<module name="com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter">
	<property name="file" value="/Users/hgh/Documents/GitHub/patch-filters/DiffReport/patch.txt"/>
	</module>

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=....

Suspicious case: strategy/FileTabCharacter/SingleChangedLine/patchedline/

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

collect ideas on context strategy for SuppressionPatchXpathFilter

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.

Distinguish between new and changed lines

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.

make UT to run all from folder

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.

Update README.md to avoid Hidden bug about commitID

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

Fix boundary bug in PatchFilter

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

Use SupprresionPatchFilter to test checks belong to TreeWalker and generate diff reports

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).

Links patch-filter diff report with ` LineLength` check violations

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>

A bug about generatePatchFile

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)

and when I open patch file, IDEA show a info:

Cannot initialize module com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter

[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.

Change config to remove not relevant Check instances

1. remove Translation Check .... it is very special check let not support it for now.

2. 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=&quot;(?!\/|https?:\/\/).*?\.dtd&quot;"/>
        <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...

Master build is failed

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)

use system variable to keep common part of path for tests

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.

Analyze dependencies jgit brings when included in the project

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.

move use cases from Gsoc to project

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.

Create unit tests for line strategy for Checker checks (non-TreeWalker)

"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:

• there are only deleted lines in patch file
• there are only new lines in patch file
• there are only changed lines in patch file
• new lines in existing file vs new file
• mixed changes - changed + deleted + new lines
• etc...

collect ideas on context strategy for SuppressionPatchFilter

context - consider a wider context when new code introduces violations outside of new/changed lines

Create Treewalker filter SuppressionPatchXpathFilter that suppress all violations on non affected lines

similar #4 but for new Filter.

Take exact changed lines and only show violation on them, not all lines in patch file

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.

update generator to generate patch file by 'git' commands

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.

Wrong expected results for patchedline strategy (RegexpMultiline)

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:

patch-filters/src/test/resources/com/puppycrawl/tools/checkstyle/filters/suppressionpatchfilter/RegexpMultiline/patchedline/expected.txt

Line 1 in 4504cdf

Test.java:18: Line matches the illegal pattern 'System\.out\.\n print\('.

patch-filters/src/test/resources/com/puppycrawl/tools/checkstyle/filters/suppressionpatchfilter/RegexpMultiline/patchedline/Test.java

Lines 18 to 19 in 4504cdf

	System.out. // violation patchedline
	print("Example");

patch-filters/src/test/resources/com/puppycrawl/tools/checkstyle/filters/suppressionpatchfilter/RegexpMultiline/patchedline/zeroContext.patch

Lines 5 to 7 in 4504cdf

	@@ -19 +19 @@ public class Test {
	- printf("Example");
	+ print("Example");

And please remove @ignore from this unit test:

patch-filters/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionPatchFilterTest.java

Lines 149 to 152 in 4504cdf

	@Ignore("RegexpMultilineCheck has a problem is that violation's "
	+ "line information is not on newline/patchedline,"
	+ "this maybe solve on context strategy")
	public void testRegexpMultiline() throws Exception {

update README to provide instruction on how to use report generator

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).

SuppressionPatchFilter: new property 'neverSuppressedChecks'

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.

Unit tests fail on Windows

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).

Specify a list of commit hashes to generate report for

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.

Use full checkstyle run in UTs

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.

extend AutomaticBean in SuppressionPatchFilter

As mentioned in #21 (comment), we need to extend AutomaticBean in SuppressionPatchFilter

Organize all Inputs in special folder

each group of Inputs:
Origin
update
patch

should be placed in special folder, to be isolated from all other files.

Remove java-diff-utils from pom and other places

As we are moving to JGit we don't need java-diff-utils anymore. Please do cleanup.

provide UT for correct patch file loading that contains multiple files in it

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.

make font on index.html good for usage from phone

let make font if size same as "Checkstyle configuration report" at https://huganghui.github.io/generatePatchReport/DiffReport/guava-fb6ef19/diff/guava/index.html