Skip to content

Commit 3d9fa63

Browse files
committed
Replace the use of Elements.overrides in MoreElements.getLocalAndInheritedMethods with a reimplementation that should work with Eclipse. Fixes #372.
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=133716523
1 parent 004463e commit 3d9fa63

File tree

8 files changed

+555
-10
lines changed

8 files changed

+555
-10
lines changed

common/src/main/java/com/google/auto/common/MoreElements.java

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.LinkedHashMultimap;
2727
import com.google.common.collect.SetMultimap;
2828
import java.lang.annotation.Annotation;
29+
import java.util.Collection;
2930
import java.util.LinkedHashSet;
3031
import java.util.List;
3132
import java.util.Set;
@@ -41,6 +42,7 @@
4142
import javax.lang.model.util.ElementFilter;
4243
import javax.lang.model.util.Elements;
4344
import javax.lang.model.util.SimpleElementVisitor6;
45+
import javax.lang.model.util.Types;
4446

4547
/**
4648
* Static utility methods pertaining to {@link Element} instances.
@@ -270,9 +272,50 @@ public boolean apply(Element input) {
270272
* {@link javax.annotation.processing.AbstractProcessor#processingEnv processingEnv}<!--
271273
* -->.{@link javax.annotation.processing.ProcessingEnvironment.getElementUtils()
272274
* getElementUtils()}
275+
*
276+
* @deprecated The method {@link #getLocalAndInheritedMethods(TypeElement, Types, Elements)}
277+
* has better consistency between Java compilers.
273278
*/
279+
@Deprecated
274280
public static ImmutableSet<ExecutableElement> getLocalAndInheritedMethods(
275281
TypeElement type, Elements elementUtils) {
282+
Overrides overrides = new Overrides.NativeOverrides(elementUtils);
283+
return getLocalAndInheritedMethods(type, overrides);
284+
}
285+
286+
/**
287+
* Returns the set of all non-private methods from {@code type}, including methods that it
288+
* inherits from its ancestors. Inherited methods that are overridden are not included in the
289+
* result. So if {@code type} defines {@code public String toString()}, the returned set will
290+
* contain that method, but not the {@code toString()} method defined by {@code Object}.
291+
*
292+
* <p>The returned set may contain more than one method with the same signature, if
293+
* {@code type} inherits those methods from different ancestors. For example, if it
294+
* inherits from unrelated interfaces {@code One} and {@code Two} which each define
295+
* {@code void foo();}, and if it does not itself override the {@code foo()} method,
296+
* then both {@code One.foo()} and {@code Two.foo()} will be in the returned set.
297+
*
298+
* @param type the type whose own and inherited methods are to be returned
299+
* @param typeUtils a {@link Types} object, typically returned by
300+
* {@link javax.annotation.processing.AbstractProcessor#processingEnv processingEnv}<!--
301+
* -->.{@link javax.annotation.processing.ProcessingEnvironment.getTypeUtils()
302+
* getTypeUtils()}
303+
* @param elementUtils an {@link Elements} object, typically returned by
304+
* {@link javax.annotation.processing.AbstractProcessor#processingEnv processingEnv}<!--
305+
* -->.{@link javax.annotation.processing.ProcessingEnvironment.getElementUtils()
306+
* getElementUtils()}
307+
*/
308+
public static ImmutableSet<ExecutableElement> getLocalAndInheritedMethods(
309+
TypeElement type, Types typeUtils, Elements elementUtils) {
310+
// TODO(emcmanus): detect if the Types and Elements are the javac ones, and use
311+
// NativeOverrides if so. We may need to adjust the logic further to avoid the bug
312+
// tested for by MoreElementsTest.getLocalAndInheritedMethods_DaggerBug.
313+
Overrides overrides = new Overrides.ExplicitOverrides(typeUtils, elementUtils);
314+
return getLocalAndInheritedMethods(type, overrides);
315+
}
316+
317+
private static ImmutableSet<ExecutableElement> getLocalAndInheritedMethods(
318+
TypeElement type, Overrides overrides) {
276319
SetMultimap<String, ExecutableElement> methodMap = LinkedHashMultimap.create();
277320
getLocalAndInheritedMethods(getPackage(type), type, methodMap);
278321
// Find methods that are overridden. We do this using `Elements.overrides`, which means
@@ -282,13 +325,13 @@ public static ImmutableSet<ExecutableElement> getLocalAndInheritedMethods(
282325
// methods in ancestor types precede those in descendant types, which means we only have to
283326
// check a method against the ones that follow it in that order.
284327
Set<ExecutableElement> overridden = new LinkedHashSet<ExecutableElement>();
285-
for (String methodName : methodMap.keySet()) {
286-
List<ExecutableElement> methodList = ImmutableList.copyOf(methodMap.get(methodName));
328+
for (Collection<ExecutableElement> methods : methodMap.asMap().values()) {
329+
List<ExecutableElement> methodList = ImmutableList.copyOf(methods);
287330
for (int i = 0; i < methodList.size(); i++) {
288331
ExecutableElement methodI = methodList.get(i);
289332
for (int j = i + 1; j < methodList.size(); j++) {
290333
ExecutableElement methodJ = methodList.get(j);
291-
if (elementUtils.overrides(methodJ, methodI, type)) {
334+
if (overrides.overrides(methodJ, methodI, type)) {
292335
overridden.add(methodI);
293336
}
294337
}
@@ -325,7 +368,7 @@ && methodVisibleFromPackage(method, pkg)) {
325368
}
326369
}
327370

328-
private static boolean methodVisibleFromPackage(ExecutableElement method, PackageElement pkg) {
371+
static boolean methodVisibleFromPackage(ExecutableElement method, PackageElement pkg) {
329372
// We use Visibility.ofElement rather than .effectiveVisibilityOfElement because it doesn't
330373
// really matter whether the containing class is visible. If you inherit a public method
331374
// then you have a public method, regardless of whether you inherit it from a public class.
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
* Copyright (C) 2016 Google, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.auto.common;
17+
18+
import javax.lang.model.element.ExecutableElement;
19+
import javax.lang.model.element.Modifier;
20+
import javax.lang.model.element.TypeElement;
21+
import javax.lang.model.type.DeclaredType;
22+
import javax.lang.model.type.ExecutableType;
23+
import javax.lang.model.util.Elements;
24+
import javax.lang.model.util.Types;
25+
26+
/**
27+
* Determines if one method overrides another. This class defines two ways of doing that:
28+
* {@code NativeOverrides} uses the method
29+
* {@link Elements#overrides(ExecutableElement, ExecutableElement, TypeElement)} while
30+
* {@code ExplicitOverrides} reimplements that method in a way that is more consistent between
31+
* compilers, in particular between javac and ecj (the Eclipse compiler).
32+
*
33+
* @author [email protected] (Éamonn McManus)
34+
*/
35+
abstract class Overrides {
36+
abstract boolean overrides(
37+
ExecutableElement overrider, ExecutableElement overridden, TypeElement in);
38+
39+
static class NativeOverrides extends Overrides {
40+
private final Elements elementUtils;
41+
42+
NativeOverrides(Elements elementUtils) {
43+
this.elementUtils = elementUtils;
44+
}
45+
46+
@Override
47+
boolean overrides(ExecutableElement overrider, ExecutableElement overridden, TypeElement in) {
48+
return elementUtils.overrides(overrider, overridden, in);
49+
}
50+
}
51+
52+
static class ExplicitOverrides extends Overrides {
53+
private final Types typeUtils;
54+
private final Elements elementUtils;
55+
56+
ExplicitOverrides(Types typeUtils, Elements elementUtils) {
57+
this.typeUtils = typeUtils;
58+
this.elementUtils = elementUtils;
59+
}
60+
61+
@Override
62+
public boolean overrides(ExecutableElement overrider, ExecutableElement overridden,
63+
TypeElement in) {
64+
if (overrider.equals(overridden)) {
65+
return false;
66+
}
67+
if (!overrider.getSimpleName().equals(overridden.getSimpleName())) {
68+
// They must have the same name.
69+
return false;
70+
}
71+
if (overridden.getModifiers().contains(Modifier.STATIC)) {
72+
// Static methods can't be overridden (though they can be hidden by other static methods).
73+
return false;
74+
}
75+
Visibility overriddenVisibility = Visibility.ofElement(overridden);
76+
Visibility overriderVisibility = Visibility.ofElement(overrider);
77+
if (overriddenVisibility.equals(Visibility.PRIVATE)
78+
|| overriderVisibility.compareTo(overriddenVisibility) < 0) {
79+
// Private methods can't be overridden, and methods can't be overridden by less-visible
80+
// methods. The latter condition is enforced by the compiler so in theory we might report
81+
// an "incorrect" result here for code that javac would not have allowed.
82+
return false;
83+
}
84+
DeclaredType inType = MoreTypes.asDeclared(in.asType());
85+
ExecutableType overriderExecutable;
86+
ExecutableType overriddenExecutable;
87+
try {
88+
overriderExecutable = MoreTypes.asExecutable(typeUtils.asMemberOf(inType, overrider));
89+
overriddenExecutable = MoreTypes.asExecutable(typeUtils.asMemberOf(inType, overridden));
90+
} catch (IllegalArgumentException e) {
91+
// This might mean that at least one of the methods is not in fact declared in or inherited
92+
// by `in` (in which case we should indeed return false); or it might mean that we are
93+
// tickling an Eclipse bug such as https://bugs.eclipse.org/bugs/show_bug.cgi?id=499026
94+
// (in which case we can't do any better than returning false).
95+
return false;
96+
}
97+
if (!typeUtils.isSubsignature(overriderExecutable, overriddenExecutable)) {
98+
return false;
99+
}
100+
if (!MoreElements.methodVisibleFromPackage(overridden, MoreElements.getPackage(overrider))) {
101+
// If the overridden method is a package-private method in a different package then it
102+
// can't be overridden.
103+
return false;
104+
}
105+
TypeElement overriddenType;
106+
if (!(overridden.getEnclosingElement() instanceof TypeElement)) {
107+
return false;
108+
// We don't know how this could happen but we avoid blowing up if it does.
109+
}
110+
overriddenType = MoreElements.asType(overridden.getEnclosingElement());
111+
// We erase the types before checking subtypes, because the TypeMirror we get for List<E> is
112+
// not a subtype of the one we get for Collection<E> since the two E instances are not the
113+
// same. For the purposes of overriding, type parameters in the containing type should not
114+
// matter because if the code compiles at all then they must be consistent.
115+
if (!typeUtils.isSubtype(
116+
typeUtils.erasure(in.asType()), typeUtils.erasure(overriddenType.asType()))) {
117+
return false;
118+
}
119+
if (in.getKind().isClass()) {
120+
// Method mC in or inherited by class C (JLS 8.4.8.1)...
121+
if (overriddenType.getKind().isClass()) {
122+
// ...overrides from C another method mA declared in class A. The only condition we
123+
// haven't checked is that C does not inherit mA.
124+
return !elementUtils.getAllMembers(in).contains(overridden);
125+
} else if (overriddenType.getKind().isInterface()) {
126+
// ...overrides from C another method mI declared in interface I. We've already checked
127+
// the conditions (assuming that the only alternative to mI being abstract or default is
128+
// mI being static, which we eliminated above). However, it appears that the logic here
129+
// is necessary in order to be compatible with javac's `overrides` method. An inherited
130+
// abstract method does not override another method. (But, if it is not inherited,
131+
// it does, including if `in` inherits a concrete method of the same name from its
132+
// superclass.)
133+
if (overrider.getModifiers().contains(Modifier.ABSTRACT)) {
134+
return !elementUtils.getAllMembers(in).contains(overridden);
135+
} else {
136+
return true;
137+
}
138+
} else {
139+
// We don't know what this is so say no.
140+
return false;
141+
}
142+
} else {
143+
return in.getKind().isInterface();
144+
// Method mI in or inherited by interface I (JLS 9.4.1.1). We've already checked everything.
145+
// If this is not an interface then we don't know what it is so we say no.
146+
}
147+
}
148+
}
149+
}

common/src/test/java/com/google/auto/common/MoreElementsTest.java

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ public void initializeTestElements() {
7070

7171
@Test
7272
public void getPackage() {
73-
assertThat(javaLangPackageElement).isEqualTo(javaLangPackageElement);
7473
assertThat(MoreElements.getPackage(stringElement)).isEqualTo(javaLangPackageElement);
7574
for (Element childElement : stringElement.getEnclosedElements()) {
7675
assertThat(MoreElements.getPackage(childElement)).isEqualTo(javaLangPackageElement);
@@ -229,12 +228,13 @@ void buh(int x, int y) {}
229228
}
230229

231230
@Test
232-
public void getLocalAndInheritedMethods() {
231+
public void getLocalAndInheritedMethods_Old() {
233232
Elements elements = compilation.getElements();
234233
Types types = compilation.getTypes();
235234
TypeMirror intMirror = types.getPrimitiveType(TypeKind.INT);
236235
TypeMirror longMirror = types.getPrimitiveType(TypeKind.LONG);
237236
TypeElement childType = elements.getTypeElement(Child.class.getCanonicalName());
237+
@SuppressWarnings("deprecation")
238238
Set<ExecutableElement> childTypeMethods =
239239
MoreElements.getLocalAndInheritedMethods(childType, elements);
240240
Set<ExecutableElement> objectMethods = visibleMethodsFromObject();
@@ -249,6 +249,63 @@ public void getLocalAndInheritedMethods() {
249249
getMethod(Child.class, "buh", intMirror, intMirror));
250250
}
251251

252+
@Test
253+
public void getLocalAndInheritedMethods() {
254+
Elements elements = compilation.getElements();
255+
Types types = compilation.getTypes();
256+
TypeMirror intMirror = types.getPrimitiveType(TypeKind.INT);
257+
TypeMirror longMirror = types.getPrimitiveType(TypeKind.LONG);
258+
TypeElement childType = elements.getTypeElement(Child.class.getCanonicalName());
259+
@SuppressWarnings("deprecation")
260+
Set<ExecutableElement> childTypeMethods =
261+
MoreElements.getLocalAndInheritedMethods(childType, types, elements);
262+
Set<ExecutableElement> objectMethods = visibleMethodsFromObject();
263+
assertThat(childTypeMethods).containsAllIn(objectMethods);
264+
Set<ExecutableElement> nonObjectMethods = Sets.difference(childTypeMethods, objectMethods);
265+
assertThat(nonObjectMethods).containsExactly(
266+
getMethod(ParentClass.class, "foo"),
267+
getMethod(ParentInterface.class, "bar", longMirror),
268+
getMethod(Child.class, "bar"),
269+
getMethod(Child.class, "baz"),
270+
getMethod(Child.class, "buh", intMirror),
271+
getMethod(Child.class, "buh", intMirror, intMirror));
272+
}
273+
274+
static class Injectable {}
275+
276+
public static class MenuManager {
277+
public interface ParentComponent extends MenuItemA.ParentComponent, MenuItemB.ParentComponent {}
278+
}
279+
280+
public static class MenuItemA {
281+
public interface ParentComponent {
282+
Injectable injectable();
283+
}
284+
}
285+
286+
public static class MenuItemB {
287+
public interface ParentComponent {
288+
Injectable injectable();
289+
}
290+
}
291+
292+
public static class Main {
293+
public interface ParentComponent extends MenuManager.ParentComponent {}
294+
}
295+
296+
// Example from https://github.com/williamlian/daggerbug
297+
@Test
298+
public void getLocalAndInheritedMethods_DaggerBug() {
299+
Elements elementUtils = compilation.getElements();
300+
TypeElement main = elementUtils.getTypeElement(Main.ParentComponent.class.getCanonicalName());
301+
Set<ExecutableElement> methods = MoreElements.getLocalAndInheritedMethods(
302+
main, compilation.getTypes(), elementUtils);
303+
assertThat(methods).hasSize(1);
304+
ExecutableElement method = methods.iterator().next();
305+
assertThat(method.getSimpleName().toString()).isEqualTo("injectable");
306+
assertThat(method.getParameters()).isEmpty();
307+
}
308+
252309
private Set<ExecutableElement> visibleMethodsFromObject() {
253310
Types types = compilation.getTypes();
254311
TypeMirror intMirror = types.getPrimitiveType(TypeKind.INT);

0 commit comments

Comments
 (0)