[MNG-7791] Split validation issues by "locality" aspect (#1122)

Changes:
* introduce "internal" (to project) and "external" aspect
* make "inline" default as it was in 3.9.0 and 3.9.1
* remove "default" report level
* summary and verbose reports "internal only" or ALL issues

Example of outputs while building maven-3.8.x:
https://gist.github.com/cstamas/defd6a52939f5a277bf5fc669c01aedb

---

https://issues.apache.org/jira/browse/MNG-7791
This commit is contained in:
Tamas Cservenak 2023-05-26 18:22:12 +02:00
parent 6f136ef4d2
commit 46807790f6
11 changed files with 161 additions and 73 deletions

View File

@ -29,6 +29,25 @@ import org.eclipse.aether.artifact.Artifact;
* @since 3.9.2
*/
public interface PluginValidationManager {
enum IssueLocality {
/**
* Issue is "user actionable", is internal to the currently built project and is reparable from scope of it
* by doing some change (for example by changing POM and fixing the problematic plugin configuration).
*/
INTERNAL,
/**
* Issue (present in some plugin) is "developer actionable" (of given plugin, by changing code and doing
* new release), is NOT local to the currently built project. It may be reparable by updating given plugin
* to new fixed version, or by dropping plugin use from currently built project.
* <p>
* Note: if a reactor build contains a plugin (with issues) and later that built plugin is used in build,
* it will be reported as "external". It is up to developer to correctly interpret output (GAV) of issues
* and realize that in this case he wears two hats:" "user" and "(plugin) developer".
*/
EXTERNAL
}
/**
* Reports plugin issues applicable to the plugin as a whole.
* <p>
@ -36,14 +55,16 @@ public interface PluginValidationManager {
* does not exist yet. In turn, this method will not record extra information like plugin occurrence or declaration
* location as those are not yet available.
*/
void reportPluginValidationIssue(RepositorySystemSession session, Artifact pluginArtifact, String issue);
void reportPluginValidationIssue(
IssueLocality locality, RepositorySystemSession session, Artifact pluginArtifact, String issue);
/**
* Reports plugin issues applicable to the plugin as a whole.
* <p>
* This method will record extra information as well, like plugin occurrence or declaration location.
*/
void reportPluginValidationIssue(MavenSession mavenSession, MojoDescriptor mojoDescriptor, String issue);
void reportPluginValidationIssue(
IssueLocality locality, MavenSession mavenSession, MojoDescriptor mojoDescriptor, String issue);
/**
* Reports plugin Mojo issues applicable to the Mojo itself.
@ -51,5 +72,9 @@ public interface PluginValidationManager {
* This method will record extra information as well, like plugin occurrence or declaration location.
*/
void reportPluginMojoValidationIssue(
MavenSession mavenSession, MojoDescriptor mojoDescriptor, Class<?> mojoClass, String issue);
IssueLocality locality,
MavenSession mavenSession,
MojoDescriptor mojoDescriptor,
Class<?> mojoClass,
String issue);
}

View File

@ -565,6 +565,7 @@ public class DefaultMavenPluginManager implements MavenPluginManager {
if (mojo instanceof Contextualizable) {
pluginValidationManager.reportPluginMojoValidationIssue(
PluginValidationManager.IssueLocality.EXTERNAL,
session,
mojoDescriptor,
mojo.getClass(),

View File

@ -56,10 +56,9 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
private enum ValidationReportLevel {
NONE, // mute validation completely (validation issue collection still happens, it is just not reported!)
INLINE, // inline, each problem one line next to mojo invocation, repeated as many times as mojo is executed
BRIEF, // at end, one line with count of plugins in the build having validation issues
DEFAULT, // at end, list of plugin GAVs in the build having validation issues
VERBOSE // at end, detailed report of plugins in the build having validation issues
INLINE, // inline, each "internal" problem one line next to mojo invocation
SUMMARY, // at end, list of plugin GAVs along with "internal" issues
VERBOSE // at end, list of plugin GAVs along with detailed report of ANY validation issues
}
private final Logger logger = LoggerFactory.getLogger(getClass());
@ -77,7 +76,7 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
private ValidationReportLevel validationReportLevel(RepositorySystemSession session) {
String level = ConfigUtils.getString(session, null, MAVEN_PLUGIN_VALIDATION_KEY);
if (level == null || level.isEmpty()) {
return ValidationReportLevel.DEFAULT;
return ValidationReportLevel.INLINE;
}
try {
return ValidationReportLevel.valueOf(level.toUpperCase(Locale.ENGLISH));
@ -87,7 +86,7 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
MAVEN_PLUGIN_VALIDATION_KEY,
level,
Arrays.toString(ValidationReportLevel.values()));
return ValidationReportLevel.DEFAULT;
return ValidationReportLevel.INLINE;
}
}
@ -104,46 +103,51 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
return pluginKey(pluginArtifact.getGroupId(), pluginArtifact.getArtifactId(), pluginArtifact.getVersion());
}
@Override
public void reportPluginValidationIssue(RepositorySystemSession session, Artifact pluginArtifact, String issue) {
String pluginKey = pluginKey(pluginArtifact);
PluginValidationIssues pluginIssues =
pluginIssues(session).computeIfAbsent(pluginKey, k -> new PluginValidationIssues());
pluginIssues.reportPluginIssue(null, null, issue);
private void mayReportInline(RepositorySystemSession session, IssueLocality locality, String issue) {
ValidationReportLevel validationReportLevel = validationReportLevel(session);
if (validationReportLevel == ValidationReportLevel.INLINE) {
if (locality == IssueLocality.INTERNAL && validationReportLevel == ValidationReportLevel.INLINE) {
logger.warn(" {}", issue);
}
}
@Override
public void reportPluginValidationIssue(MavenSession mavenSession, MojoDescriptor mojoDescriptor, String issue) {
public void reportPluginValidationIssue(
IssueLocality locality, RepositorySystemSession session, Artifact pluginArtifact, String issue) {
String pluginKey = pluginKey(pluginArtifact);
PluginValidationIssues pluginIssues =
pluginIssues(session).computeIfAbsent(pluginKey, k -> new PluginValidationIssues());
pluginIssues.reportPluginIssue(locality, null, null, issue);
mayReportInline(session, locality, issue);
}
@Override
public void reportPluginValidationIssue(
IssueLocality locality, MavenSession mavenSession, MojoDescriptor mojoDescriptor, String issue) {
String pluginKey = pluginKey(mojoDescriptor);
PluginValidationIssues pluginIssues = pluginIssues(mavenSession.getRepositorySession())
.computeIfAbsent(pluginKey, k -> new PluginValidationIssues());
pluginIssues.reportPluginIssue(
pluginDeclaration(mavenSession, mojoDescriptor), pluginOccurrence(mavenSession), issue);
ValidationReportLevel validationReportLevel = validationReportLevel(mavenSession.getRepositorySession());
if (validationReportLevel == ValidationReportLevel.INLINE) {
logger.warn(" {}", issue);
}
locality, pluginDeclaration(mavenSession, mojoDescriptor), pluginOccurrence(mavenSession), issue);
mayReportInline(mavenSession.getRepositorySession(), locality, issue);
}
@Override
public void reportPluginMojoValidationIssue(
MavenSession mavenSession, MojoDescriptor mojoDescriptor, Class<?> mojoClass, String issue) {
IssueLocality locality,
MavenSession mavenSession,
MojoDescriptor mojoDescriptor,
Class<?> mojoClass,
String issue) {
String pluginKey = pluginKey(mojoDescriptor);
PluginValidationIssues pluginIssues = pluginIssues(mavenSession.getRepositorySession())
.computeIfAbsent(pluginKey, k -> new PluginValidationIssues());
pluginIssues.reportPluginMojoIssue(
locality,
pluginDeclaration(mavenSession, mojoDescriptor),
pluginOccurrence(mavenSession),
mojoInfo(mojoDescriptor, mojoClass),
issue);
ValidationReportLevel validationReportLevel = validationReportLevel(mavenSession.getRepositorySession());
if (validationReportLevel == ValidationReportLevel.INLINE) {
logger.warn(" {}", issue);
}
mayReportInline(mavenSession.getRepositorySession(), locality, issue);
}
private void reportSessionCollectedValidationIssues(MavenSession mavenSession) {
@ -158,46 +162,57 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
ConcurrentHashMap<String, PluginValidationIssues> issuesMap = pluginIssues(mavenSession.getRepositorySession());
if (!issuesMap.isEmpty()) {
logger.warn("");
logger.warn("Plugin validation issues were detected in {} plugin(s)", issuesMap.size());
logger.warn("");
if (validationReportLevel == ValidationReportLevel.BRIEF) {
return;
}
EnumSet<IssueLocality> issueLocalitiesToReport = validationReportLevel == ValidationReportLevel.VERBOSE
? EnumSet.allOf(IssueLocality.class)
: EnumSet.of(IssueLocality.INTERNAL);
logger.warn("");
logger.warn("Plugin {} validation issues were detected in following plugin(s)", issueLocalitiesToReport);
logger.warn("");
for (Map.Entry<String, PluginValidationIssues> entry : issuesMap.entrySet()) {
PluginValidationIssues issues = entry.getValue();
if (!hasAnythingToReport(issues, issueLocalitiesToReport)) {
continue;
}
logger.warn(" * {}", entry.getKey());
if (validationReportLevel == ValidationReportLevel.VERBOSE) {
PluginValidationIssues issues = entry.getValue();
if (!issues.pluginDeclarations.isEmpty()) {
logger.warn(" Declared at location(s):");
for (String pluginDeclaration : issues.pluginDeclarations) {
logger.warn(" * {}", pluginDeclaration);
}
if (!issues.pluginDeclarations.isEmpty()) {
logger.warn(" Declared at location(s):");
for (String pluginDeclaration : issues.pluginDeclarations) {
logger.warn(" * {}", pluginDeclaration);
}
if (!issues.pluginOccurrences.isEmpty()) {
logger.warn(" Used in module(s):");
for (String pluginOccurrence : issues.pluginOccurrences) {
logger.warn(" * {}", pluginOccurrence);
}
}
if (!issues.pluginOccurrences.isEmpty()) {
logger.warn(" Used in module(s):");
for (String pluginOccurrence : issues.pluginOccurrences) {
logger.warn(" * {}", pluginOccurrence);
}
if (!issues.pluginIssues.isEmpty()) {
logger.warn(" Plugin issue(s):");
for (String pluginIssue : issues.pluginIssues) {
logger.warn(" * {}", pluginIssue);
}
}
if (!issues.mojoIssues.isEmpty()) {
logger.warn(" Mojo issue(s):");
for (String mojoInfo : issues.mojoIssues.keySet()) {
logger.warn(" * Mojo {}", mojoInfo);
for (String mojoIssue : issues.mojoIssues.get(mojoInfo)) {
logger.warn(" - {}", mojoIssue);
}
if (!issues.pluginIssues.isEmpty()) {
for (IssueLocality issueLocality : issueLocalitiesToReport) {
Set<String> pluginIssues = issues.pluginIssues.get(issueLocality);
if (pluginIssues != null && !pluginIssues.isEmpty()) {
logger.warn(" Plugin {} issue(s):", issueLocality);
for (String pluginIssue : pluginIssues) {
logger.warn(" * {}", pluginIssue);
}
}
}
logger.warn("");
}
if (!issues.mojoIssues.isEmpty()) {
for (IssueLocality issueLocality : issueLocalitiesToReport) {
Map<String, LinkedHashSet<String>> mojoIssues = issues.mojoIssues.get(issueLocality);
if (mojoIssues != null && !mojoIssues.isEmpty()) {
logger.warn(" Mojo {} issue(s):", issueLocality);
for (String mojoInfo : mojoIssues.keySet()) {
logger.warn(" * Mojo {}", mojoInfo);
for (String mojoIssue : mojoIssues.get(mojoInfo)) {
logger.warn(" - {}", mojoIssue);
}
}
}
}
}
logger.warn("");
}
logger.warn("");
if (validationReportLevel == ValidationReportLevel.VERBOSE) {
@ -211,6 +226,20 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
}
}
private boolean hasAnythingToReport(PluginValidationIssues issues, EnumSet<IssueLocality> issueLocalitiesToReport) {
for (IssueLocality issueLocality : issueLocalitiesToReport) {
Set<String> pluginIssues = issues.pluginIssues.get(issueLocality);
if (pluginIssues != null && !pluginIssues.isEmpty()) {
return true;
}
Map<String, LinkedHashSet<String>> mojoIssues = issues.mojoIssues.get(issueLocality);
if (mojoIssues != null && !mojoIssues.isEmpty()) {
return true;
}
}
return false;
}
private String pluginDeclaration(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
InputLocation inputLocation =
mojoDescriptor.getPluginDescriptor().getPlugin().getLocation("");
@ -267,36 +296,46 @@ public final class DefaultPluginValidationManager extends AbstractEventSpy imple
private final LinkedHashSet<String> pluginOccurrences;
private final LinkedHashSet<String> pluginIssues;
private final HashMap<IssueLocality, LinkedHashSet<String>> pluginIssues;
private final LinkedHashMap<String, LinkedHashSet<String>> mojoIssues;
private final HashMap<IssueLocality, LinkedHashMap<String, LinkedHashSet<String>>> mojoIssues;
private PluginValidationIssues() {
this.pluginDeclarations = new LinkedHashSet<>();
this.pluginOccurrences = new LinkedHashSet<>();
this.pluginIssues = new LinkedHashSet<>();
this.mojoIssues = new LinkedHashMap<>();
this.pluginIssues = new HashMap<>();
this.mojoIssues = new HashMap<>();
}
private synchronized void reportPluginIssue(String pluginDeclaration, String pluginOccurrence, String issue) {
private synchronized void reportPluginIssue(
IssueLocality issueLocality, String pluginDeclaration, String pluginOccurrence, String issue) {
if (pluginDeclaration != null) {
pluginDeclarations.add(pluginDeclaration);
}
if (pluginOccurrence != null) {
pluginOccurrences.add(pluginOccurrence);
}
pluginIssues.add(issue);
pluginIssues
.computeIfAbsent(issueLocality, k -> new LinkedHashSet<>())
.add(issue);
}
private synchronized void reportPluginMojoIssue(
String pluginDeclaration, String pluginOccurrence, String mojoInfo, String issue) {
IssueLocality issueLocality,
String pluginDeclaration,
String pluginOccurrence,
String mojoInfo,
String issue) {
if (pluginDeclaration != null) {
pluginDeclarations.add(pluginDeclaration);
}
if (pluginOccurrence != null) {
pluginOccurrences.add(pluginOccurrence);
}
mojoIssues.computeIfAbsent(mojoInfo, k -> new LinkedHashSet<>()).add(issue);
mojoIssues
.computeIfAbsent(issueLocality, k -> new LinkedHashMap<>())
.computeIfAbsent(mojoInfo, k -> new LinkedHashSet<>())
.add(issue);
}
}
}

View File

@ -78,7 +78,7 @@ class DeprecatedCoreExpressionValidator extends AbstractMavenPluginParametersVal
.filter(this::isDeprecated)
.map(this::formatParameter)
.forEach(m -> pluginValidationManager.reportPluginMojoValidationIssue(
mavenSession, mojoDescriptor, mojoClass, m));
PluginValidationManager.IssueLocality.EXTERNAL, mavenSession, mojoDescriptor, mojoClass, m));
}
private boolean isDeprecated(Parameter parameter) {

View File

@ -58,7 +58,11 @@ class DeprecatedPluginValidator extends AbstractMavenPluginDescriptorSourcedPara
ExpressionEvaluator expressionEvaluator) {
if (mojoDescriptor.getDeprecated() != null) {
pluginValidationManager.reportPluginMojoValidationIssue(
mavenSession, mojoDescriptor, mojoClass, logDeprecatedMojo(mojoDescriptor));
PluginValidationManager.IssueLocality.INTERNAL,
mavenSession,
mojoDescriptor,
mojoClass,
logDeprecatedMojo(mojoDescriptor));
}
if (mojoDescriptor.getParameters() != null) {
@ -81,7 +85,11 @@ class DeprecatedPluginValidator extends AbstractMavenPluginDescriptorSourcedPara
if (isValueSet(config, expressionEvaluator)) {
pluginValidationManager.reportPluginMojoValidationIssue(
mavenSession, mojoDescriptor, mojoClass, formatParameter(parameter));
PluginValidationManager.IssueLocality.INTERNAL,
mavenSession,
mojoDescriptor,
mojoClass,
formatParameter(parameter));
}
}

View File

@ -61,7 +61,10 @@ class Maven2DependenciesValidator extends AbstractMavenPluginDependenciesValidat
if (!maven2Versions.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
session, pluginArtifact, "Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
PluginValidationManager.IssueLocality.EXTERNAL,
session,
pluginArtifact,
"Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
}
}
}

View File

@ -52,6 +52,7 @@ class Maven3CompatDependenciesValidator extends AbstractMavenPluginDependenciesV
&& "maven-compat".equals(dependency.getArtifact().getArtifactId())
&& !JavaScopes.TEST.equals(dependency.getScope())) {
pluginValidationManager.reportPluginValidationIssue(
PluginValidationManager.IssueLocality.EXTERNAL,
session,
pluginArtifact,
"Plugin depends on the deprecated Maven 2.x compatibility layer, which will be not supported in Maven 4.x");

View File

@ -60,7 +60,10 @@ class MavenMixedDependenciesValidator extends AbstractMavenPluginDependenciesVal
if (mavenVersions.size() > 1) {
pluginValidationManager.reportPluginValidationIssue(
session, pluginArtifact, "Plugin mixes multiple Maven versions: " + mavenVersions);
PluginValidationManager.IssueLocality.EXTERNAL,
session,
pluginArtifact,
"Plugin mixes multiple Maven versions: " + mavenVersions);
}
}
}

View File

@ -62,6 +62,7 @@ class MavenScopeDependenciesValidator extends AbstractMavenPluginDependenciesVal
if (!mavenArtifacts.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
PluginValidationManager.IssueLocality.EXTERNAL,
session,
pluginArtifact,
"Plugin should declare Maven artifacts in `provided` scope. If the plugin already declares them in `provided` scope, update the maven-plugin-plugin to latest version. Artifacts found with wrong scope: "

View File

@ -51,7 +51,10 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
PluginValidationManager.IssueLocality.EXTERNAL,
session,
pluginArtifact,
"Plugin depends on plexus-container-default, which is EOL");
}
}
}

View File

@ -76,7 +76,11 @@ class ReadOnlyPluginParametersValidator extends AbstractMavenPluginDescriptorSou
if (isValueSet(config, expressionEvaluator)) {
pluginValidationManager.reportPluginMojoValidationIssue(
mavenSession, mojoDescriptor, mojoClass, formatParameter(parameter));
PluginValidationManager.IssueLocality.INTERNAL,
mavenSession,
mojoDescriptor,
mojoClass,
formatParameter(parameter));
}
}
}