Skip to content

Commit

Permalink
Refine the check for overriding interface methods.
Browse files Browse the repository at this point in the history
One interface method overrides another only if the overrider is in a subinterface of the overridden. This is the first bullet in [JLS §9.4.1.1](https://docs.oracle.com/javase/specs/jls/se17/html/jls-9.html#jls-9.4.1.1).

Remove a test that was checking for the old behaviour. It was intended to fix a bug in Dagger, but Dagger now works fine without this fix, and has its own test for the relevant situation.

RELNOTES=In `MoreElements.overrides` and `.getLocalAndInheritedMethods`, an interface method is no longer considered to override another interface method unless the first interface inherits from the second. This means that `.getLocalAndInheritedMethods` may now return two methods with the same signature where before it only returned one.
PiperOrigin-RevId: 403430316
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Oct 15, 2021
1 parent c0ed322 commit d8c1934
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 38 deletions.
9 changes: 7 additions & 2 deletions common/src/main/java/com/google/auto/common/Overrides.java
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,14 @@ public boolean overrides(
return false;
}
} else {
return in.getKind().isInterface();
// Method mI in or inherited by interface I (JLS 9.4.1.1). We've already checked everything.
// Method mI in or inherited by interface I (JLS 9.4.1.1). We've already checked everything,
// except that `overrider` must also be in a subinterface of `overridden`.
// If this is not an interface then we don't know what it is so we say no.
TypeElement overriderType = MoreElements.asType(overrider.getEnclosingElement());
return in.getKind().isInterface()
&& typeUtils.isSubtype(
typeUtils.erasure(overriderType.asType()),
typeUtils.erasure(overriddenType.asType()));
}
}

Expand Down
34 changes: 0 additions & 34 deletions common/src/test/java/com/google/auto/common/MoreElementsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -388,40 +388,6 @@ public void getAllMethods() {
.inOrder();
}

static class Injectable {}

public static class MenuManager {
public interface ParentComponent extends MenuItemA.ParentComponent, MenuItemB.ParentComponent {}
}

public static class MenuItemA {
public interface ParentComponent {
Injectable injectable();
}
}

public static class MenuItemB {
public interface ParentComponent {
Injectable injectable();
}
}

public static class Main {
public interface ParentComponent extends MenuManager.ParentComponent {}
}

// Example from https://github.com/williamlian/daggerbug
@Test
public void getLocalAndInheritedMethods_DaggerBug() {
TypeElement main = elements.getTypeElement(Main.ParentComponent.class.getCanonicalName());
Set<ExecutableElement> methods =
MoreElements.getLocalAndInheritedMethods(main, compilation.getTypes(), elements);
assertThat(methods).hasSize(1);
ExecutableElement method = methods.iterator().next();
assertThat(method.getSimpleName().toString()).isEqualTo("injectable");
assertThat(method.getParameters()).isEmpty();
}

private Set<ExecutableElement> visibleMethodsFromObject() {
Types types = compilation.getTypes();
TypeMirror intMirror = types.getPrimitiveType(TypeKind.INT);
Expand Down
30 changes: 28 additions & 2 deletions common/src/test/java/com/google/auto/common/OverridesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@
@RunWith(Parameterized.class)
public class OverridesTest {
@Parameterized.Parameters(name = "{0}")
public static ImmutableList<CompilerType> data() {
return ImmutableList.of(CompilerType.JAVAC, CompilerType.ECJ);
public static CompilerType[] data() {
return CompilerType.values();
}

@Rule public CompilationRule compilation = new CompilationRule();
Expand Down Expand Up @@ -133,12 +133,16 @@ interface One {
void m(String x);

void n();

Number number();
}

interface Two {
void m();

void m(int x);

Integer number();
}

static class Parent {
Expand All @@ -156,6 +160,11 @@ public void m(String x) {}

@Override
public void n() {}

@Override
public Number number() {
return 0;
}
}

static class ChildOfOneAndTwo implements One, Two {
Expand All @@ -170,6 +179,11 @@ public void m(int x) {}

@Override
public void n() {}

@Override
public Integer number() {
return 0;
}
}

static class ChildOfParentAndOne extends Parent implements One {
Expand All @@ -181,6 +195,11 @@ public void m(String x) {}

@Override
public void n() {}

@Override
public Number number() {
return 0;
}
}

static class ChildOfParentAndOneAndTwo extends Parent implements One, Two {
Expand All @@ -192,13 +211,20 @@ public void m(int x) {}

@Override
public void n() {}

@Override
public Integer number() {
return 0;
}
}

abstract static class AbstractChildOfOne implements One {}

abstract static class AbstractChildOfOneAndTwo implements One, Two {}

abstract static class AbstractChildOfParentAndOneAndTwo extends Parent implements One, Two {}

interface ExtendingOneAndTwo extends One, Two {}
}

static class MoreTypesForInheritance {
Expand Down

0 comments on commit d8c1934

Please sign in to comment.