-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Processors should be considered as rightful dependencies #82
Processors should be considered as rightful dependencies #82
Conversation
Codecov Report
@@ Coverage Diff @@
## master #82 +/- ##
===========================================
- Coverage 6.00% 5.96% -0.05%
Complexity 23 23
===========================================
Files 26 26
Lines 966 973 +7
Branches 133 130 -3
===========================================
Hits 58 58
- Misses 896 903 +7
Partials 12 12 Continue to review full report at Codecov.
|
Side-effect on the |
Hi @afillatre, I'm OK with the proposal. I agree the |
private Set<Artifact> collectUsedArtifacts(Map<Artifact, Set<String>> artifactClassMap, | ||
Set<String> referencedClasses) { | ||
Set<Artifact> usedArtifacts = new HashSet<>(); | ||
// find for used members in each class in the dependency classes | ||
for (String clazz : referencedClasses) { | ||
Artifact artifact = findArtifactForClassName(artifactClassMap, clazz); | ||
if (artifact != null) { | ||
if (!artifactUsedClassesMap.containsKey(artifact)) { | ||
artifactUsedClassesMap.put(artifact, new HashSet<>()); | ||
} | ||
artifactUsedClassesMap.get(artifact).add(clazz); | ||
usedArtifacts.add(artifact); | ||
} | ||
findArtifactForClassName(artifactClassMap, clazz) | ||
.ifPresent(artifact -> artifactUsedClassesMap.putIfAbsent(artifact, new HashSet<>())); | ||
} | ||
return usedArtifacts; | ||
return artifactUsedClassesMap.keySet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this refactoring!
There are too many null
in the project :)
@MavenTest | ||
@DisplayName("Test that DepClean runs in a Maven project with processors") | ||
void processor_used(MavenExecutionResult result) { | ||
assertThat(result).isSuccessful().out() | ||
.plain().contains( | ||
"-------------------------------------------------------", | ||
" D E P C L E A N A N A L Y S I S R E S U L T S", | ||
"-------------------------------------------------------", | ||
"USED DIRECT DEPENDENCIES [1]: ", | ||
" org.mapstruct:mapstruct-processor:1.4.2.Final:provided (1 MB)", | ||
"USED INHERITED DEPENDENCIES [0]: ", | ||
"USED TRANSITIVE DEPENDENCIES [1]: ", | ||
" com.fasterxml.jackson.core:jackson-core:2.12.2:compile (356 KB)", | ||
"POTENTIALLY UNUSED DIRECT DEPENDENCIES [1]: ", | ||
" com.fasterxml.jackson.core:jackson-databind:2.12.2:compile (1 MB)", | ||
"POTENTIALLY UNUSED INHERITED DEPENDENCIES [0]: ", | ||
"POTENTIALLY UNUSED TRANSITIVE DEPENDENCIES [1]: ", | ||
" com.fasterxml.jackson.core:jackson-annotations:2.12.2:compile (73 KB)" | ||
); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that you included a proper IT!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a very hard time debugging the plugin though, maybe I'm doing something wrong... I was unable to catch a breakpoint in the core project.
With more ITs, this big assertion can be refined, though.
Kudos, SonarCloud Quality Gate passed! |
Co-authored-by: Alexandre Fillatre <[email protected]> Co-authored-by: cesarsotovalero <[email protected]>
fixes #76