[jira] [Commented] (MCHECKSTYLE-314) checkstyle:check should invoke the execution of this plugin's goal "checkstyle" prior to executing itself

["Stig Rohde Døssing (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MCHECKSTYLE-314?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16036129#comment-16036129 ] 

Stig Rohde Døssing commented on MCHECKSTYLE-314:
------------------------------------------------

[~michael-o] This fix seems broken to me. checkstyle:check runs Checkstyle on its own, so with this patch Checkstyle ends up running twice when checkstyle:check is invoked. For example, we have the following configuration for Apache Storm (plugin version is bumped from 2.17, but that's the only change):
{code}
                <plugin>
                    <!--To support checkstyle goals. For example: "mvn checkstyle:checkstyle"-->
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-checkstyle-plugin</artifactId>
                    <version>3.0.0-SNAPSHOT</version>
                    <dependencies>
                        <dependency>
                            <groupId>com.puppycrawl.tools</groupId>
                            <artifactId>checkstyle</artifactId>
                            <!-- If you change this, you should also update the storm_checkstyle.xml file to be
                            based on the google_checks.xml from the version of checkstyle you are choosing. -->
                            <version>7.7</version>
                        </dependency>
                    </dependencies>
                    <executions>
                        <execution>
                            <id>validate</id>
                            <phase>validate</phase>
                            <configuration>
                                <configLocation>storm-buildtools/storm_checkstyle.xml</configLocation>
                                <encoding>UTF-8</encoding>
                                <failOnViolation>true</failOnViolation>
                                <logViolationsToConsole>false</logViolationsToConsole>
                                <outputFile>target/checkstyle-violation.xml</outputFile>
                                <violationSeverity>warning</violationSeverity>
                            </configuration>
                            <goals>
                                <goal>check</goal>
                            </goals>
                        </execution>
                    </executions>
                </plugin>
{code}

With version 2.17 we correctly get a checkstyle-violation.xml in our target folder, and warnings in the console when there are violations.
With the code currently on trunk we get an additional 
{code}
[INFO] There are 34506 errors reported by Checkstyle 7.7 with sun_checks.xml ruleset.
[WARNING] Unable to locate Source XRef to link to - DISABLED
{code}
which is clearly misleading, since we are not using the sun_checks.xml ruleset. We also end up with both the expected checkstyle-violation.xml and an additional checkstyle-results.xml in the target folder. The checkstyle-results.xml is generated by the checkstyle:checkstyle execution, and so contains violations for the Sun ruleset. It seems likely to me that someone will accidentally open the wrong file at some point, and start correcting the wrong violations.

I think either this change should be reverted, or the CheckstyleViolationCheckMojo should be rewritten so it doesn't run Checkstyle itself, but delegates that task to the checkstyle:checkstyle goal. I'm not familiar enough with Mojo development to know if it's possible to do the second option without breaking backward compatibility. The PMD plugin is able to make {{check}} run {{pmd}} because they have a cleaner separation between the {{pmd}} and {{check}} goals, e.g. the {{check}} goal doesn't have any parameters to configure PMD execution. This makes it easy for them to just invoke the {{pmd}} goal, because they don't have to deal with trying to pass configuration from the {{check}} goal to {{pmd}}.

> checkstyle:check should invoke the execution of this plugin's goal "checkstyle" prior to executing itself
> ---------------------------------------------------------------------------------------------------------
>
>                 Key: MCHECKSTYLE-314
>                 URL: https://issues.apache.org/jira/browse/MCHECKSTYLE-314
>             Project: Maven Checkstyle Plugin
>          Issue Type: Improvement
>    Affects Versions: 2.17
>            Reporter: Roman Ivanov
>            Assignee: Michael Osipov
>             Fix For: 3.0.0
>
>
> I run into problem with using checkstyles goal "checkstyle:check" in sevntu.checkstyle project (https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/pom.xml#L75) as it always use default Sun config when I run "checkstyle:check", It took me a while to figure out why that does not work.
> I had a habit and convenience to run in such a ways pmd and findbug, just "mvn pmd:check", "mvn findbug:check" it is human friendly.
> PMD and Findbugs plugins already do this:
> http://gleclaire.github.io/findbugs-maven-plugin/check-mojo.html
> https://maven.apache.org/plugins/maven-pmd-plugin/check-mojo.html
> search for "Invokes the execution of this plugin's goal"
> vs
> http://maven.apache.org/plugins/maven-checkstyle-plugin/check-mojo.html
> Problem was also raised at : http://stackoverflow.com/questions/11106767/maven-checkstylecheck-not-working
> Please add support of this, most users are not professionals in Maven. I use Maven for many years and it took me too much time to find a reason why it does not work.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)