Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 2.1
    • 2.13
    • None
    • None

    Description

      It would be great if this plugin would support multiple source folders added by http://mojo.codehaus.org/build-helper-maven-plugin/ (or similar), and by default inspect sources from these folders instead of just ${project.build.sourceDirectory}. Correspondingly with respect to test sources if those are configured to be included.

      There are other plugins available solving this problem (somehow), eg:

      Maybe they can give some inspiration for how to make this possible?

      Attachments

        Issue Links

          Activity

            jan.palmquist Jan Palmquist added a comment -

            Many other plugins have the same limitation, and after looking at the pmd-plugin, I found a way to solve my issue temporarily simply letting <sourceDirectory> point to the parent project src/main instead of pointing to src/main/java.
            This, however, breaks the xref-links since the src/main is not the actual source folder.
            In my case, I have an additional source folder src/main/java-generated holding generated source code which I wan't to be separated from the hand written source in order to delete it whenever I run mvn clean...

            jan.palmquist Jan Palmquist added a comment - Many other plugins have the same limitation, and after looking at the pmd-plugin, I found a way to solve my issue temporarily simply letting <sourceDirectory> point to the parent project src/main instead of pointing to src/main/java. This, however, breaks the xref-links since the src/main is not the actual source folder. In my case, I have an additional source folder src/main/java-generated holding generated source code which I wan't to be separated from the hand written source in order to delete it whenever I run mvn clean...
            michael.tamm Michael Tamm added a comment -

            src/main/resources should also be included a a source directory by default, and the default for the includes configuration should be "*/.java,*/.properties" - otherwise the <module name="Translation" /> (see http://checkstyle.sourceforge.net/config_misc.html#Translation) does not work.

            michael.tamm Michael Tamm added a comment - src/main/resources should also be included a a source directory by default, and the default for the includes configuration should be "* / .java,* / .properties" - otherwise the <module name="Translation" /> (see http://checkstyle.sourceforge.net/config_misc.html#Translation ) does not work.

            Michael,

            With the solution of MCHECKSTYLE-40, your problem should be solved. Please try the latest 2.2-SNAPSHOT version.

            dennisl@apache.org Dennis Lundberg added a comment - Michael, With the solution of MCHECKSTYLE-40 , your problem should be solved. Please try the latest 2.2-SNAPSHOT version.
            ttres Taciano Tres added a comment -

            Is this already solved?

            ttres Taciano Tres added a comment - Is this already solved?

            No, I don't see this solved... I think the Checkstyle plugin should ask the MavenProject for its compileSourceRoots. See:

            http://maven.apache.org/ref/3.0.3/maven-core/apidocs/org/apache/maven/project/MavenProject.html#getCompileSourceRoots()

            awhitford Anthony Whitford added a comment - No, I don't see this solved... I think the Checkstyle plugin should ask the MavenProject for its compileSourceRoots. See: http://maven.apache.org/ref/3.0.3/maven-core/apidocs/org/apache/maven/project/MavenProject.html#getCompileSourceRoots( )
            rfscholte Robert Scholte added a comment -

            Fixed in r1618905

            rfscholte Robert Scholte added a comment - Fixed in r1618905

            How does this deal with generated sources?

            arlol Arlo Louis O'Keeffe added a comment - How does this deal with generated sources?
            rfscholte Robert Scholte added a comment -

            arlol, you probably already guessed the answer: since these are compileSourceRoots, they are included as well. Maven doesn't distinguish generated sourceroots from real sourceroots, you can hope that the generated end up in the target-directory. Up until 2.12 you could only specify 1 source directory, now you can have more and specify if you want.
            I'd love to implement MCHECKSTYLE-187, which would solve this issue as well, but that will have a huge impact.

            rfscholte Robert Scholte added a comment - arlol , you probably already guessed the answer: since these are compileSourceRoots, they are included as well. Maven doesn't distinguish generated sourceroots from real sourceroots, you can hope that the generated end up in the target-directory. Up until 2.12 you could only specify 1 source directory, now you can have more and specify if you want. I'd love to implement MCHECKSTYLE-187 , which would solve this issue as well, but that will have a huge impact.
            michael-o Michael Osipov added a comment -

            rfscholte, why did you actually introduce another @Parameter variable instead of calling project.getCompileSourceRoots() as other plugins do?

            michael-o Michael Osipov added a comment - rfscholte , why did you actually introduce another @Parameter variable instead of calling project.getCompileSourceRoots() as other plugins do?
            rfscholte Robert Scholte added a comment - - edited

            michael-o, for several reasons:
            1. backwards compatibility. With previous version you could change the sourceDirectory, this should still be possible with the current plugin.
            2. checkstyle is a bit different compared to other plugins. You don't always have control over the style of the java-files, especially if these files are generated. I could assume that every generated source-directory is placed under target/, but that would make the plugin much more complicator.
            3. the checkstyle-plugin shouldn't be aware of other plugins, like it would allow sourceDirectories added by the buildhelper-maven-plugin, but refuse the sourceDirectories of jaxb-maven-plugin.
            So I exposed sourceDirectories with default project.getCompileSourceRoots() (as your wish), which is already better compared to the previous versions of the checkstyle-plugin.
            As said in my latest comment: the checkstyle is actually using the wrong roots, since it is not only about java-files. It should be:
            main: includes **/*, excludes src/test/**/*, target/**/*
            test: include src/test/**/*
            But that's a whole new approach, so a huge impact. (and even then you should offer users to change it)

            rfscholte Robert Scholte added a comment - - edited michael-o , for several reasons: 1. backwards compatibility. With previous version you could change the sourceDirectory , this should still be possible with the current plugin. 2. checkstyle is a bit different compared to other plugins. You don't always have control over the style of the java-files, especially if these files are generated. I could assume that every generated source-directory is placed under target/ , but that would make the plugin much more complicator. 3. the checkstyle-plugin shouldn't be aware of other plugins, like it would allow sourceDirectories added by the buildhelper-maven-plugin, but refuse the sourceDirectories of jaxb-maven-plugin. So I exposed sourceDirectories with default project.getCompileSourceRoots() (as your wish), which is already better compared to the previous versions of the checkstyle-plugin. As said in my latest comment: the checkstyle is actually using the wrong roots, since it is not only about java-files. It should be: main: includes **/* , excludes src/test/**/*, target/**/* test: include src/test/**/* But that's a whole new approach, so a huge impact. (and even then you should offer users to change it)
            hboutemy Herve Boutemy added a comment -

            uh, why check generated sources by default?

            this is causing problems, since plugin-tools HelpMojo generated file, for example, doesn't follow conventions (and it can't follow conventions from every team using it: it has its own conventions)

            is there a way to automatically remove generated sources directory?

            hboutemy Herve Boutemy added a comment - uh, why check generated sources by default? this is causing problems, since plugin-tools HelpMojo generated file, for example, doesn't follow conventions (and it can't follow conventions from every team using it: it has its own conventions) is there a way to automatically remove generated sources directory?
            rfscholte Robert Scholte added a comment -

            Now we have to developers with opposites wishes, which was exactly what I expected to happen. The current implementation meets in the middle.
            The MavenProject should split the generated sources from the main sources, now it's hard to define which folders are used for generation. We should hope they all end up in the target-directory. So yes, we might exclude target/**/* by default to cover most usecases, but that's a new request.

            rfscholte Robert Scholte added a comment - Now we have to developers with opposites wishes, which was exactly what I expected to happen. The current implementation meets in the middle. The MavenProject should split the generated sources from the main sources, now it's hard to define which folders are used for generation. We should hope they all end up in the target-directory. So yes, we might exclude target/**/* by default to cover most usecases, but that's a new request.
            hboutemy Herve Boutemy added a comment -

            exclude target/*/: perfect idea, that will do the job for me

            now we have time to think more about it: does it deserve a new option or not? or simply documentation?

            hboutemy Herve Boutemy added a comment - exclude target/* / : perfect idea, that will do the job for me now we have time to think more about it: does it deserve a new option or not? or simply documentation?
            hboutemy Herve Boutemy added a comment -

            just tested on dist-tool-plugin: target/*/ doesn't work...

            hboutemy Herve Boutemy added a comment - just tested on dist-tool-plugin: target/* / doesn't work...

            This change that now applies Checkstyle to generated-source is a major problem for me too.

            I thought that I could work around it by being explicit like:

            <configuration>
              <sourceDirectories>
                <sourceDirectory>${basedir}/src/main/java</sourceDirectory>
              </sourceDirectories>
              <testSourceDirectories>
                <testSourceDirectory>${basedir}/src/test/java</testSourceDirectory>
              </testSourceDirectories>
            </configuration>
            

            but this doesn't seem to make it work.

            Note that for the PMD plugin, it appears that it has an excludeRoots configuration property, and I use it like:

              <compileSourceRoots>${project.compileSourceRoots}</compileSourceRoots>
              <excludeRoots>
                <excludeRoot>target/generated-sources/jaxb</excludeRoot>
                <excludeRoot>target/generated-sources/xjc</excludeRoot>
                <excludeRoot>target/generated-sources/wsimport</excludeRoot>
              </excludeRoots>
            
            awhitford Anthony Whitford added a comment - This change that now applies Checkstyle to generated-source is a major problem for me too. I thought that I could work around it by being explicit like: <configuration> <sourceDirectories> <sourceDirectory> ${basedir}/src/main/java </sourceDirectory> </sourceDirectories> <testSourceDirectories> <testSourceDirectory> ${basedir}/src/test/java </testSourceDirectory> </testSourceDirectories> </configuration> but this doesn't seem to make it work. Note that for the PMD plugin, it appears that it has an excludeRoots configuration property, and I use it like: <compileSourceRoots> ${project.compileSourceRoots} </compileSourceRoots> <excludeRoots> <excludeRoot> target/generated-sources/jaxb </excludeRoot> <excludeRoot> target/generated-sources/xjc </excludeRoot> <excludeRoot> target/generated-sources/wsimport </excludeRoot> </excludeRoots>

            I was able to restore prior functionality with the following configuration:

            <configuration>
              <sourceDirectory>src/main/java</sourceDirectory>
              <testSourceDirectory>src/test/java</testSourceDirectory>
            </configuration>
            
            awhitford Anthony Whitford added a comment - I was able to restore prior functionality with the following configuration: <configuration> <sourceDirectory> src/main/java </sourceDirectory> <testSourceDirectory> src/test/java </testSourceDirectory> </configuration>
            antonsmyk@gmail.com Anton Smyk added a comment -

            Actually that worked for Maven 3.3.9 but not for 3.0.5. Here is corrected config satisfying both versions:

            <configuration>
              <sourceDirectories>
                <sourceDirectory>src/main/java</sourceDirectory>
              </sourceDirectories>
              <testSourceDirectories>
                <testSourceDirectory>src/test/java</testSourceDirectory>
              </testSourceDirectories>
            </configuration>
            
            antonsmyk@gmail.com Anton Smyk added a comment - Actually that worked for Maven 3.3.9 but not for 3.0.5. Here is corrected config satisfying both versions: <configuration> <sourceDirectories> <sourceDirectory> src/main/java </sourceDirectory> </sourceDirectories> <testSourceDirectories> <testSourceDirectory> src/test/java </testSourceDirectory> </testSourceDirectories> </configuration>

            People

              rfscholte Robert Scholte
              jan.palmquist Jan Palmquist
              Votes:
              12 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: