[ 
https://issues.apache.org/jira/browse/MPMD-379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17834415#comment-17834415
 ] 

ASF GitHub Bot commented on MPMD-379:
-------------------------------------

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.





> Upgrade to use PMD 7.0.0 by default
> -----------------------------------
>
>                 Key: MPMD-379
>                 URL: https://issues.apache.org/jira/browse/MPMD-379
>             Project: Maven PMD Plugin
>          Issue Type: Improvement
>          Components: CPD, PMD
>            Reporter: Andreas Dangel
>            Assignee: Andreas Dangel
>            Priority: Major
>
> Add support for the new major version of PMD.
> This gives support for analyzing Java 21 code.
> The upgrade from PMD 6 to PMD 7 is a major upgrade, that might impact 
> end-users, if they use custom rulesets (see 
> [https://maven.apache.org/plugins/maven-pmd-plugin/examples/usingRuleSets.html])
>  or if they override the dependencies to upgrade PMD at runtime and currently 
> use PMD 6.x (see 
> [https://maven.apache.org/plugins/maven-pmd-plugin/examples/upgrading-PMD-at-runtime.html]).
>  
> Most likely, end-users have to review their rulesets and migrate them to PMD 
> 7. Rules might have been renamed or replaced. See 
> [https://docs.pmd-code.org/latest/pmd_release_notes_pmd7.html] and 
> [https://docs.pmd-code.org/latest/pmd_userdocs_migrating_to_pmd7.html] .
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to