Skip to content

MissingOverrideAnnotation: removeFalsyOverrideAnnotation #684

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

MissingOverrideAnnotation: removeFalsyOverrideAnnotation #684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions src/main/java/org/openrewrite/staticanalysis/MissingOverrideAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,22 @@

import lombok.EqualsAndHashCode;
import lombok.Value;
import lombok.var;
import org.jspecify.annotations.Nullable;
Comment on lines +20 to 21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import lombok.var;
import org.jspecify.annotations.Nullable;
import org.jspecify.annotations.Nullable;

import org.openrewrite.*;
import org.openrewrite.java.AnnotationMatcher;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.service.AnnotationService;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;
import org.openrewrite.java.tree.TypeUtils;
import org.openrewrite.kotlin.tree.K;

import java.util.Comparator;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static java.util.Collections.singleton;

Expand All @@ -49,6 +53,7 @@ public String getDisplayName() {
@Override
public String getDescription() {
return "Adds `@Override` to methods overriding superclass methods or implementing interface methods. " +
"Also removes incorrectly applied `@Override` annotations from methods that do not override anything. " +
"Annotating methods improves readability by showing the author's intent to override. " +
"Additionally, when annotated, the compiler will emit an error when a signature of the overridden method does not match the superclass method.";
}
Expand Down Expand Up @@ -78,15 +83,33 @@ private Cursor getCursorToParentScope(Cursor cursor) {

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
if (!method.hasModifier(J.Modifier.Type.Static) &&
!method.isConstructor() &&
!service(AnnotationService.class).matches(getCursor(), OVERRIDE_ANNOTATION) &&
TypeUtils.isOverride(method.getMethodType()) &&
!(Boolean.TRUE.equals(ignoreAnonymousClassMethods) &&
getCursorToParentScope(getCursor()).getValue() instanceof J.NewClass)) {

method = JavaTemplate.apply("@Override", getCursor(), method.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName)));
boolean hasOverride = service(AnnotationService.class).matches(getCursor(), OVERRIDE_ANNOTATION);
boolean isOverride = TypeUtils.isOverride(method.getMethodType());

if (!method.hasModifier(J.Modifier.Type.Static) && !method.isConstructor()) {

// Remove incorrect @Override
if (hasOverride && !isOverride) {
// Filter out the @Override annotation
method = method.withLeadingAnnotations(method.getLeadingAnnotations().stream()
.filter(ann -> !OVERRIDE_ANNOTATION.matches(ann))
.collect(Collectors.toList()));
// Remove extra prefix whitespace caused by annotation removal (if any)
method = method.withPrefix(Space.format(method.getPrefix().getWhitespace().replaceAll("[ \t]+(?=\\n)", "")));
}

// Add @Override if missing and appropriate
else if (!hasOverride && isOverride &&
!(Boolean.TRUE.equals(ignoreAnonymousClassMethods) &&
getCursorToParentScope(getCursor()).getValue() instanceof J.NewClass)) {
method = JavaTemplate.apply(
"@Override",
getCursor(),
method.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName))
);
}
}

return super.visitMethodDeclaration(method, ctx);
}
}
Expand Down
202 changes: 201 additions & 1 deletion src/test/java/org/openrewrite/staticanalysis/MissingOverrideAnnotationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package org.openrewrite.staticanalysis;

import org.intellij.lang.annotations.Language;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.test.RecipeSpec;
Expand Down Expand Up @@ -524,4 +526,202 @@ public interface Reproducer extends Cloneable {
"""
));
}
}

@Nested
class removeFalsyOverrideAnnotation {

@Test
void simple() {
rewriteRun(
//language=java
java(
"""
package com.example;

class Test {
@Override
public void doesNotOverrideAnything() {
}
}
""",
"""
package com.example;

class Test {
\s
public void doesNotOverrideAnything() {
}
}
""")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
"""
)

);
}

@Test
void multipleMethods() {
rewriteRun(
//language=java
java(
"""
package com.example;

class Test {
@Override
public void method1() {
}

@Override
public void method2() {
}
}
""",
"""
package com.example;

class Test {
\s
public void method1() {
}

\s
public void method2() {
}
}
""")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
"""
)

);
}

@Test
void withValidOverride() {
rewriteRun(
//language=java
java(
"""
package com.example;

class Parent {
public void validMethod() {}
}

class Test extends Parent {
@Override
public void validMethod() {}

@Override
public void invalidMethod() {}
}
""",
"""
package com.example;

class Parent {
public void validMethod() {}
}

class Test extends Parent {
@Override
public void validMethod() {}

\s
public void invalidMethod() {}
}
""")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
"""
)

);
}

@Test
void withInterfaceImplementation() {
rewriteRun(
//language=java
java(
"""
package com.example;

interface MyInterface {
void interfaceMethod();
}

class Test implements MyInterface {
@Override
public void interfaceMethod() {}

@Override
public void nonInterfaceMethod() {}
}
""",
"""
package com.example;

interface MyInterface {
void interfaceMethod();
}

class Test implements MyInterface {
@Override
public void interfaceMethod() {}

\s
public void nonInterfaceMethod() {}
}
""")
);
Comment on lines +665 to +666
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
);
"""
)
@Test

}

@Test
@Disabled
void withComments() {
rewriteRun(
//language=java
java(
"""
package com.example;

class Test {
/**
* Some documentation
*/
@Override
public void documentedMethod() {
}
}
""",
"""
package com.example;

class Test {
/**
* Some documentation
*/
public void documentedMethod() {
}
}
""")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
"""
)

);
}

@Test
void withAnnotations() {
rewriteRun(
//language=java
java(
"""
package com.example;

class Test {
@Deprecated
@Override
public void annotatedMethod() {
}
}
""",
"""
package com.example;

class Test {
@Deprecated
public void annotatedMethod() {
}
}
""")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""")
"""
)

);
}
}}
Loading