Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .filter(a -> a.getEffectiveVersion() != null) | ||
| .filter(artifact -> !isExcluded(artifact.getVersion())) | ||
| .collect(groupingBy(Artifact::getEffectiveVersion, toList())); | ||
| .collect(groupingBy(Artifact::uniqueness, toList())); |
There was a problem hiding this comment.
🔴 Grouping by uniqueness() causes false-positive redundant-property violations when a shared property is used across different groupIds
The change from groupingBy(Artifact::getEffectiveVersion) to groupingBy(Artifact::uniqueness) (where uniqueness() returns groupId:effectiveVersion) breaks the detection of shared properties used across different groupIds.
Consider two dependencies from different groupIds that both reference the same property: withDependency("com.a", "artifact-a", "common.version", "1.0.0") and withDependency("com.b", "artifact-b", "common.version", "1.0.0"). Previously these were grouped together under effective version 1.0.0 (2 artifacts, both using the property → no violation). Now they form two separate groups (com.a:1.0.0 and com.b:1.0.0), each with 1 artifact that has hasImplicitVersion() == true, which triggers redundantPropertyViolation for each — a false positive, since the property IS legitimately shared.
Concrete example of the false positive path through scan()
For each single-artifact group where propertyCount == 1 (line 89), redundantPropertyViolation is called. The property is actually used in 2 places, but the grouping hides this. The fix should consider a uniqueness key that avoids coincidental version matches (the original problem) without breaking shared property detection across groupIds.
Prompt for agents
In src/main/java/com/solubris/enforcer/VersionPropertyRule.java at line 73, the grouping by Artifact::uniqueness (which is groupId:effectiveVersion) incorrectly splits artifacts that share a property across different groupIds, causing false-positive redundant-property violations.
The fix needs to handle two competing concerns:
1. Coincidental versions (same version string, different groupIds, no shared property) should NOT be flagged as needing a shared property.
2. Shared properties across different groupIds SHOULD be recognized as legitimate multi-use properties.
One approach: keep grouping by effectiveVersion, but in the missingPropertyViolation check (the propertyCount == 0 case on line 86), add an additional filter to only flag artifacts that share the same groupId. Alternatively, do a two-pass analysis: first detect shared properties, then check for coincidental explicit versions only among same-groupId artifacts.
Was this helpful? React with 👍 or 👎 to provide feedback.
Test Results35 tests 35 ✅ 2s ⏱️ Results for commit 4c8093d. |
1 participant
Uh oh!
There was an error while loading. Please reload this page.