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

Stephan Schroevers commented on MCHECKSTYLE-444:
------------------------------------------------

TBH, when I first read about {{{}{{excludeGeneratedSources}}{}}}, I did expect 
all generated files to be covered. I guess that this is in part because when 
Maven plugins generate files, they generally allow users to specify the target 
directory, but not whether the result is designated "source" or "resource". But 
I'm willing to admit that this is simply "user error" from my side.

The second scenario I describe however does point at a bug proper: 
{{{{excludeGeneratedSources}}}} excludes generated sources in a directory 
{{{{X}}}} _only_ if {{{{X}}}} isn't also designated a resource directory. 
That's what happens in case of {{{}META-INF/sun-jaxb.episode{}}}, which causes 
all generated _Java_ files to be analyzed by Checkstyle. One can work around 
that by setting e.g. {{<resourceIncludes>**/*.properties</resourceIncludes>}} 
(which happens to be the default) but from a user perspective it does not make 
sense that by restricting resources, I actually exclude source files.

Put another way: the plugin current doesn't properly distinguish between 
(generated) sources and resources. If I read the code correctly, one can also 
see that 
[here|https://github.com/apache/maven-checkstyle-plugin/blob/a96a4da1ec18a755f232fd3ac36ace42de8b7283/src/main/java/org/apache/maven/plugins/checkstyle/exec/DefaultCheckstyleExecutor.java#L435],
 where all sources and resources are eventually collected in a single place for 
input to Checkstyle.

That said, my ticket title does hint a feature request, not a bug report, and 
the existing properties already distinguish between sources and resources. So 
indeed it makes more sense to introduce a separate flag. I guess it means I 
should have filed two distinct tickets:
 # One to suggest the introduction of an {{excludeGeneratedResources}} flag.
 # One to flag that the plugin doesn't properly keep sources and resources 
apart.

For our use case a solution to (1) would suffice, as we'd simply configure both 
{{excludeGeneratedSources}} and {{{}excludeGeneratedResources{}}}. On cursory 
inspection this change would likely not be too hard; I can open a PR for that. 
A solution to (2) looks like it'd require a larger refactoring; it's unlikely 
I'll find time for that any time soon.

> Support exclusion of generated (test) resources
> -----------------------------------------------
>
>                 Key: MCHECKSTYLE-444
>                 URL: https://issues.apache.org/jira/browse/MCHECKSTYLE-444
>             Project: Maven Checkstyle Plugin
>          Issue Type: Bug
>          Components: checkstyle:check
>    Affects Versions: 3.3.1
>            Reporter: Stephan Schroevers
>            Priority: Major
>
> In the context of MCHECKSTYLE-412 a new {{excludeGeneratedSources}} option 
> was introduced. This feature does not exclude generated {_}resources{_}, 
> however. Generated resources are not uncommon:
>  # The 
> [license-maven-plugin|https://github.com/mojohaus/license-maven-plugin] 
> attaches a {{THIRD-PARTY.txt}} file.
>  # The [hisrc-basicjaxb plugin|https://github.com/patrodyne/hisrc-basicjaxb] 
> attaches a {{META-INF/sun-jaxb.episode}} file, located in a directory that 
> also contains generated sources ((!)).
> As a result, if one sets
> {code:xml}
> <excludeGeneratedSources>true</excludeGeneratedSources>
> <resourceIncludes>**/*</resourceIncludes>
> {code}
> Then the plugin will flag {{THIRD-PARTY.txt}} files (not completely 
> unexpected) as well as XJC-generated Java files (very unexpected).
> Possible fixes:
>  # Update the {{excludeGeneratedSources}} flag to also cover (test) resource 
> directories.
>  # Introduce an analogous {{excludeGeneratedResources}} flag.
> (The first option may seem less flexible, but would be less surprising i.c.w. 
> plugins that generate sources and resources into the same directory, as in 
> example (2) above.)
> I'm open to contributing a PR for this change; it would be nice to first hear 
> from the maintainers whether they prefer approach (1) or (2).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to