[I] excludeFromFailureFile and maxAllowedViolations gate CPD by aggregate/file-pair, not by duplicate-block identity [maven-pmd-plugin]

["yunusdim (via GitHub)" <gi...@apache.org>]

yunusdim opened a new issue, #724:
URL: https://github.com/apache/maven-pmd-plugin/issues/724

   ### Affected version
   
   3.28.1-SNAPSHOT (also present in released versions -- read from source, not yet reproduced live against Maven Central due to a network restriction in the environment I used, see note below)
   
   ### Bug description
   
   Note: this is a source-code-only finding (AbstractPmdViolationCheckMojo.executeCheck/getViolations, ExcludeDuplicationsFromFile.isExcludedFromFailure), not yet reproduced against a live build -- repo.maven.apache.org was blocked in the sandbox I was working from. Filing anyway because the mechanism is unambiguous from the source; happy to attach a live repro if useful.
   
   Two separate aggregate/no-identity gaps in how cpd-check consumes CPD's Match list:
   
   Gap 1) pmd.maxAllowedViolations (default 0): the gate is failureCount > maxAllowedViolations, a raw count of duplications, no location. If a team tolerates 1 known duplication and later fixes it while a different, unrelated duplication appears elsewhere, failureCount is still 1 -- build stays green.
   
   Gap 2) pmd.excludeFromFailureFile: isExcludedFromFailure(Match) only collects the SET OF FILE PATHS involved in a match, and excludes the duplication if both paths match a group already listed in the exclusion file -- it never looks at line ranges or block content/hash. Once a file pair (A, B) is in the exclusion list, ANY future duplication between A and B, anywhere in either file, with any content, is excluded forever.
   
   Repro (conceptual, not yet run live): 1) create a real duplication between A.java and B.java, add "A,B" to excludeFromFailureFile (or set maxAllowedViolations=1); 2) mvn pmd:cpd pmd:cpd-check -> green; 3) remove that duplication, introduce a different, unrelated duplication within the same A/B pair (or, for maxAllowedViolations, anywhere in the project) of similar size; 4) run again with the same config -> still green, now masking a completely different duplicate.
   
   Why it matters: this is the standard way most teams gate CPD in CI (via the Maven plugin, not the bare CPD CLI), so the aggregate/file-pair blind spot applies to the common case, not an edge case.
   
   Suggest: for excludeFromFailureFile, match on block identity (line range and/or content hash) instead of just the file-pair; for maxAllowedViolations, consider tracking identity across runs the same way, so a same-size swap surfaces instead of netting to zero.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] excludeFromFailureFile and maxAllowedViolations gate CPD by aggregate/file-pair, not by duplicate-block identity [maven-pmd-plugin]

["elharo (via GitHub)" <gi...@apache.org>]

elharo closed issue #724: excludeFromFailureFile and maxAllowedViolations gate CPD by aggregate/file-pair, not by duplicate-block identity
URL: https://github.com/apache/maven-pmd-plugin/issues/724


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] excludeFromFailureFile and maxAllowedViolations gate CPD by aggregate/file-pair, not by duplicate-block identity [maven-pmd-plugin]

["elharo (via GitHub)" <gi...@apache.org>]

elharo commented on issue #724:
URL: https://github.com/apache/maven-pmd-plugin/issues/724#issuecomment-6108375902

   GAP 1 is, I think, working as intended and documented and expected. 
   
   I think Gap 2 is as well. 
   
   This seems to be requesting that the plugin behave in a different way, but the current approach does not feel like a bug.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org