adangel commented on code in PR #144:
URL: https://github.com/apache/maven-pmd-plugin/pull/144#discussion_r1553758485
##########
src/site/apt/examples/upgrading-PMD-at-runtime.apt.vm:
##########
@@ -88,6 +88,8 @@ Upgrading PMD at Runtime
*--------------------------------------------------------------------------------*--------------------------------------------------*
| <<maven-pmd-plugin>>
| <<PMD>> |
*--------------------------------------------------------------------------------*--------------------------------------------------*
+|
{{{https://maven.apache.org/plugins-archives/maven-pmd-plugin-3.22.0/}3.22.0}}
| {{{https://pmd.github.io/pmd-7.0.0/}7.0.0}} |
Review Comment:
```suggestion
|
{{{https://maven.apache.org/plugins-archives/maven-pmd-plugin-3.22.0/}3.22.0}}
| {{{https://docs.pmd-code.org/pmd-doc-7.0.0/}7.0.0}} |
```
##########
src/site/apt/index.apt.vm:
##########
@@ -89,6 +89,11 @@ ${project.name}
* Upgrading Notes
+ <<Note:>> Starting with Maven PMD Plugin 3.22.0, the plugin requires PDM
version 7.0.0 or higher.
Review Comment:
```suggestion
<<Note:>> Starting with Maven PMD Plugin 3.22.0, the plugin requires PMD
version 7.0.0 or higher.
```
##########
src/main/java/org/apache/maven/plugins/pmd/exec/PmdExecutor.java:
##########
@@ -190,7 +189,7 @@ private PmdResult run() throws MavenReportException {
configuration.setRuleSets(request.getRulesets());
configuration.setMinimumPriority(RulePriority.valueOf(request.getMinimumPriority()));
if (request.getBenchmarkOutputLocation() != null) {
- configuration.setBenchmark(true);
+ TimeTracker.startGlobalTracking();
Review Comment:
Alright, we have even ITs
(https://github.com/apache/maven-pmd-plugin/tree/master/src/it/MPMD-181-benchmark).
So, this should be fine.
##########
src/site/apt/index.apt.vm:
##########
@@ -89,6 +89,11 @@ ${project.name}
* Upgrading Notes
+ <<Note:>> Starting with Maven PMD Plugin 3.22.0, the plugin requires PDM
version 7.0.0 or higher.
+ The PMD 7.0.0 switched to the SLF4J and since Maven 3.1.0+ the SLF4J is the
default logging API,
+ because of that the <<<showPmdLog>>> makes no sense to exist. See
+ {{{}https://maven.apache.org/ref/3.9.6/maven-embedder/logging.html}}Maven
Logging for more details.
Review Comment:
We also should mention, that the switch from PMD 6.x to PMD 7 is a major
version change... That's the biggest impact on end-users, who innocently update
m-pmd-p version from 3.21.2 to 3.22.0...
Maybe something along these lines?
```
* The upgrade from PMD 6 to PMD 7.0.0 is a major version change. If you
use the default ruleset
from Maven PMD Plugin, then everything should just work. But if you
use a custom ruleset, you
most likely need to review your ruleset and migrate it to PMD 7. Rules
might have been renamed or
replaced. See
{{{https://docs.pmd-code.org/latest/pmd_release_notes_pmd7.html}Detailed
Release Notes for PMD 7}}
and
{{{https://docs.pmd-code.org/latest/pmd_userdocs_migrating_to_pmd7.html}Migration
Guide for PMD 7}}.
```
##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java:
##########
@@ -155,46 +152,53 @@ private CpdResult run() throws MavenReportException {
CPDConfiguration cpdConfiguration = new CPDConfiguration();
cpdConfiguration.setMinimumTileSize(request.getMinimumTokens());
+ cpdConfiguration.setIgnoreAnnotations(request.isIgnoreAnnotations());
+ cpdConfiguration.setIgnoreLiterals(request.isIgnoreLiterals());
+ cpdConfiguration.setIgnoreIdentifiers(request.isIgnoreIdentifiers());
Language cpdLanguage;
if ("java".equals(request.getLanguage()) || null ==
request.getLanguage()) {
- cpdLanguage = new JavaLanguage(request.getLanguageProperties());
+ cpdLanguage = new JavaLanguageModule();
} else if ("javascript".equals(request.getLanguage())) {
- cpdLanguage = new EcmascriptLanguage();
+ cpdLanguage = new EcmascriptLanguageModule();
} else if ("jsp".equals(request.getLanguage())) {
- cpdLanguage = new JSPLanguage();
+ cpdLanguage = new JspLanguageModule();
} else {
- cpdLanguage =
LanguageFactory.createLanguage(request.getLanguage(),
request.getLanguageProperties());
+ cpdLanguage =
cpdConfiguration.getLanguageRegistry().getLanguageById(request.getLanguage());
}
- cpdConfiguration.setLanguage(cpdLanguage);
- cpdConfiguration.setSourceEncoding(request.getSourceEncoding());
+ cpdConfiguration.setOnlyRecognizeLanguage(cpdLanguage);
+
cpdConfiguration.setSourceEncoding(Charset.forName(request.getSourceEncoding()));
- CPD cpd = new CPD(cpdConfiguration);
- try {
- cpd.add(request.getFiles());
- } catch (IOException e) {
- throw new MavenReportException(e.getMessage(), e);
- }
+ request.getFiles().forEach(f ->
cpdConfiguration.addInputPath(f.toPath()));
LOG.debug("Executing CPD...");
- cpd.go();
- LOG.debug("CPD finished.");
// always create XML format. we need to output it even if the file
list is empty or we have no duplications
// so the "check" goals can check for violations
- writeXmlReport(cpd);
+ try (CpdAnalysis cpd = CpdAnalysis.create(cpdConfiguration)) {
+ cpd.performAnalysis(report -> {
+ try {
+ writeXmlReport(report);
- // html format is handled by maven site report, xml format has already
been rendered
- String format = request.getFormat();
- if (!"html".equals(format) && !"xml".equals(format)) {
- writeFormattedReport(cpd);
+ // html format is handled by maven site report, xml format
has already been rendered
+ String format = request.getFormat();
+ if (!"html".equals(format) && !"xml".equals(format)) {
+ writeFormattedReport(report);
+ }
+ } catch (MavenReportException e) {
+ LOG.error(e.getMessage(), e);
+ }
+ });
+ } catch (IOException e) {
+ LOG.error(e.getMessage(), e);
Review Comment:
```suggestion
LOG.error("Error while executing CPD: {}", e.getMessage(), e);
```
##########
src/main/java/org/apache/maven/plugins/pmd/CpdReport.java:
##########
@@ -238,7 +228,7 @@ public String getOutputName() {
* @deprecated Use {@link CpdExecutor#createRenderer(String, String)}
instead.
*/
@Deprecated
- public CPDRenderer createRenderer() throws MavenReportException {
+ public CPDReportRenderer createRenderer() throws MavenReportException {
Review Comment:
Changing the return type makes this binary incompatible. Given that this
method is anyway deprecated already, maybe we can delete it directly?
It is not used by the plugin itself, so only by some other consumers, if
any...
##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java:
##########
@@ -155,46 +152,53 @@ private CpdResult run() throws MavenReportException {
CPDConfiguration cpdConfiguration = new CPDConfiguration();
cpdConfiguration.setMinimumTileSize(request.getMinimumTokens());
+ cpdConfiguration.setIgnoreAnnotations(request.isIgnoreAnnotations());
+ cpdConfiguration.setIgnoreLiterals(request.isIgnoreLiterals());
+ cpdConfiguration.setIgnoreIdentifiers(request.isIgnoreIdentifiers());
Language cpdLanguage;
if ("java".equals(request.getLanguage()) || null ==
request.getLanguage()) {
Review Comment:
This section can actually be a bit simplified:
```java
String languageId = request.getLanguage();
if ("javascript".equals(languageId)) {
languageId = "ecmascript";
} else if (languageId == null) {
languageId = "java"; // default
}
Language cpdLanguage =
cpdConfiguration.getLanguageRegistry().getLanguageById(languageId);
```
##########
src/site/apt/index.apt.vm:
##########
@@ -89,6 +89,11 @@ ${project.name}
* Upgrading Notes
+ <<Note:>> Starting with Maven PMD Plugin 3.22.0, the plugin requires PDM
version 7.0.0 or higher.
+ The PMD 7.0.0 switched to the SLF4J and since Maven 3.1.0+ the SLF4J is the
default logging API,
+ because of that the <<<showPmdLog>>> makes no sense to exist. See
+ {{{}https://maven.apache.org/ref/3.9.6/maven-embedder/logging.html}}Maven
Logging for more details.
Review Comment:
```suggestion
PMD 7.0.0 switched to SLF4J and since Maven 3.1.0+ SLF4J is the default
logging API. Logs from PMD are now always shown and cannot be disabled at
runtime after maven has started. The property <<<showPmdLog>>> makes no sense
anymore and is deprecated now. See
{{{https://maven.apache.org/maven-logging.html}Maven Logging}} for how to
configure logging. For disabling PMD logs, you'd need to start maven with
<<<MAVEN_OPTS=-Dorg.slf4j.simpleLogger.log.net.sourceforge.pmd=off mvn <goals>
>>>.
```
Additionally, we should deprecate the parameter, remove any usages of this
parameter in the plugin itself and log a deprecation warning, if this parameter
is ever used by someone.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
