contribution icon indicating copy to clipboard operation
contribution copied to clipboard

Issue #273: converts diff.groovy to use Checkstyle's CLI

Open rnveach opened this issue 2 years ago • 6 comments

Issue #273

Supplemental Checkstyle PR: https://github.com/checkstyle/checkstyle/pull/12576

rnveach avatar Dec 31 '22 19:12 rnveach

@rnveach , please create PR to main repo to use your branch and validate and action is working to generate diff report.

romani avatar Jan 02 '23 06:01 romani

create PR to main repo to use your branch

Done.

rnveach avatar Jan 02 '23 17:01 rnveach

Did we test failure at https://github.com/checkstyle/checkstyle/pull/12576, i.e. throw an exception in a check?

It depends on what scenario you are looking for.

Using <property name="haltOnException" value="false"/> will ignore the exception and keep going. Similar with --continueOnError since this only checks the exit code from Checkstyle. We cannot identify an exception from the CLI, only the exit code and violations and exception both produce exit code > 0.

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/Main.java#L135 https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/Main.java#L141

I did confirm locally that removing haltOnException and continueOnError, diff.groovy will stop when an exception occurs. I tested an exception from the config:

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

    <module name="RegexpSingleline">
        <property name="format" value="test(.*"/>
    </module>
</module>

With output:

> groovy diff.groovy -c M:\checkstyleWorkspaceEclipse\contribution\checkstyle-tester\my_check.xml -r Z:\checkstyle -b master -p master -l M:\checkstyleWorkspaceEclipse\contribution\checkstyle-tester\projects-to-test-on.properties

....
Running command: java -jar Z:\checkstyle\target\checkstyle-10.7.1-SNAPSHOT-all.jar -c M:\checkstyleWorkspaceEclipse\contribution\checkstyle-tester\my_check.xml repositories\guava -f xml -o reports\guava\checkstyle-result.xml -e repositories\guava\.git -e repositories\guava\.hg
Exception in thread "main" java.util.regex.PatternSyntaxException: Unclosed group near index 7
test(.*
        at java.base/java.util.regex.Pattern.error(Pattern.java:2027)
        at java.base/java.util.regex.Pattern.accept(Pattern.java:1877)
        at java.base/java.util.regex.Pattern.group0(Pattern.java:3060)
        at java.base/java.util.regex.Pattern.sequence(Pattern.java:2123)
        at java.base/java.util.regex.Pattern.expr(Pattern.java:2068)
        at java.base/java.util.regex.Pattern.compile(Pattern.java:1782)
        at java.base/java.util.regex.Pattern.<init>(Pattern.java:1428)
        at java.base/java.util.regex.Pattern.compile(Pattern.java:1094)
        at com.puppycrawl.tools.checkstyle.checks.regexp.DetectorOptions$Builder.createPattern(DetectorOptions.java:264)
        at java.base/java.util.Optional.map(Optional.java:265)
        at com.puppycrawl.tools.checkstyle.checks.regexp.DetectorOptions$Builder.build(DetectorOptions.java:249)
        at com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineCheck.beginProcessing(RegexpSinglelineCheck.java:227)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:218)
        at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:415)
        at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:338)
        at com.puppycrawl.tools.checkstyle.Main.execute(Main.java:195)
        at com.puppycrawl.tools.checkstyle.Main.main(Main.java:130)
Caught: groovy.lang.GroovyRuntimeException: Error: !
groovy.lang.GroovyRuntimeException: Error: !
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at diff.executeCliCmd(diff.groovy:698)
        at diff$executeCliCmd$17.callCurrent(Unknown Source)
        at diff.runCliExecution(diff.groovy:660)
        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 diff$_generateCheckstyleReport_closure4.doCall(diff.groovy:309)
        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 diff.generateCheckstyleReport(diff.groovy:271)
        at diff$generateCheckstyleReport$10.callCurrent(Unknown Source)
        at diff.launchCheckstyleReport(diff.groovy:233)
        at diff$launchCheckstyleReport$6.callCurrent(Unknown Source)
        at diff.run(diff.groovy:31)
        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)
>

rnveach avatar Feb 11 '23 18:02 rnveach

All our no-exception tests seem to use continueOnError. https://github.com/checkstyle/checkstyle/blob/04fea7164f272af3b73621e4753df5f207033f92/.ci/no-exception-test.sh#L127

This will have to be removed, but there is the one config I pointed out to that uses error. https://github.com/checkstyle/checkstyle/blob/d3a5f9a5ba4a1f2df28900dd70f2204c92943521/.ci/no-exception-test.sh#L44-L45

It looks like continueOnError is not a solution, and something in main repo will be needed.

Example run: https://github.com/checkstyle/checkstyle/actions/runs/4152812321/jobs/7184025441#step:5:320 No exception run kept going after exception and only failed at diff-report generation.

rnveach avatar Feb 11 '23 19:02 rnveach

@nrmancuso , @rnveach , is there easy way to finish this PR ?

romani avatar Jul 02 '23 14:07 romani

The only easy way is to complete everything needed for the transfer ( https://github.com/checkstyle/contribution/pull/747#issuecomment-1426856400 ). This is the only PR in this repo I was working on for this issue.

rnveach avatar Jul 03 '23 19:07 rnveach