adangel commented on code in PR #144:
URL: https://github.com/apache/maven-pmd-plugin/pull/144#discussion_r1534120578
##########
pom.xml:
##########
@@ -83,7 +83,7 @@ under the License.
<properties>
<mavenVersion>3.2.5</mavenVersion>
<javaVersion>8</javaVersion>
- <pmdVersion>6.55.0</pmdVersion>
+ <pmdVersion>7.0.0-rc4</pmdVersion>
Review Comment:
I'm going to release PMD 7.0.0 tomorrow. After that, we can use directly
7.0.0 here.
Note, that there will be a few more changes required...
Also, please add a new row in
https://github.com/apache/maven-pmd-plugin/blob/master/src/site/apt/examples/upgrading-PMD-at-runtime.apt.vm#L91
This should document the default pmd version, that is used by
maven-pmd-plugin - and this is determined by this property here.
##########
src/it/MPMD-244-logging/verify.groovy:
##########
@@ -20,12 +20,13 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
assert buildLog.text.contains( "PMD processing errors" )
-assert buildLog.text.contains( "Error while parsing" )
+assert buildLog.text.contains( "Parse exception in file" )
String disabledPath = new File( basedir,
'logging-disabled/src/main/java/BrokenFile.java' ).getCanonicalPath()
String enabledPath = new File( basedir,
'logging-enabled/src/main/java/BrokenFile.java' ).getCanonicalPath()
// logging disabled: the pmd exception is only output through the processing
error reporting (since MPMD-246)
-assert 1 == buildLog.text.count( "net.sourceforge.pmd.PMDException: Error
while parsing ${disabledPath}" )
+assert 1 == buildLog.text.count( "net.sourceforge.pmd.lang.ast.ParseException:
Parse exception in file \'${disabledPath}\'" )
// logging enabled: the pmd exception is output twice: through the processing
error reporting (since MPMD-246) and through PMD's own logging
-assert 2 == buildLog.text.count( "net.sourceforge.pmd.PMDException: Error
while parsing ${enabledPath}" )
+// not true anymore, logged always once
+assert 1 == buildLog.text.count( "net.sourceforge.pmd.lang.ast.ParseException:
Parse exception in file \'${enabledPath}\'" )
Review Comment:
I think, this deserves a little bit more work. I noted in my personal branch:
```
// build.log contains the logging from the two PMD executions
// only one execution has logging enabled, so we expect only one log output
// TODO assert 1 == buildLog.text.count( "[DEBUG] Rules loaded from" )
// TODO logging is always enabled and can't be disabled, because PMD 7
switched to slf4j
```
Which essentially means, that the property
[showPmdLog](https://maven.apache.org/plugins/maven-pmd-plugin/pmd-mojo.html#showPmdLog)
is broken now. Either we need to deprecate it and eventually remove it, or we
need to fix it.
##########
src/main/java/org/apache/maven/plugins/pmd/ExcludeViolationsFromFile.java:
##########
@@ -84,8 +84,10 @@ public boolean isExcludedFromFailure(final Violation
errorDetail) {
* @return <code>true</code> if the violation should be excluded,
<code>false</code> otherwise.
*/
public boolean isExcludedFromFailure(final RuleViolation errorDetail) {
- final String className =
- extractClassName(errorDetail.getPackageName(),
errorDetail.getClassName(), errorDetail.getFilename());
+ final String className = extractClassName(
+ errorDetail.getPackageName(),
+ errorDetail.getClassName(),
+ errorDetail.getFileId().getAbsolutePath());
Review Comment:
This will change in final PMD 7.0.0, see here for the needed change:
https://github.com/apache/maven-pmd-plugin/blob/02910c902b168ce14ef948d65db7b239a5ee56c4/src/main/java/org/apache/maven/plugins/pmd/ExcludeViolationsFromFile.java#L87-L92
##########
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:
Does this feature still work? It is enabled with the property
[benchmark](https://maven.apache.org/plugins/maven-pmd-plugin/pmd-mojo.html#benchmark)
##########
src/it/MPMD-258-multiple-executions/invoker.properties:
##########
@@ -15,5 +15,6 @@
# specific language governing permissions and limitations
# under the License.
+invoker.debug = true
Review Comment:
this might not be needed?
Update: ok, yes it is needed. Can you please update verify.groovy for this
test to explain it, similar what I have done here:
https://github.com/apache/maven-pmd-plugin/blob/02910c902b168ce14ef948d65db7b239a5ee56c4/src/it/MPMD-258-multiple-executions/verify.groovy#L24
and
https://github.com/apache/maven-pmd-plugin/blob/02910c902b168ce14ef948d65db7b239a5ee56c4/src/it/MPMD-258-multiple-executions/verify.groovy#L28
##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java:
##########
@@ -158,43 +157,44 @@ private CpdResult run() throws MavenReportException {
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);
+ CpdAnalysis cpd = CpdAnalysis.create(cpdConfiguration);
Review Comment:
CpdAnalysis should be used within a try-with-resources statement
##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java:
##########
@@ -158,43 +157,44 @@ private CpdResult run() throws MavenReportException {
Language cpdLanguage;
if ("java".equals(request.getLanguage()) || null ==
request.getLanguage()) {
- cpdLanguage = new JavaLanguage(request.getLanguageProperties());
Review Comment:
The languageProperties are not used anymore now.
These are needed for the properties like
[ignoreAnnotations](https://maven.apache.org/plugins/maven-pmd-plugin/cpd-mojo.html#ignoreAnnotations).
With PMD 7, these flags need to be set now directly on cpdConfiguration, e.g.
```
cpdConfiguration.setIgnoreAnnotations(...)
```
--
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]
