Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] ReplaceVectorWithList false-positive (neither Vector nor List usage) #4852

Closed
blacelle opened this issue Mar 1, 2024 · 4 comments · Fixed by #4984
Closed

[java] ReplaceVectorWithList false-positive (neither Vector nor List usage) #4852

blacelle opened this issue Mar 1, 2024 · 4 comments · Fixed by #4984
Assignees
Labels
a:false-positive PMD flags a piece of code that is not problematic in:type-resolution Affects the type resolution code
Milestone

Comments

@blacelle
Copy link

blacelle commented Mar 1, 2024

This follows some feedback in #4816

Affects PMD Version: 7.0.0-rc4

Rule: ReplaceVectorWithList

Description:
The 3 sets of classes generate false-positive.

[INFO] PMD Failure: pmd.false_positive.AAntiFraudPolicy:8 Rule:ReplaceVectorWithList Priority:3 Consider replacing this Vector with the newer java.util.List.
[INFO] PMD Failure: pmd.false_positive.AAntiFraudPolicy:8 Rule:ReplaceVectorWithList Priority:3 Consider replacing this Vector with the newer java.util.List.
[INFO] PMD Failure: pmd.false_positive.AAntiFraudPolicy:8 Rule:ReplaceVectorWithList Priority:3 Consider replacing this Vector with the newer java.util.List.
[INFO] PMD Failure: pmd.false_positive.AAntiFraudPolicy:9 Rule:ReplaceVectorWithList Priority:3 Consider replacing this Vector with the newer java.util.List.

Code Sample demonstrating the issue:

package pmd.false_positive;

public enum AntiFraudCheckResult {
	UNAVAILABLE;
}
package pmd.false_positive;

public interface IAntiFraudCriterion {
}
package pmd.false_positive;

import java.util.Map;
import java.util.function.Predicate;

public class AAntiFraudPolicy<C extends IAntiFraudCriterion> {

	public static <E extends Enum<E> & IAntiFraudCriterion> AAntiFraudPolicy<E> of(Class<E> enumClass,
			Map<E, Predicate<AntiFraudCheckResult>> criterionChecks) {
		return new AAntiFraudPolicy<E>();
	}

}

Expected outcome:

PMD reports a violation at line ..., but that's wrong. That's a false positive.

Running PMD through: [CLI | Ant | Maven | Gradle | Designer | Other]

@blacelle blacelle added the a:false-positive PMD flags a piece of code that is not problematic label Mar 1, 2024
@blacelle blacelle changed the title [java] ReplaceVectorWithList false-positive [java] ReplaceVectorWithList false-positive (neither Vector nor List usage) Mar 1, 2024
@adangel adangel added the in:type-resolution Affects the type resolution code label Mar 4, 2024
@adangel
Copy link
Member

adangel commented Mar 4, 2024

Thanks for reporting this issue.

I can only reproduce the problem, if I call PMD without compiling the project first - that means, without providing a correct --aux-classpath. Without a correct auxclasspath, PMD can't correctly determine the types.

The auxclasspath is usually determined automatically by build tools like maven or gradle. How do you call PMD exactly?

I'll keep this issue open, to analyze the problem further. Because I don't understand atm, why we would figure out, that "E" would be of type "java.util.Vector". Ideally we would just not detect any problems, if the auxclasspath is wrong (that means, I would expect false negatives rather than false positives).

@blacelle
Copy link
Author

blacelle commented Mar 4, 2024

How do you call PMD exactly?

For this issue, I was indeed doing mvn clean pmd:check hence to provide compiled classes to PMD. However, this was a second-order issue from #4816, which was (seemingly) encountering this on a plain mvn install on a plain mvn compile at the root of a multi-module project, hence with a proper --aux-classpath (?) (given our mvn+PMD setup is not fancy).

@blacelle
Copy link
Author

blacelle commented Mar 8, 2024

A resync of our project around PMD7 migration led me to check the default CI command: ./mvnw clean compile -DskipTests -PskipITs,skipJs

Is it possible using compile instead of install leads to improper --aux-classpath ?

I tried locally with compile: it leads to additional reports from PMD, again different from CI:

[INFO] --- pmd:3.21.2:check (default) @ oauth2-client ---
[INFO] PMD version: 7.0.0-rc4
[INFO] PMD Failure: io.XXX:187 Rule:CloseResource Priority:3 Ensure that resources like this ExecutorService object are closed after use.
[INFO] PMD Failure: io.YYY:145 Rule:CloseResource Priority:3 Ensure that resources like this ExecutorService object are closed after use.

while ExecutorService does not implement AutoCloseable and it seems not a special case of CloseResource (https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java#L89).

  1. Is mvn compile considered improper for PMD in multi-module setup (regarding --aux-classpath build)? I would expect PMD to have access to all necessary compiled stuff given the (multi-module) project compilation is OK (and other static analysis tools relying on compiled stuff are happy (e.g. Spotbugs))
  2. As a PMD User, should I report false-positive with improper --aux-classpath? (And, should PMD helps detecting+reporting improper --aux-classpath? (e.g. at least a WARN indicating something seems wrong with current --aux-classpath given type resolution issues))

@jsotuyod jsotuyod added needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale and removed needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale labels Mar 17, 2024
@jsotuyod
Copy link
Member

@blacelle sorry for the long delay, I'm circling back on this…

while ExecutorService does not implement AutoCloseable and it seems not a special case of CloseResource

That's not entirely true… since Java 20, ExecutorService does implement AutoClosable. If that's the version of Java you are running, that is what PMD will consider by default, as the Maven plugin has no support for toolchains AFAIK.

So if CI and local are using different Java versions, these differences are to be expected.

Is it possible using compile instead of install leads to improper --aux-classpath ?

I wouldn't think so, but @adangel is probably better suited to respond…

However, I would recommend first updating the plugin to version 3.22.0 which natively supports PMD 7, and using PMD 7.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic in:type-resolution Affects the type resolution code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants