Remove duplicate isEnclosedIn logic from DaggerElements and ValidationReport.
This CL removes logic that was duplicated between the two classes. It also simplifies DaggerElements version to check the `enclosed.getEnclosingElement()` rather than comparing the `enclosed` with every recursively inclosed element of `encloser`, which should be a lot more efficient.
I've also rewritten closestEnclosingTypeElement to avoid using the ElementVisitor, which will likely be more difficult to use once we convert to XProcessing.
RELNOTES=N/A
PiperOrigin-RevId: 390419676
diff --git a/java/dagger/internal/codegen/langmodel/DaggerElements.java b/java/dagger/internal/codegen/langmodel/DaggerElements.java
index 413e3b5..0ca2a0c 100644
--- a/java/dagger/internal/codegen/langmodel/DaggerElements.java
+++ b/java/dagger/internal/codegen/langmodel/DaggerElements.java
@@ -30,7 +30,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.common.graph.Traverser;
import com.squareup.javapoet.ClassName;
import dagger.Reusable;
import dagger.internal.codegen.base.ClearableCache;
@@ -49,7 +48,6 @@
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
-import javax.lang.model.element.ElementVisitor;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Name;
import javax.lang.model.element.PackageElement;
@@ -70,7 +68,6 @@
import javax.lang.model.type.WildcardType;
import javax.lang.model.util.AbstractTypeVisitor8;
import javax.lang.model.util.Elements;
-import javax.lang.model.util.SimpleElementVisitor8;
import javax.lang.model.util.Types;
/** Extension of {@link Elements} that adds Dagger-specific methods. */
@@ -91,16 +88,19 @@
}
/**
- * Returns {@code true} if {@code encloser} is equal to {@code enclosed} or recursively encloses
- * it.
+ * Returns {@code true} if {@code encloser} is equal to or recursively encloses {@code enclosed}.
*/
- public static boolean elementEncloses(TypeElement encloser, Element enclosed) {
- return Iterables.contains(GET_ENCLOSED_ELEMENTS.breadthFirst(encloser), enclosed);
+ public static boolean transitivelyEncloses(Element encloser, Element enclosed) {
+ Element current = enclosed;
+ while (current != null) {
+ if (current.equals(encloser)) {
+ return true;
+ }
+ current = current.getEnclosingElement();
+ }
+ return false;
}
- private static final Traverser<Element> GET_ENCLOSED_ELEMENTS =
- Traverser.forTree(Element::getEnclosedElements);
-
public ImmutableSet<ExecutableElement> getLocalAndInheritedMethods(TypeElement type) {
return getLocalAndInheritedMethodsCache.computeIfAbsent(
type, k -> MoreElements.getLocalAndInheritedMethods(type, types, elements));
@@ -129,22 +129,16 @@
/** Returns the argument or the closest enclosing element that is a {@link TypeElement}. */
public static TypeElement closestEnclosingTypeElement(Element element) {
- return element.accept(CLOSEST_ENCLOSING_TYPE_ELEMENT, null);
+ Element current = element;
+ while (current != null) {
+ if (MoreElements.isType(current)) {
+ return MoreElements.asType(current);
+ }
+ current = current.getEnclosingElement();
+ }
+ throw new IllegalStateException("There is no enclosing TypeElement for: " + element);
}
- private static final ElementVisitor<TypeElement, Void> CLOSEST_ENCLOSING_TYPE_ELEMENT =
- new SimpleElementVisitor8<TypeElement, Void>() {
- @Override
- protected TypeElement defaultAction(Element element, Void p) {
- return element.getEnclosingElement().accept(this, null);
- }
-
- @Override
- public TypeElement visitType(TypeElement type, Void p) {
- return type;
- }
- };
-
/**
* Compares elements according to their declaration order among siblings. Only valid to compare
* elements enclosed by the same parent.
diff --git a/java/dagger/internal/codegen/validation/CompositeBindingGraphPlugin.java b/java/dagger/internal/codegen/validation/CompositeBindingGraphPlugin.java
index dfd93f5..63bf32b 100644
--- a/java/dagger/internal/codegen/validation/CompositeBindingGraphPlugin.java
+++ b/java/dagger/internal/codegen/validation/CompositeBindingGraphPlugin.java
@@ -22,7 +22,7 @@
import static com.google.common.collect.Lists.asList;
import static dagger.internal.codegen.base.ElementFormatter.elementToString;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
-import static dagger.internal.codegen.langmodel.DaggerElements.elementEncloses;
+import static dagger.internal.codegen.langmodel.DaggerElements.transitivelyEncloses;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.FormatMethod;
@@ -216,7 +216,7 @@
String message) {
// TODO(erichang): This repeats some of the logic in DiagnosticReporterImpl. Remove when
// merged.
- if (elementEncloses(
+ if (transitivelyEncloses(
graph.rootComponentNode().componentPath().currentComponent().java(),
childFactoryMethodEdge.factoryMethod().java())) {
// Let this pass through since it is not an error reported on the root component
diff --git a/java/dagger/internal/codegen/validation/DiagnosticReporterFactory.java b/java/dagger/internal/codegen/validation/DiagnosticReporterFactory.java
index 17c7048..a613d18 100644
--- a/java/dagger/internal/codegen/validation/DiagnosticReporterFactory.java
+++ b/java/dagger/internal/codegen/validation/DiagnosticReporterFactory.java
@@ -18,7 +18,7 @@
import static com.google.common.collect.Lists.asList;
import static dagger.internal.codegen.base.ElementFormatter.elementToString;
-import static dagger.internal.codegen.langmodel.DaggerElements.elementEncloses;
+import static dagger.internal.codegen.langmodel.DaggerElements.transitivelyEncloses;
import static javax.tools.Diagnostic.Kind.ERROR;
import com.google.common.collect.ImmutableSet;
@@ -173,9 +173,7 @@
StringBuilder fullMessage = new StringBuilder();
appendBracketPrefix(fullMessage, plugin);
- // TODO(ronshapiro): should we create a HashSet out of elementEncloses() so we don't
- // need to do an O(n) contains() each time?
- if (elementToReport != null && !elementEncloses(rootComponent, elementToReport)) {
+ if (elementToReport != null && !transitivelyEncloses(rootComponent, elementToReport)) {
appendBracketPrefix(fullMessage, elementToString(elementToReport));
elementToReport = rootComponent;
}
diff --git a/java/dagger/internal/codegen/validation/ValidationReport.java b/java/dagger/internal/codegen/validation/ValidationReport.java
index 7f37375..5401dfa 100644
--- a/java/dagger/internal/codegen/validation/ValidationReport.java
+++ b/java/dagger/internal/codegen/validation/ValidationReport.java
@@ -18,6 +18,7 @@
import static dagger.internal.codegen.base.ElementFormatter.elementToString;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
+import static dagger.internal.codegen.langmodel.DaggerElements.transitivelyEncloses;
import static javax.tools.Diagnostic.Kind.ERROR;
import static javax.tools.Diagnostic.Kind.NOTE;
import static javax.tools.Diagnostic.Kind.WARNING;
@@ -103,7 +104,7 @@
}
hasPrintedErrors = true;
for (Item item : items) {
- if (isEnclosedIn(subject, item.element())) {
+ if (transitivelyEncloses(subject, item.element())) {
if (item.annotation().isPresent()) {
if (item.annotationValue().isPresent()) {
messager.printMessage(
@@ -129,17 +130,6 @@
}
}
- private static boolean isEnclosedIn(Element parent, Element child) {
- Element current = child;
- while (current != null) {
- if (current.equals(parent)) {
- return true;
- }
- current = current.getEnclosingElement();
- }
- return false;
- }
-
/** Metadata about a {@link ValidationReport} item. */
@AutoValue
public abstract static class Item {
