checkstyle / patch-filters

Suppression Filter for Checkstyle that is based on patch file
GNU Lesser General Public License v2.1
4 stars 7 forks source link

Report on files being validated against #346

Open rnveach opened 1 year ago

rnveach commented 1 year ago

Identified at https://github.com/checkstyle/eclipse-cs/pull/380#issuecomment-1310507743 ,

It was not clear why we were getting random issues and then it became clear patch-filters prevented a Checkstyle run in some cases, which is its purpose.

It would be helpful if patch-filters reported something minor like "No files identified as changed" or "Identified X files as changed" so it is clear when allows Checkstyle to run and not run.

I don't think it would have helped this case, but it will make what it is doing clearer.

romani commented 1 year ago

It just another filter for checkstyle. We do not have ability communicate to execution module. Can you share more details on how you see it !

rnveach commented 1 year ago

Currently output is:

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.2.0:check (sevntu-checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ net.sf.eclipsecs-updatesite ---

Following is what I think should be displayed:

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters prevented any files from being validated.
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.2.0:check (sevntu-checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters allowed 1 file(s) to be validated.
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ net.sf.eclipsecs-updatesite ---

But your right, as just a normal filter this is not currently possible. This would either have to be more of a BeforeExecutionFileFilter, which would still require it to be allowed to print output like an Audit Listener, or create a separate Audit Listener to display notifications all together. A second module is not how I would want this to go, but the only other option then is a change to allow a filter to also be an audit listener (implement Filter and AuditListener, both are interfaces).

Also displaying number of files seems pointless now, as patch-filters slims down the file to where the changes are, so its not the full file validation. I guess my final recommendation then, would be the number of violations patch-filters prevented.

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters suppressed XXX violation(s) in unchanged files and lines.
[INFO] You have 0 Checkstyle violations.
[INFO] 
romani commented 1 year ago

Output that you reference is output of plugin, not a core checkstyle library

rnveach commented 1 year ago

Trimmed from my examples above, the output I am asking from this plugin is one of the following:

Patch-Filters prevented any files from being validated. Patch-Filters allowed 1 file(s) to be validated. Patch-Filters suppressed XXX violation(s) in unchanged files and lines.

I am just looking for some feedback form patch-filters on what it is doing so its a bit clearer to the user(s) who want to know.

romani commented 1 year ago

asking from this plugin

How you envision filter to print output to plugin output? This project is just filter implementation.

rnveach commented 1 year ago

Another reason to provide some feedback is to show progress to removing all violations for reporting. The purpose of the filter is to allow users to resolve new violations slowly.

There is no way to know right now how many violations are left, or when all violations are now resolved and project team can discuss removing suppression filter if they wish. It may be beneficial for this number to be seen to show management that they are now fully in compliance with all styling for project.

Also, upgrading to new version of Checkstyle or adding new modules will not show how many new violations are being added also, as the only thing initially changing in commit is the configuration file.

rnveach commented 1 year ago

How you envision filter to print output to plugin output?

This will most likely require a lifecycle change from main project for proper implementation. I am open to ideas.

Maybe all modules can be given a custom output function where the data makes it way to AuditListener to handle and print.

This may just be similar to a violation log (with info severity), so maybe all type of modules, including filters, should be allowed to print violations. The only difference is this type of printing discussed here won't have a file attached to it.