You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2024/07/07 21:04:45 UTC
(maven-pmd-plugin) branch MPMD-391 created (now a5edaa2)
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a change to branch MPMD-391
in repository https://gitbox.apache.org/repos/asf/maven-pmd-plugin.git
at a5edaa2 [MPMD-391] Log what developers care about and not what they don't
This branch includes the following new commits:
new a5edaa2 [MPMD-391] Log what developers care about and not what they don't
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(maven-pmd-plugin) 01/01: [MPMD-391] Log what developers care about and not what they don't
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch MPMD-391
in repository https://gitbox.apache.org/repos/asf/maven-pmd-plugin.git
commit a5edaa2fdf7575ec1be676e32e8cc68095d7204f
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sun Jul 7 23:04:28 2024 +0200
[MPMD-391] Log what developers care about and not what they don't
---
src/it/MPMD-163/verify.groovy | 4 +--
src/it/MPMD-205-pmd-js-check/verify.groovy | 2 +-
.../invoker.properties | 1 -
.../MPMD-243-excludeFromFailureFile/verify.groovy | 4 +--
src/it/MPMD-270-325-JDK11/verify.groovy | 3 +-
src/it/MPMD-290-cpd-for-csharp/invoker.properties | 2 --
src/it/MPMD-290-cpd-for-csharp/verify.groovy | 4 +--
src/it/mpmd-138/verify.groovy | 17 +++++----
.../plugins/pmd/AbstractPmdViolationCheckMojo.java | 41 ++++++++++++++--------
.../org/apache/maven/plugins/pmd/CpdReport.java | 3 +-
.../maven/plugins/pmd/CpdViolationCheckMojo.java | 7 ++--
.../maven/plugins/pmd/PmdViolationCheckMojo.java | 5 ++-
.../apache/maven/plugins/pmd/CpdReportTest.java | 3 --
.../plugins/pmd/CpdViolationCheckMojoTest.java | 7 ++--
.../plugins/pmd/PmdViolationCheckMojoTest.java | 27 +++++---------
15 files changed, 60 insertions(+), 70 deletions(-)
diff --git a/src/it/MPMD-163/verify.groovy b/src/it/MPMD-163/verify.groovy
index 4d70d26..0d03952 100644
--- a/src/it/MPMD-163/verify.groovy
+++ b/src/it/MPMD-163/verify.groovy
@@ -21,5 +21,5 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert 1 == buildLog.getText().count('You have 1 warning')
-assert 1 == buildLog.getText().count("PMD Warning: Foo:23 Rule:UnusedFormalParameter Priority:3 Avoid unused constructor parameters such as 'foo'..")
\ No newline at end of file
+assert buildLog.getText().contains('[WARNING] PMD 7.0.0 has issued 1 warning. For more details see:')
+assert buildLog.getText().contains("[WARNING] PMD Warning: Foo:23 Rule:UnusedFormalParameter Priority:3 Avoid unused constructor parameters such as 'foo'..")
diff --git a/src/it/MPMD-205-pmd-js-check/verify.groovy b/src/it/MPMD-205-pmd-js-check/verify.groovy
index b146fbd..f5fba04 100644
--- a/src/it/MPMD-205-pmd-js-check/verify.groovy
+++ b/src/it/MPMD-205-pmd-js-check/verify.groovy
@@ -20,4 +20,4 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.getText().contains("[INFO] PMD Failure: PmdJsCheck.js:23 Rule:GlobalVariable Priority:1 Avoid using global variables.");
+assert buildLog.getText().contains("[WARNING] PMD Failure: PmdJsCheck.js:23 Rule:GlobalVariable Priority:1 Avoid using global variables.");
diff --git a/src/it/MPMD-243-excludeFromFailureFile/invoker.properties b/src/it/MPMD-243-excludeFromFailureFile/invoker.properties
index 1e595ef..4b2a684 100644
--- a/src/it/MPMD-243-excludeFromFailureFile/invoker.properties
+++ b/src/it/MPMD-243-excludeFromFailureFile/invoker.properties
@@ -16,4 +16,3 @@
# under the License.
invoker.goals = clean pmd:check
-invoker.maven.version = 3+
diff --git a/src/it/MPMD-243-excludeFromFailureFile/verify.groovy b/src/it/MPMD-243-excludeFromFailureFile/verify.groovy
index 418f435..cdf38bd 100644
--- a/src/it/MPMD-243-excludeFromFailureFile/verify.groovy
+++ b/src/it/MPMD-243-excludeFromFailureFile/verify.groovy
@@ -19,5 +19,5 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.text.contains( "PMD Warning: com.example.ClassWithLotsOfStaticImports" )
-assert buildLog.text.contains( "You have 1 warning. For more details see" )
+assert buildLog.text.contains( "[WARNING] PMD Warning: com.example.ClassWithLotsOfStaticImports" )
+assert buildLog.text.contains( "[WARNING] PMD 7.0.0 has issued 1 warning. For more details see:" )
diff --git a/src/it/MPMD-270-325-JDK11/verify.groovy b/src/it/MPMD-270-325-JDK11/verify.groovy
index 2846ad3..ab39fcd 100644
--- a/src/it/MPMD-270-325-JDK11/verify.groovy
+++ b/src/it/MPMD-270-325-JDK11/verify.groovy
@@ -20,5 +20,4 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.text.contains( '[INFO] PMD Failure: com.mycompany.app.Foo:26 Rule:UnusedFormalParameter Priority:3' )
-assert !buildLog.text.contains( '[WARNING] PMD' )
+assert buildLog.text.contains( '[WARNING] PMD Failure: com.mycompany.app.Foo:26 Rule:UnusedFormalParameter Priority:3' )
diff --git a/src/it/MPMD-290-cpd-for-csharp/invoker.properties b/src/it/MPMD-290-cpd-for-csharp/invoker.properties
index 8e84538..472318a 100644
--- a/src/it/MPMD-290-cpd-for-csharp/invoker.properties
+++ b/src/it/MPMD-290-cpd-for-csharp/invoker.properties
@@ -15,7 +15,5 @@
# specific language governing permissions and limitations
# under the License.
-invoker.maven.version = 3.3+
invoker.goals = clean verify
invoker.buildResult = failure
-invoker.debug = true
diff --git a/src/it/MPMD-290-cpd-for-csharp/verify.groovy b/src/it/MPMD-290-cpd-for-csharp/verify.groovy
index 18b724a..b56192e 100644
--- a/src/it/MPMD-290-cpd-for-csharp/verify.groovy
+++ b/src/it/MPMD-290-cpd-for-csharp/verify.groovy
@@ -21,8 +21,8 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.text.contains( "[INFO] CPD Failure: Found 7 lines of duplicated code at locations" )
-assert buildLog.text.contains( "[DEBUG] PMD failureCount: 1, warningCount: 0" )
+assert buildLog.text.contains( "[WARNING] CPD Failure: Found 7 lines of duplicated code at locations" )
+assert buildLog.text.contains( "CPD 7.0.0 has found 1 duplication. For more details see:" )
File cpdXml = new File( basedir, 'target/cpd.xml' )
assert cpdXml.exists()
diff --git a/src/it/mpmd-138/verify.groovy b/src/it/mpmd-138/verify.groovy
index b9dc0f7..f7a42bd 100644
--- a/src/it/mpmd-138/verify.groovy
+++ b/src/it/mpmd-138/verify.groovy
@@ -22,16 +22,19 @@ File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
// Module 1
-assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:27 Rule:UnnecessarySemicolon Priority:3 Unnecessary semicolon')
-assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:28 Rule:UnnecessaryReturn Priority:3 Unnecessary return statement')
-assert 1 == buildLog.getText().count('[INFO] You have 2 PMD violations. For more details see:')
+assert buildLog.getText().contains('[WARNING] PMD Failure: test.MyClass:27 Rule:UnnecessarySemicolon Priority:3 Unnecessary semicolon')
+assert buildLog.getText().contains('[WARNING] PMD Failure: test.MyClass:28 Rule:UnnecessaryReturn Priority:3 Unnecessary return statement')
+assert buildLog.getText().contains('[WARNING] PMD 7.0.0 has found 2 violations. For more details see:')
// Module 2
-assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:24 Rule:UnusedPrivateField Priority:3 Avoid unused private fields such as \'x\'')
-assert 1 == buildLog.getText().count('[INFO] You have 1 PMD violation. For more details see:')
+assert buildLog.getText().contains('[WARNING] PMD Failure: test.MyClass:24 Rule:UnusedPrivateField Priority:3 Avoid unused private fields such as \'x\'')
+assert buildLog.getText().contains('[WARNING] PMD 7.0.0 has found 1 violation. For more details see:')
// Module 3
-assert 1 == buildLog.getText().count('[INFO] You have 1 CPD duplication. For more details see:')
+assert buildLog.getText().contains('[WARNING] CPD Failure: Found 37 lines of duplicated code at locations:')
+assert buildLog.getText().contains('[WARNING] CPD 7.0.0 has found 1 duplication. For more details see:')
// Module 4
-assert 1 == buildLog.getText().count('[INFO] You have 2 CPD duplications. For more details see:')
+assert buildLog.getText().contains('[WARNING] CPD Failure: Found 35 lines of duplicated code at locations:')
+assert buildLog.getText().contains('[WARNING] CPD Failure: Found 34 lines of duplicated code at locations:'=
+assert buildLog.getText().contains('[WARNING] CPD 7.0.0 has found 2 duplications. For more details see:')
diff --git a/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdViolationCheckMojo.java b/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdViolationCheckMojo.java
index 48cda50..e6f5fb1 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdViolationCheckMojo.java
@@ -120,7 +120,7 @@ public abstract class AbstractPmdViolationCheckMojo<D> extends AbstractMojo {
protected MavenProject project;
protected void executeCheck(
- final String filename, final String tagName, final String key, final int failurePriority)
+ final String filename, final String analyzerName, final String failureName, final int failurePriority)
throws MojoFailureException, MojoExecutionException {
if (aggregate && !project.isExecutionRoot()) {
return;
@@ -134,8 +134,6 @@ public abstract class AbstractPmdViolationCheckMojo<D> extends AbstractMojo {
final File outputFile = new File(targetDirectory, filename);
if (outputFile.exists()) {
- getLog().info("PMD version: " + AbstractPmdReport.getPmdVersion());
-
try {
final ViolationDetails<D> violations = getViolations(outputFile, failurePriority);
@@ -149,23 +147,24 @@ public abstract class AbstractPmdViolationCheckMojo<D> extends AbstractMojo {
final int failureCount = failures.size();
final int warningCount = warnings.size();
- final String message = getMessage(failureCount, warningCount, key, outputFile);
-
- getLog().debug("PMD failureCount: " + failureCount + ", warningCount: " + warningCount);
+ final String message = getMessage(failureCount, warningCount, analyzerName, failureName, outputFile);
if (failureCount > getMaxAllowedViolations() && isFailOnViolation()) {
throw new MojoFailureException(message);
}
- this.getLog().info(message);
+ if (!message.isEmpty()) {
+ this.getLog().warn(message);
+ }
if (failureCount > 0 && isFailOnViolation() && failureCount <= getMaxAllowedViolations()) {
this.getLog()
- .info("The build is not failed, since " + getMaxAllowedViolations()
+ .info("The build has not failed because " + getMaxAllowedViolations()
+ " violations are allowed (maxAllowedViolations).");
}
} catch (final IOException | XmlPullParserException e) {
- throw new MojoExecutionException("Unable to read PMD results xml: " + outputFile.getAbsolutePath(), e);
+ throw new MojoExecutionException(
+ "Unable to read " + analyzerName + " results XML: " + outputFile.getAbsolutePath(), e);
}
} else {
throw new MojoFailureException("Unable to perform check, " + "unable to find " + outputFile);
@@ -231,26 +230,38 @@ public abstract class AbstractPmdViolationCheckMojo<D> extends AbstractMojo {
*
* @param failureCount
* @param warningCount
- * @param key
+ * @param analyzerName
+ * @param failureName
* @param outputFile
* @return
*/
- private String getMessage(final int failureCount, final int warningCount, final String key, final File outputFile) {
+ private String getMessage(
+ final int failureCount,
+ final int warningCount,
+ final String analyzerName,
+ final String failureName,
+ final File outputFile) {
final StringBuilder message = new StringBuilder(256);
if (failureCount > 0 || warningCount > 0) {
if (failureCount > 0) {
- message.append("You have ")
+ message.append(analyzerName)
+ .append(" ")
+ .append(AbstractPmdReport.getPmdVersion())
+ .append(" has found ")
.append(failureCount)
.append(" ")
- .append(key)
+ .append(failureName)
.append(failureCount > 1 ? "s" : "");
}
if (warningCount > 0) {
if (failureCount > 0) {
- message.append(" and ");
+ message.append(" and issued ");
} else {
- message.append("You have ");
+ message.append(analyzerName)
+ .append(" ")
+ .append(AbstractPmdReport.getPmdVersion())
+ .append(" has issued ");
}
message.append(warningCount).append(" warning").append(warningCount > 1 ? "s" : "");
}
diff --git a/src/main/java/org/apache/maven/plugins/pmd/CpdReport.java b/src/main/java/org/apache/maven/plugins/pmd/CpdReport.java
index 8c00364..30158ec 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/CpdReport.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/CpdReport.java
@@ -195,12 +195,11 @@ public class CpdReport extends AbstractPmdReport {
Toolchain tc = getToolchain();
if (tc != null) {
- getLog().info("Toolchain in maven-pmd-plugin: " + tc);
+ getLog().debug("Toolchain in maven-pmd-plugin: " + tc);
String javaExecutable = tc.findTool("java"); // NOI18N
request.setJavaExecutable(javaExecutable);
}
- getLog().info("PMD version: " + AbstractPmdReport.getPmdVersion());
cpdResult = CpdExecutor.execute(request);
} catch (UnsupportedEncodingException e) {
throw new MavenReportException("Encoding '" + getInputEncoding() + "' is not supported.", e);
diff --git a/src/main/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojo.java b/src/main/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojo.java
index 7852f76..6d9e204 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojo.java
@@ -39,7 +39,6 @@ import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
/**
* Fails the build if there were any CPD violations in the source code.
*
- * @version $Id$
* @since 2.0
*/
@Mojo(name = "cpd-check", defaultPhase = LifecyclePhase.VERIFY, threadSafe = true)
@@ -75,7 +74,7 @@ public class CpdViolationCheckMojo extends AbstractPmdViolationCheckMojo<Duplica
return;
}
- executeCheck("cpd.xml", "duplication", "CPD duplication", 10);
+ executeCheck("cpd.xml", "CPD", "duplication", 10);
}
/**
@@ -88,14 +87,14 @@ public class CpdViolationCheckMojo extends AbstractPmdViolationCheckMojo<Duplica
StringBuilder buff = new StringBuilder(100);
buff.append("CPD ").append(severity).append(": Found ");
buff.append(lines).append(" lines of duplicated code at locations:");
- this.getLog().info(buff.toString());
+ this.getLog().warn(buff.toString());
for (CpdFile file : item.getFiles()) {
buff.setLength(0);
buff.append(" ");
buff.append(file.getPath());
buff.append(" line ").append(file.getLine());
- this.getLog().info(buff.toString());
+ this.getLog().warn(buff.toString());
}
this.getLog().debug("CPD " + severity + ": Code Fragment ");
diff --git a/src/main/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojo.java b/src/main/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojo.java
index 2fcb1b3..4d33f5b 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojo.java
@@ -40,7 +40,6 @@ import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
/**
* Fails the build if there were any PMD violations in the source code.
*
- * @version $Id$
* @since 2.0
*/
@Mojo(name = "check", defaultPhase = LifecyclePhase.VERIFY, threadSafe = true)
@@ -83,7 +82,7 @@ public class PmdViolationCheckMojo extends AbstractPmdViolationCheckMojo<Violati
return;
}
- executeCheck("pmd.xml", "violation", "PMD violation", failurePriority);
+ executeCheck("pmd.xml", "PMD", "violation", failurePriority);
}
/**
@@ -108,7 +107,7 @@ public class PmdViolationCheckMojo extends AbstractPmdViolationCheckMojo<Violati
buff.append(" Priority:").append(item.getPriority());
buff.append(" ").append(item.getText()).append(".");
- this.getLog().info(buff.toString());
+ this.getLog().warn(buff.toString());
}
@Override
diff --git a/src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java b/src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java
index 7b47026..86a2b8a 100644
--- a/src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java
+++ b/src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java
@@ -64,9 +64,6 @@ public class CpdReportTest extends AbstractPmdReportTestCase {
assertTrue(lowerCaseContains(str, "App.java"));
assertTrue(lowerCaseContains(str, "public String dup( String str )"));
assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, i + 1);"));
-
- // the version should be logged
- assertLogOutputContains("PMD version: " + AbstractPmdReport.getPmdVersion());
}
/**
diff --git a/src/test/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojoTest.java b/src/test/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojoTest.java
index 5d7db5b..1e28e5c 100644
--- a/src/test/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojoTest.java
+++ b/src/test/java/org/apache/maven/plugins/pmd/CpdViolationCheckMojoTest.java
@@ -41,11 +41,8 @@ public class CpdViolationCheckMojoTest extends AbstractPmdReportTestCase {
fail("MojoFailureException should be thrown.");
} catch (final Exception e) {
- // the version should be logged
- String output = CapturingPrintStream.getOutput();
- assertTrue(output.contains("PMD version: " + AbstractPmdReport.getPmdVersion()));
-
- assertTrue(e.getMessage().startsWith("You have 1 CPD duplication."));
+ assertTrue(e.getMessage()
+ .startsWith("CPD " + AbstractPmdReport.getPmdVersion() + " has found 1 duplication."));
}
}
diff --git a/src/test/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojoTest.java b/src/test/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojoTest.java
index b47508b..dff209a 100644
--- a/src/test/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojoTest.java
+++ b/src/test/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojoTest.java
@@ -43,11 +43,8 @@ public class PmdViolationCheckMojoTest extends AbstractPmdReportTestCase {
fail("MojoFailureException should be thrown.");
} catch (final Exception e) {
- // the version should be logged
- String output = CapturingPrintStream.getOutput();
- assertTrue(output.contains("PMD version: " + AbstractPmdReport.getPmdVersion()));
-
- assertTrue(e.getMessage().startsWith("You have 8 PMD violations."));
+ assertTrue(
+ e.getMessage().startsWith("PMD " + AbstractPmdReport.getPmdVersion() + " has found 8 violations."));
}
}
@@ -81,13 +78,9 @@ public class PmdViolationCheckMojoTest extends AbstractPmdReportTestCase {
pmdViolationMojoFail.execute();
fail("Exception Expected");
} catch (final MojoFailureException e) {
- String message = e.getMessage();
- if (message.contains("You have 5 PMD violations and 3 warnings.")) {
- System.out.println("Caught expected message: " + e.getMessage()); // expected
- } else {
- throw new AssertionError(
- "Expected: '" + message + "' to contain 'You have 5 PMD violations and 3 warnings.'");
- }
+ assertTrue(e.getMessage()
+ .startsWith("PMD " + AbstractPmdReport.getPmdVersion()
+ + " has found 5 violations and issued 3 warnings."));
}
}
@@ -108,13 +101,9 @@ public class PmdViolationCheckMojoTest extends AbstractPmdReportTestCase {
pmdViolationMojo.execute();
fail("Exception Expected");
} catch (final MojoFailureException e) {
- String message = e.getMessage();
- if (message.contains("You have 5 PMD violations and 3 warnings.")) {
- System.out.println("Caught expected message: " + e.getMessage()); // expected
- } else {
- throw new AssertionError(
- "Expected: '" + message + "' to contain 'You have 5 PMD violations and 3 warnings.'");
- }
+ assertTrue(e.getMessage()
+ .startsWith("PMD " + AbstractPmdReport.getPmdVersion()
+ + " has found 5 violations and issued 3 warnings."));
}
}