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] MissingOverride long-standing issues #2797

Closed
oowekyala opened this issue Sep 19, 2020 · 1 comment · Fixed by #2800
Closed

[java] MissingOverride long-standing issues #2797

oowekyala opened this issue Sep 19, 2020 · 1 comment · Fixed by #2800
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Milestone

Comments

@oowekyala
Copy link
Member

oowekyala commented Sep 19, 2020

Affects PMD Version: Since 6.2.0, when the rule was introduced

Rule: MissingOverride

Description: There are 2 long-standing issues with MissingOverride, related to overriding of a method using type parameters of a parameterized supertype. This is caused by its implementation strategy, since it counts bridge methods in a Class file.

The issue is described at length around this line:

There are already regression tests in the test file:

<test-code>
<description>Avoid false positives when in ambiguous situation</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
package net.sourceforge.pmd.lang.java.rule.bestpractices.missingoverride;
import java.util.Comparator;
public class AmbiguousOverload implements Comparator<StringBuilder> {
// only one of those overloads is an override, and so there's only one bridge,
// so we can't choose the inherited overload
// missing
public int compare(StringBuilder o1, StringBuilder o2) {
return 0;
}
public int compare(String s, String s2) {
return 0;
}
}
]]></code>
</test-code>
<test-code>
<description>Avoid false positives when in ambiguous situation</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
package net.sourceforge.pmd.lang.java.rule.bestpractices.missingoverride;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.AbstractJavaTypeNode;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
public abstract class HierarchyWithSeveralBridges<T extends Node> {
abstract void foo(T node);
public static abstract class SubclassOne<T extends JavaNode> extends HierarchyWithSeveralBridges<T> {
// this one could be resolved
// @Override
// abstract void foo(T node);
}
public static abstract class SubclassTwo<T extends AbstractJavaTypeNode> extends SubclassOne<T> {
}
public static class Concrete extends SubclassTwo<ASTType> {
// bridges: foo(AbstractJavaTypeNode), foo(JavaNode), foo(Node)
// missing
void foo(ASTType node) {
}
}
}
]]></code>
</test-code>

This may be fixed easily in PMD 7, since we don't use Class files and have all necessary utilities to test for overriding in TypeOps already:

public static boolean overrides(JMethodSig m1, JMethodSig m2, JTypeMirror origin) {

public static boolean areOverrideEquivalent(JMethodSig m1, JMethodSig m2) {

Expected outcome:

PMD should report violations for the two test cases, this is a false negative

@oowekyala oowekyala added the a:false-negative PMD doesn't flag a problematic piece of code label Sep 19, 2020
@oowekyala oowekyala added this to the 7.0.0 milestone Sep 19, 2020
@oowekyala oowekyala linked a pull request Oct 3, 2020 that will close this issue
4 tasks
adangel added a commit that referenced this issue Oct 16, 2020
@adangel
Copy link
Member

adangel commented Oct 16, 2020

Fixed by #2800

@adangel adangel closed this as completed Oct 16, 2020
@adangel adangel mentioned this issue Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants