Generate report that have violations from each Check in config #21

romani · 2020-06-20T18:39:43Z

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

romani · 2020-06-23T13:41:26Z

example: https://huganghui.github.io/generatePatchReport/DiffReport/

HuGanghui · 2020-06-24T10:42:11Z

Before generate report on open source project, I make small example test on each check mentioned above to better understand every check and also know some checks' violation line information is at where it violates rules, but some are at the first line of whole.

check whose violation line information is at where it was changed and violates rules:

LineLength
RegexpSingleline
OrderedProperties
FileTabCharacter

these will be exactly captured by this version's patch-filter, no ignore.

check whose violation line information is at the first line of whole file:

FileLength
RegexpOnFilename
NewlineAtEndOfFile

Considering that these violations are only one in one file, so we can exactly captured them through when we know the AuditEvents are created these checks, only using isFileNameMatching in patch-filter not need isLineMatching.

special two

UniqueProperties
RegexpMultiline

although RegexpMultiline's violation line information is at where it was changed and violates rules, but a situation will happen:

cat config.xml

 <module name="RegexpMultiline">
        <property name="matchAcrossLines" value="true"/>
        <property name="format" value="System\.out.*print\("/>
    </module>

cat test.java

void method() {
        System.out. // line 2
        printf("Example"); // line3
        System.out.
        print("Example");
        }

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar /var/tmp/checkstyle-8.34-SNAPSHOT-all.jar -c config.xml test.java

Starting audit...
Audit done.

but if we we change printf to print that match regex, will have a violation, but although changed line is 3, but violation line information is 2, so will be ignored by patch-filter.

cat test.java

void method() {
        System.out. // line 2
        print("Example"); // line3
        System.out.
        print("Example");
        }

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar /var/tmp/checkstyle-8.34-SNAPSHOT-all.jar -c config.xml test.java

Starting audit...
[ERROR] test.java:2: Line matches the illegal pattern 'System\.out.*print\('. [RegexpMultiline]
Audit done.
Checkstyle ends with 1 errors.

Process finished with exit code 1

rdiachenko · 2020-06-24T14:38:10Z

@HuGanghui the two cases where violations are ignored because no changes in code for those lines are fine for now. Those cases will be covered by context strategy. For now we try to make line strategy work which is the simplest one. So, please proceed with turning on all checks and getting violations from all of them.

romani · 2020-06-24T15:06:35Z

it is completely ok that some Check's violation will not be suppressed we will review them put all of them to inputs and think how to do suppression later on.

HuGanghui · 2020-06-25T13:26:42Z

I make new commit by myself on guava, then test on it for OrderedProperties,UniqueProperties and NewlineAtEndOfFile , FileTabCharacter, link is here https://huganghui.github.io/OrderedProperties-FileTabCharacter/DiffReport/

but no violations in report, but should have, you can know from https://huganghui.github.io/OrderedProperties-FileTabCharacter/DiffReport/guava-ddbd27f/guava-patch-bcc7d34-ddbd27f.txt, that I add two file one java file, one properties to guava-ddbd27f commit, so in guava-ddbd27f-diff the files number should be 3162, but The fact is that both commit-diff report's total file number is 3160, I am confused with it.

attention: this report does not use patch-filter

rdiachenko · 2020-06-28T20:56:52Z

@HuGanghui the problem why SuppressionPatchFilter skips all violations is that the patch file is not set. To make it work you need to extend AutomaticBean in SuppressionPatchFilter. This class sets the properties according to your config file. Without extending this class none of the properties are set.

Please read few times:

Here's the diff which made SuppressionPatchFilter work on my local machine:

$ git diff src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilter.java
diff --git a/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilter.java b/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilter.java
index eaa60ba..99c6ba7 100644
--- a/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilter.java
+++ b/src/main/java/com/github/checkstyle/patchfilter/SuppressionPatchFilter.java
@@ -29,6 +29,8 @@ import com.github.difflib.UnifiedDiffUtils;
 import com.github.difflib.patch.Chunk;
 import com.github.difflib.patch.Patch;
 import com.puppycrawl.tools.checkstyle.api.AuditEvent;
+import com.puppycrawl.tools.checkstyle.api.AutomaticBean;
+import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
 import com.puppycrawl.tools.checkstyle.api.Filter;
 
 /**
@@ -39,7 +41,7 @@ import com.puppycrawl.tools.checkstyle.api.Filter;
  *
  * @since 8.34
  */
-public class SuppressionPatchFilter implements Filter {
+public class SuppressionPatchFilter extends AutomaticBean implements Filter {
 
     /**
      * Specify the location of the patch file.
@@ -90,9 +92,13 @@ public class SuppressionPatchFilter implements Filter {
      *
      * @throws Exception if there is a configuration error.
      */
-    protected void finishLocalSetup() throws Exception {
+    protected void finishLocalSetup() {
         if (file != null) {
-            loadPatchFile(file);
+            try {
+                loadPatchFile(file);
+            } catch (IOException e) {
+                throw new IllegalStateException("Failed to load patch file: " + file);
+            }
         }
     }

HuGanghui · 2020-06-29T04:12:35Z

@rdiachenko when I do these tests, I using patch-filter in checkstyle checkstyle/checkstyle#8318 that has extended AutomaticBean in SuppressionPatchFilter, that why only files in path are reported violations in diff report, which prove that patch file is set and has effect. And also I change it in patch-filter project and run the example in #21 (comment) again, it is the same result. So I think the reason is not this one.

HuGanghui · 2020-06-29T11:28:28Z

@rdiachenko @romani please review this report: https://huganghui.github.io/Check-diff-report/DiffReport/

this report contains most of Checks mentioned in #21 (comment)

and I finally find the reason of problem mentioned in #21 (comment) and #21 (comment), the fact is that

https://github.com/checkstyle/contribution/blob/ebfe884d4ed6de955515396a45e3fc2f78ab0ec9/checkstyle-tester/projects-to-test-on.properties#L33

https://github.com/checkstyle/contribution/blob/ebfe884d4ed6de955515396a45e3fc2f78ab0ec9/checkstyle-tester/launch.groovy#L134-L142

In launch.groovy if the project such as guava, it has a fix commitID: v28.2, then eveytime we run launch.groovy, it will auto checkout to this commit, so it cause that I create a new file in a new commit, but in diff report not display this file, because launch.groovy auto checkout to commit v28.2. This detail is easy to overlook...sorry.

so when I remove the |v28.2||, the problem disappers.

rdiachenko · 2020-06-29T21:54:32Z

@HuGanghui

so it cause that I create a new file in a new commit, but in diff report not display this file, because launch.groovy auto checkout to commit v28.2

I don't understand this, please clarify.

https://huganghui.github.io/new-file-not-display/DiffReport/Check-diff-report/DiffReport/

Did you check your link before posting it?

HuGanghui · 2020-06-30T01:10:43Z

@rdiachenko sorry, it's ok now, report: https://huganghui.github.io/Check-diff-report/DiffReport/

HuGanghui · 2020-06-30T01:46:16Z

@rdiachenko What I want to express is that when running diff.groovy,

patch-filters/src/main/java/com/github/checkstyle/generatepatchfile/GeneratePatchFile.java

Lines 308 to 322 in 84a8c20

    
           private File generate() 
        
                   throws InterruptedException, IOException { 
        
               final Process process = new ProcessBuilder() 
        
                       .directory(new File(testerPath)) 
        
                       .command( 
        
                               "groovy", "diff.groovy", 
        
                               "-r", checkstyleRepoPath, 
        
                               "-b", "master", 
        
                               "-p", checkstyleBranch, 
        
                               "-bc", baseConfigFile.getAbsolutePath(), 
        
                               "-pc", patchConfigFile.getAbsolutePath(), 
        
                               "-l", "projects-to-test-on.properties" 
        
                       ) 
        
                       .inheritIO() 
        
                       .start();

launch.groovy(be called in diff.groovy to generate single violation report) will check projects-to-test-on.properties' every line to get the project that you want to create diff report, for example guava|git|https://github.com/google/guava|v28.2||, problem is that this guava example provide a specific version/commitID——v28.2 which causes that launch.groovy will using git checkout to checkout guava to v28.2, then create report. It means every time no matter what commitID you choose and checkout to it manually, actually the report is created based on v28.2 not commitID you choose.

And the solution is to remove v28.2, using guava|git|https://github.com/google/guava|| in projects-to-test-on.properties

rdiachenko · 2020-06-30T20:49:42Z

@HuGanghui

It means every time no matter what commitID you choose and checkout to it manually, actually the report is created based on v28.2 not commitID you choose.

I got it, thanks. And this is not a problem. Why would you need to do checkout manually? You should update projects-to-test-on.properties and specify a commit hash or version there and rely on diff.groovy to do checkout. Consider this file as a config to diff.groovy. This line guava|git|https://github.com/google/guava|v28.2|| means that your HEAD will point to v28.2 and your HEAD may point to any commit hash you specified in projects-to-test-on.properties. If you remove v28.2 the HEAD will point to master by default. So, it's up to use where to point your HEAD.

rdiachenko · 2020-06-30T21:15:16Z

@HuGanghui ,

it's ok now, report: https://huganghui.github.io/Check-diff-report/DiffReport/

We need more violations and cover rest of the checks. The patch you used in the report is very simple - all lines were added. We need commits with a lot of code changes where multiple lines were added/updated. Please finish #50 asap, it will allow to specify hashes of big commits and build report for them only.

HuGanghui · 2020-07-01T02:15:29Z

@rdiachenko

Why would you need to do checkout manually? You should update projects-to-test-on.properties and specify a commit hash
or version there

yes, I understand, it's ok to update projects-to-test-on.properties, but one situation is that when using GeneratePatchFile to create diff report, most of cases, we need to create 2 or more reports, so it's not convenient to update projects-to-test-on.properties by ourself. And do checkout manually is not really manually doing, What I mean is that now in GeneratePatchFile, we already have checkout function, for example checkoutWithGitCommand();, checkout():

patch-filters/src/main/java/com/github/checkstyle/generatepatchfile/GeneratePatchFile.java

Lines 173 to 179 in 0eee80d

    
           public void generatePatchWithGitCommand(int runPatchNum, String patchFormat) throws Exception { 
        
               for (int i = 1; i < runPatchNum; i++) { 
        
                   generateDiffPatchWithGitCommand(i, patchFormat); 
        
                   checkoutWithGitCommand(); 
        
               } 
        
               generateSummaryIndexHtml(); 
        
           }

https://github.com/checkstyle/patch-filters/blob/0eee80dba327c03addf92682fd2fc4200517b55d/src/main/java/com/github/checkstyle/generatepatchfile/GeneratePatchFile.java#L300-302

it help us to checkout automatically when more one diff report need to be created. but If specify a commit hash on projects-to-test-on.properties will cause what I mentioned in #21 (comment), unless you change projects-to-test-on.properties every time, but we now have a better solution, and I test it, it is work.

This line guava|git|https://github.com/google/guava|v28.2|| means that your HEAD will point to v28.2 and your HEAD may >point to any commit hash you specified in projects-to-test-on.properties.

HEAD branch can be specified before run GeneratePatchFile , this need users manually to checkout to what branch they want, and then every thing will ok and work.

rdiachenko · 2020-07-01T21:13:25Z

@HuGanghui ok, I see. I haven't use generatePatchWithGitCommand and use generatePatch instead which doesn't do checkout. Please update README in scope of #59.

HuGanghui · 2020-07-02T04:12:56Z

Here is a new diff report with three commits: https://huganghui.github.io/patch-filter-50/DiffReport2/

rdiachenko · 2020-07-02T21:06:13Z

@HuGanghui

https://huganghui.github.io/patch-filter-50/DiffReport2/guava-7009a3c/guava-patch-bcc7d34-7009a3c.txt
diff --git a/guava/src/com/google/common/io/PatchFilter.java b/guava/src/com/google/common/io/PatchFilter.java

What's this? Why did you add your file manually? This is not a way to go.

Please checkout https://github.com/checkstyle/eclipse-cs, it should have a lot of violations. Find commit hashes and build a report for them. You need to get violations for all checks. Please list here those checks you could not find violations for and a link to the report witch contains violations for the rest of checks.

HuGanghui · 2020-07-03T03:07:02Z

@rdiachenko

What's this? Why did you add your file manually? This is not a way to go.

yes, I add a commit created by myself, this is just for test.

Please checkout https://github.com/checkstyle/eclipse-cs, it should have a lot of violations.

ok, I will do it.

HuGanghui · 2020-07-03T05:14:40Z

eclipse-cs diff reports about the first 5 commits, link is here: https://huganghui.github.io/eclipse-cs-checker/DiffReport/

in first 5 commits, only LineLength and RegexpSingleline violations in diff reports, we need more commits to test.

OrderedProperties
UniqueProperties

need .properties files, but as mentioned in #31 (comment), diff reports can only display violations on .java files. So these two checks are special.

cat patchConfig.xml

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name = "Checker">
    <property name="charset" value="UTF-8"/>

    <!-- do not change severity to 'error', as that will hide errors caused by exceptions -->
    <property name="severity" value="warning"/>

    <!-- haltOnException is required for exception fixes and reporting of all exceptions -->
    <property name="haltOnException" value="false"/>

    <!-- BeforeExecutionFileFilters is required for sources of java9 -->
    <module name="BeforeExecutionExclusionFileFilter">
        <property name="fileNamePattern" value="module\-info\.java$" />
    </module>

    <module name="com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter">
        <property name="file" value="${checkstyle.patchfilter.patch}"/>
    </module>

    <!--<module name="Header">-->
    <!--<property name="headerFile" value="${checkstyle.header.file}"/>-->
    <!--<property name="fileExtensions" value="java"/>-->
    <!--<property name="id" value="header"/>-->
    <!--</module>-->
    <!--<module name="RegexpHeader">-->
    <!--<property name="headerFile" value="${checkstyle.regexp.header.file}"/>-->
    <!--<property name="fileExtensions" value="java"/>-->
    <!--</module>-->

    <!-- Javadoc Comments -->
    <module name="JavadocPackage">
        <property name="allowLegacy" value="false"/>
    </module>

    <!-- Miscellaneous -->
    <module name="NewlineAtEndOfFile"/>
    <module name="UniqueProperties"/>
    <module name="OrderedProperties" />

    <!-- Regexp -->
    <module name="RegexpMultiline"/>
    <module name="RegexpMultiline">
        <property name="format" value="\r?\n[\t ]*\r?\n[\t ]*\r?\n"/>
        <property name="fileExtensions" value="java,xml,properties"/>
        <property name="message" value="Unnecessary consecutive lines"/>
    </module>
    <module name="RegexpMultiline">
        <property name="format" value="/\*\*\W+\* +\p{javaLowerCase}"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="First sentence in a comment should start with a capital letter"/>
    </module>
    <module name="RegexpMultiline">
        <property name="format" value="^\s*$" />
        <property name="matchAcrossLines" value="true" />
        <property name="message" value="Empty file is not allowed" />
    </module>
    <module name="RegexpSingleline">
        <property name="format" value="\s+$"/>
        <property name="minimum" value="0"/>
        <property name="maximum" value="0"/>
    </module>
    <module name="RegexpSingleline">
        <property name="format" value="/\*\* +\p{javaLowerCase}"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="First sentence in a comment should start with a capital letter"/>
    </module>
    <module name="RegexpSingleline">
        <property name="format"
                  value="^(?!(\s*,?\s*&lt;a href=&quot;[^&quot;]+&quot;&gt;|.*http)).{101,}$"/>
        <property name="fileExtensions" value="xml, vm"/>
        <property name="message" value="Line should not be longer than 100 symbols"/>
    </module>
    <!--
     Links to .dtd files should start with "/", "http://" or "https://",
     otherwise they will be broken after archiving the documentation.
     See https://github.com/checkstyle/checkstyle/issues/7340 for details.
    -->
    <module name="RegexpSingleline">
        <property name="id" value="noPackageCommentWithOtherVisibility"/>
        <property name="format" value="/\*\s+package\s+\*/\s+(private|protected|public)"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="Package comment marker should not be used if other visibility is defined"/>
    </module>
    <module name="RegexpOnFilename" />
    <module name="RegexpOnFilename">
        <property name="folderPattern" value="[\\/]src[\\/]\w+[\\/]java[\\/]"/>
        <property name="fileNamePattern" value="\.java$"/>
        <property name="match" value="false"/>
        <message key="regexp.filepath.mismatch"
                 value="Only java files should be located in the ''src/*/java'' folders."/>
    </module>

    <!-- Size Violations -->
    <module name="FileLength">
        <property name="fileExtensions" value="java"/>
    </module>
    <module name="LineLength">
        <property name="max" value="80"/>
    </module>

    <!-- Whitespace -->
    <module name="FileTabCharacter">
        <property name="eachLine" value="false"/>
    </module>
    <!-- Example of filter -->
    <!--
    <module name="SeverityMatchFilter">
        <property name="severity" value="warning"/>
        <property name="acceptOnMatch" value="false"/>
    </module>
    -->

    <!-- as we run on regression even on non-compiled files we need to skip exceptions on them -->
    <module name="SuppressionSingleFilter">
      <property name="message" value="Exception occurred while parsing"/>
      <property name="checks" value="Checker"/>
    </module>
</module>

HuGanghui · 2020-07-03T08:33:52Z

eclipse-cs diff reports about the first 5 commits, with all checks in here: https://github.com/checkstyle/checkstyle/blob/ec4d06712ab203d31d73c5c6d5c46067f3a6d5b3/config/checkstyle_checks.xml, link is here: https://huganghui.github.io/eclipse-cs-checker/DiffReport-with-all-checks/

current version of patch-filter can correctly deal with all checks whose violation line information is at where it was changed and violates rules, no matter belong to Checker module or TreeWalker module

as for checks whose violation line information is not at where it was changed and violates rules, no matter belong to Checker module or TreeWalker module. This kind check can be split to two kinds: it only has one violation in one file, and it can have many violations in one file.

and as mentioned in #21 (comment),
such as

FileLength
FileTabCharacter
RegexpOnFilename
NewlineAtEndOfFile

Considering that these violations are the only in one file if occur, so we can exactly captured them through when we know the AuditEvents are created these checks, only using isFileNameMatching condition in patch-filter not need isLineMatching condition.

And

RegexpMultiline

represents checks whose violation line information may happen many times in one file. it should be covered by context strategy.

Of course, both two kind checks (it only has one violation in one file, and it can have many violations in one file.) I think can be solve by Xpath, context strategy.

rdiachenko · 2020-07-03T21:31:04Z

@HuGanghui

Please checkout https://github.com/checkstyle/eclipse-cs, it should have a lot of violations. Find commit hashes and build a report for them. You need to get violations for all checks. Please list here those checks you could not find violations for and a link to the report witch contains violations for the rest of checks.

Please reread issue description. By all checks I meant those which are listed in the description only. We don't need TreeWalker checks for now. Could you please do what I asked:

Please checkout https://github.com/checkstyle/eclipse-cs, it should have a lot of violations
Find commit hashes and build a report for them. You need to get violations for all checks (only those listed in the issue description)
List here those checks you could not find violations for and a link to the report witch contains violations for the rest of checks

HuGanghui · 2020-07-06T08:01:00Z

a link to the report witch contains violations for the rest of checks

a new diff report with all checks except OrderedProperties and UniqueProperties: https://huganghui.github.io/patch-filter-21/DiffReport/

List here those checks you could not find violations for

OrderedProperties
UniqueProperties

rdiachenko · 2020-07-06T21:19:03Z

@HuGanghui please add few more commits in the report where you have deletions/changes/additions.

HuGanghui · 2020-07-07T01:48:12Z

@rdiachenko it means creating some commits by myself and add them to this report or finding other commits in t https://github.com/checkstyle/eclipse-cs

rdiachenko · 2020-07-07T12:29:50Z

@HuGanghui find additional other commits with deletions/additions, don't add your own commits.

HuGanghui · 2020-07-07T14:48:36Z

diff report with additional commits: https://huganghui.github.io/patch-filter-21/DiffReportWithFour/

rdiachenko · 2020-07-08T21:58:32Z

@HuGanghui is the patchConfig.xml up to date? I mean, if I take a master branch and commit hashes from your report and build a report on my localhost, will it be the same as yours?

HuGanghui · 2020-07-09T03:58:51Z

@rdiachenko yes, it is updated. here is patchConfig.xml that I use:

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name = "Checker">
    <property name="charset" value="UTF-8"/>

    <!-- do not change severity to 'error', as that will hide errors caused by exceptions -->
    <property name="severity" value="warning"/>

    <!-- haltOnException is required for exception fixes and reporting of all exceptions -->
    <property name="haltOnException" value="false"/>

    <!-- BeforeExecutionFileFilters is required for sources of java9 -->
    <module name="BeforeExecutionExclusionFileFilter">
        <property name="fileNamePattern" value="module\-info\.java$" />
    </module>

    <module name="com.puppycrawl.tools.checkstyle.filters.SuppressionPatchFilter">
        <property name="file" value="${checkstyle.patchfilter.patch}"/>
    </module>

    <!--<module name="Header">-->
    <!--<property name="headerFile" value="${checkstyle.header.file}"/>-->
    <!--<property name="fileExtensions" value="java"/>-->
    <!--<property name="id" value="header"/>-->
    <!--</module>-->
    <!--<module name="RegexpHeader">-->
    <!--<property name="headerFile" value="${checkstyle.regexp.header.file}"/>-->
    <!--<property name="fileExtensions" value="java"/>-->
    <!--</module>-->

    <!-- Javadoc Comments -->
    <module name="JavadocPackage">
        <property name="allowLegacy" value="false"/>
    </module>

    <!-- Miscellaneous -->
    <module name="NewlineAtEndOfFile"/>
    <module name="UniqueProperties"/>
    <module name="OrderedProperties" />

    <!-- Regexp -->
    <module name="RegexpMultiline"/>
    <module name="RegexpMultiline">
        <property name="format" value="\r?\n[\t ]*\r?\n[\t ]*\r?\n"/>
        <property name="fileExtensions" value="java,xml,properties"/>
        <property name="message" value="Unnecessary consecutive lines"/>
    </module>
    <module name="RegexpMultiline">
        <property name="format" value="/\*\*\W+\* +\p{javaLowerCase}"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="First sentence in a comment should start with a capital letter"/>
    </module>
    <module name="RegexpMultiline">
        <property name="format" value="^\s*$" />
        <property name="matchAcrossLines" value="true" />
        <property name="message" value="Empty file is not allowed" />
    </module>
    <module name="RegexpSingleline">
        <property name="format" value="\s+$"/>
        <property name="minimum" value="0"/>
        <property name="maximum" value="0"/>
    </module>
    <module name="RegexpSingleline">
        <property name="format" value="/\*\* +\p{javaLowerCase}"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="First sentence in a comment should start with a capital letter"/>
    </module>
    <module name="RegexpSingleline">
        <property name="format"
                  value="^(?!(\s*,?\s*&lt;a href=&quot;[^&quot;]+&quot;&gt;|.*http)).{101,}$"/>
        <property name="fileExtensions" value="xml, vm"/>
        <property name="message" value="Line should not be longer than 100 symbols"/>
    </module>
    <!--
     Links to .dtd files should start with "/", "http://" or "https://",
     otherwise they will be broken after archiving the documentation.
     See https://github.com/checkstyle/checkstyle/issues/7340 for details.
    -->
    <module name="RegexpSingleline">
        <property name="id" value="noPackageCommentWithOtherVisibility"/>
        <property name="format" value="/\*\s+package\s+\*/\s+(private|protected|public)"/>
        <property name="fileExtensions" value="java"/>
        <property name="message"
                  value="Package comment marker should not be used if other visibility is defined"/>
    </module>
    <module name="RegexpOnFilename" />
    <module name="RegexpOnFilename">
        <property name="folderPattern" value="[\\/]src[\\/]\w+[\\/]java[\\/]"/>
        <property name="fileNamePattern" value="\.java$"/>
        <property name="match" value="false"/>
        <message key="regexp.filepath.mismatch"
                 value="Only java files should be located in the ''src/*/java'' folders."/>
    </module>

    <!-- Size Violations -->
    <module name="FileLength">
        <property name="fileExtensions" value="java"/>
        <property name="max" value="400"/>
    </module>

    <module name="LineLength">
        <property name="max" value="80"/>
    </module>

    <!-- Whitespace -->
    <module name="FileTabCharacter">
        <property name="eachLine" value="false"/>
    </module>

    <module name="RegexpOnFilename">
        <property name="fileNamePattern" value="\.java$"/>
    </module>
    <module name="RegexpSingleline">
        <property name="format" value="Test"/>
    </module>
    <module name="RegexpMultiline">
        <property name="matchAcrossLines" value="true"/>
        <property name="format" value="jupiter\.api"/>
    </module>

    <module name="OrderedProperties">
        <!--<property name="fileExtensions" value=".java"/>-->
    </module>
    <module name="UniqueProperties">
        <!--<property name="fileExtensions" value=".java"/>-->
    </module>
    <module name="NewlineAtEndOfFile"/>

    <!-- Example of filter -->
    <!--
    <module name="SeverityMatchFilter">
        <property name="severity" value="warning"/>
        <property name="acceptOnMatch" value="false"/>
    </module>
    -->

    <!-- as we run on regression even on non-compiled files we need to skip exceptions on them -->
    <module name="SuppressionSingleFilter">
      <property name="message" value="Exception occurred while parsing"/>
      <property name="checks" value="Checker"/>
    </module>
</module>

rdiachenko · 2020-07-09T21:42:35Z

@rdiachenko yes, it is updated. here is patchConfig.xml that I use

@HuGanghui how can you explain this then? I compared patchConfig.xml from master with the one above you provided in the comment:

$ diff -u patchConfig.xml patchConfig-from-comment.xml 
--- patchConfig.xml	2020-07-02 21:17:11.682693828 +0100
+++ patchConfig-from-comment.xml	2020-07-09 22:38:50.570558601 +0100
@@ -100,7 +100,9 @@
     <!-- Size Violations -->
     <module name="FileLength">
         <property name="fileExtensions" value="java"/>
+        <property name="max" value="400"/>
     </module>
+
     <module name="LineLength">
         <property name="max" value="80"/>
     </module>
@@ -109,6 +111,26 @@
     <module name="FileTabCharacter">
         <property name="eachLine" value="false"/>
     </module>
+
+    <module name="RegexpOnFilename">
+        <property name="fileNamePattern" value="\.java$"/>
+    </module>
+    <module name="RegexpSingleline">
+        <property name="format" value="Test"/>
+    </module>
+    <module name="RegexpMultiline">
+        <property name="matchAcrossLines" value="true"/>
+        <property name="format" value="jupiter\.api"/>
+    </module>
+
+    <module name="OrderedProperties">
+        <!--<property name="fileExtensions" value=".java"/>-->
+    </module>
+    <module name="UniqueProperties">
+        <!--<property name="fileExtensions" value=".java"/>-->
+    </module>
+    <module name="NewlineAtEndOfFile"/>
+
     <!-- Example of filter -->
     <!--
     <module name="SeverityMatchFilter">

HuGanghui · 2020-07-10T03:15:21Z

@rdiachenko actually when I do diff report, I add some checks to make it easier to have violations according to

We need pack of violations from each Check , we can modify Checks settings in config to be more aggressive and prone to > violations.

but I do not add these checks in to master branch, Is necessary to add them in to it? If right, I will do it. Sorry for the confusion.

rdiachenko · 2020-07-10T21:24:48Z

@HuGanghui now, could you try to answer your question rereading initial comment:

@HuGanghui is the patchConfig.xml up to date? I mean, if I take a master branch and commit hashes from your report and build a report on my localhost, will it be the same as yours?

So, what is the problem having one config in the master and another config on your local?

HuGanghui · 2020-07-11T03:55:48Z

is the patchConfig.xml up to date?

yes, it is updated.

I mean, if I take a master branch and commit hashes from your report and build a report on my localhost, will it be the same as yours?

no, you should use patchConfig.xml that I mentioned in #21 (comment).

So, what is the problem having one config in the master and another config on your local?

Because I usually need to modify some checks' settings to get violations, and I think master/PR always contains patchConfig.xml's change is not very good, so I choose to update config when generate diff reports, but not add them to git commits.
If this is not good, I will change it.

baratali · 2020-07-11T16:35:48Z

@HuGanghui ,

need .properties files, but as mentioned in #31 (comment), diff reports can only display violations on .java files. So these two checks are special.

You can actually check properties files too with maven-checkstyle-plugin. Please see #31 (comment).
Please regenerate the report with -Dcheckstyle.includes="**/*" property. If there are still no violations for OrderedProperties
and UniqueProperties checks, please search for another commit with properties files in that repository.

baratali · 2020-07-11T16:47:50Z

@HuGanghui , also can you please reduce lines limit for FileLength check, because in the report there is only one violation from this check? And please remove fileExtensions property from it to check all files and have more violations.

romani · 2020-07-19T15:21:22Z

I closing this issue for now, as we have enough work in UTs for now.
We comeback to diff report when we have no ideas on how to be creative in UT inputs creation and we stabilize implementation and basic behavior.

romani added this to To do in Patch Suppression via automation Jun 20, 2020

HuGanghui mentioned this issue Jun 23, 2020

Links patch-filter diff report with LineLength check violations #31

Closed

romani mentioned this issue Jun 23, 2020

update README to provide instruction on how to use report generator #18

Closed

This comment has been minimized.

Sign in to view

This was referenced Jun 29, 2020

extend AutomaticBean in SuppressionPatchFilter #55

Closed

Issue #50: Specify a list of commit hashes to generate report for #53

Merged

HuGanghui mentioned this issue Jun 30, 2020

Update README.md to avoid Hidden bug about commitID #59

Closed

This was referenced Jul 16, 2020

Add Non-Treewalker filter tests to cover more cases #92

Closed

Find all TreeWalker Checks that need to use context strategy #100

Closed

romani closed this as completed Jul 19, 2020

Patch Suppression automation moved this from To do to Done Jul 19, 2020

Generate report that have violations from each Check in config #21

Generate report that have violations from each Check in config #21

Comments

romani commented Jun 20, 2020

romani commented Jun 23, 2020

HuGanghui commented Jun 24, 2020 • edited

rdiachenko commented Jun 24, 2020

romani commented Jun 24, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

HuGanghui commented Jun 25, 2020 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rdiachenko commented Jun 28, 2020

HuGanghui commented Jun 29, 2020

HuGanghui commented Jun 29, 2020 • edited

rdiachenko commented Jun 29, 2020 • edited

HuGanghui commented Jun 30, 2020 • edited

HuGanghui commented Jun 30, 2020 • edited

rdiachenko commented Jun 30, 2020 • edited

rdiachenko commented Jun 30, 2020 • edited

HuGanghui commented Jul 1, 2020 • edited

rdiachenko commented Jul 1, 2020

HuGanghui commented Jul 2, 2020 • edited

rdiachenko commented Jul 2, 2020 • edited

HuGanghui commented Jul 3, 2020

HuGanghui commented Jul 3, 2020 • edited

HuGanghui commented Jul 3, 2020 • edited

rdiachenko commented Jul 3, 2020

HuGanghui commented Jul 6, 2020

rdiachenko commented Jul 6, 2020

HuGanghui commented Jul 7, 2020 • edited

rdiachenko commented Jul 7, 2020

HuGanghui commented Jul 7, 2020

rdiachenko commented Jul 8, 2020

HuGanghui commented Jul 9, 2020

rdiachenko commented Jul 9, 2020

HuGanghui commented Jul 10, 2020 • edited

rdiachenko commented Jul 10, 2020

HuGanghui commented Jul 11, 2020 • edited

baratali commented Jul 11, 2020

baratali commented Jul 11, 2020

romani commented Jul 19, 2020

HuGanghui commented Jun 24, 2020 •

edited

HuGanghui commented Jun 25, 2020 •

edited

HuGanghui commented Jun 29, 2020 •

edited

rdiachenko commented Jun 29, 2020 •

edited

HuGanghui commented Jun 30, 2020 •

edited

HuGanghui commented Jun 30, 2020 •

edited

rdiachenko commented Jun 30, 2020 •

edited

rdiachenko commented Jun 30, 2020 •

edited

HuGanghui commented Jul 1, 2020 •

edited

HuGanghui commented Jul 2, 2020 •

edited

rdiachenko commented Jul 2, 2020 •

edited

HuGanghui commented Jul 3, 2020 •

edited

HuGanghui commented Jul 3, 2020 •

edited

HuGanghui commented Jul 7, 2020 •

edited

HuGanghui commented Jul 10, 2020 •

edited

HuGanghui commented Jul 11, 2020 •

edited