From 8f4a753b8e1bf12454060f59807302576f9a16d9 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Thu, 10 Dec 2015 00:25:47 -0500 Subject: [PATCH 1/2] allow use of dollar signs in processed class names --- compiler/pom.xml | 5 + .../internal/codegen/GraphAnalysisLoader.java | 72 +++++++++++- .../codegen/GraphAnalysisLoaderTest.java | 103 ++++++++++++++++++ .../codegen/InjectAdapterGenerationTest.java | 48 +++++++- .../main/java/dagger/internal/Memoizer.java | 4 +- .../dagger/internal/FailoverLoaderTest.java | 6 +- 6 files changed, 228 insertions(+), 10 deletions(-) create mode 100644 compiler/src/test/java/dagger/internal/codegen/GraphAnalysisLoaderTest.java diff --git a/compiler/pom.xml b/compiler/pom.xml index 88100674be8..942afe318e3 100644 --- a/compiler/pom.xml +++ b/compiler/pom.xml @@ -51,6 +51,11 @@ junit test + + org.mockito + mockito-core + test + org.easytesting fest-assert diff --git a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java index c3cd022d978..925eb729b29 100644 --- a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java +++ b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java @@ -15,6 +15,7 @@ */ package dagger.internal.codegen; +import com.google.common.annotations.VisibleForTesting; import dagger.internal.Binding; import dagger.internal.Loader; import dagger.internal.ModuleAdapter; @@ -22,6 +23,7 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.ElementKind; import javax.lang.model.element.TypeElement; +import javax.lang.model.util.Elements; /** * A {@code Binding.Resolver} suitable for tool use at build time. The bindings created by @@ -39,8 +41,7 @@ public GraphAnalysisLoader(ProcessingEnvironment processingEnv) { @Override public Binding getAtInjectBinding( String key, String className, ClassLoader classLoader, boolean mustHaveInjections) { - String sourceClassName = className.replace('$', '.'); - TypeElement type = processingEnv.getElementUtils().getTypeElement(sourceClassName); + TypeElement type = resolveType(processingEnv.getElementUtils(), className); if (type == null) { // We've encountered a type that the compiler can't introspect. If this // causes problems in practice (due to incremental compiles, etc.) we @@ -54,6 +55,73 @@ public GraphAnalysisLoader(ProcessingEnvironment processingEnv) { return GraphAnalysisInjectBinding.create(type, mustHaveInjections); } + /** + * Resolves the given class name into a {@link TypeElement}. The class name is a binary name, but + * {@link Elements#getTypeElement(CharSequence)} wants a canonical name. So this method searches + * the space of possible canonical names, starting with the most likely (since '$' is rarely used + * in canonical class names). + */ + @VisibleForTesting static TypeElement resolveType(Elements elements, String className) { + int index = nextDollar(className, className, 0); + if (index == -1) { + return elements.getTypeElement(className); + } + // have to test various possibilities of replacing '$' with '.' since '.' in a canonical name + // of a nested type is replaced with '$' in the binary name. + StringBuilder sb = new StringBuilder(className); + return resolveType(elements, className, sb, index); + } + + /** + * Recursively explores the space of possible canonical names for a given binary class name. + * + * @param elements used to resolve a name into a {@link TypeElement} + * @param className binary class name + * @param sb the current permutation of canonical name to attempt to resolve + * @param index the index of a {@code '$'} which may be changed to {@code '.'} in a canonical name + */ + private static TypeElement resolveType(Elements elements, String className, StringBuilder sb, + final int index) { + + // We assume '$' should be converted to '.'. So we search for classes with dots first. + sb.setCharAt(index, '.'); + int nextIndex = nextDollar(className, sb, index + 1); + TypeElement type = nextIndex == -1 + ? elements.getTypeElement(sb) + : resolveType(elements, className, sb, nextIndex); + if (type != null) { + return type; + } + + // if not found, change back to dollar and search. + sb.setCharAt(index, '$'); + nextIndex = nextDollar(className, sb, index + 1); + return nextIndex == -1 + ? elements.getTypeElement(sb) + : resolveType(elements, className, sb, nextIndex); + } + + /** + * Finds the next {@code '$'} in a class name which can be changed to a {@code '.'} when computing + * a canonical class name. + */ + private static int nextDollar(String className, CharSequence current, int searchStart) { + while (true) { + int index = className.indexOf('$', searchStart); + if (index == -1) { + return -1; + } + // We'll never have two dots nor will a type name end or begin with dot. So no need to + // consider dollars at the beginning, end, or adjacent to dots. + if (index == 0 || index == className.length() - 1 + || current.charAt(index - 1) == '.' || current.charAt(index + 1) == '.') { + searchStart = index + 1; + continue; + } + return index; + } + } + @Override public ModuleAdapter getModuleAdapter(Class moduleClass) { throw new UnsupportedOperationException(); } diff --git a/compiler/src/test/java/dagger/internal/codegen/GraphAnalysisLoaderTest.java b/compiler/src/test/java/dagger/internal/codegen/GraphAnalysisLoaderTest.java new file mode 100644 index 00000000000..4a258bd5336 --- /dev/null +++ b/compiler/src/test/java/dagger/internal/codegen/GraphAnalysisLoaderTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2015 Square Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package dagger.internal.codegen; + +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.List; +import javax.lang.model.element.TypeElement; +import javax.lang.model.util.Elements; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@RunWith(JUnit4.class) +public class GraphAnalysisLoaderTest { + @Test public void resolveType() { + final List resolveAttempts = new ArrayList(); + Elements elements = mock(Elements.class); + when(elements.getTypeElement(any(CharSequence.class))).then(new Answer() { + @Override public TypeElement answer(InvocationOnMock invocationOnMock) throws Throwable { + resolveAttempts.add(invocationOnMock.getArguments()[0].toString()); + return null; + } + }); + + assertNull(GraphAnalysisLoader.resolveType(elements, "blah.blah.Foo$Bar$Baz")); + List expectedAttempts = ImmutableList.builder() + .add("blah.blah.Foo.Bar.Baz") + .add("blah.blah.Foo.Bar$Baz") + .add("blah.blah.Foo$Bar.Baz") + .add("blah.blah.Foo$Bar$Baz") + .build(); + assertEquals(expectedAttempts, resolveAttempts); + + resolveAttempts.clear(); + assertNull(GraphAnalysisLoader.resolveType(elements, "$$Foo$$Bar$$Baz$$")); + expectedAttempts = ImmutableList.builder() + .add("$.Foo.$Bar.$Baz.$") + .add("$.Foo.$Bar.$Baz$$") + .add("$.Foo.$Bar$.Baz.$") + .add("$.Foo.$Bar$.Baz$$") + .add("$.Foo.$Bar$$Baz.$") + .add("$.Foo.$Bar$$Baz$$") + .add("$.Foo$.Bar.$Baz.$") + .add("$.Foo$.Bar.$Baz$$") + .add("$.Foo$.Bar$.Baz.$") + .add("$.Foo$.Bar$.Baz$$") + .add("$.Foo$.Bar$$Baz.$") + .add("$.Foo$.Bar$$Baz$$") + .add("$.Foo$$Bar.$Baz.$") + .add("$.Foo$$Bar.$Baz$$") + .add("$.Foo$$Bar$.Baz.$") + .add("$.Foo$$Bar$.Baz$$") + .add("$.Foo$$Bar$$Baz.$") + .add("$.Foo$$Bar$$Baz$$") + .add("$$Foo.$Bar.$Baz.$") + .add("$$Foo.$Bar.$Baz$$") + .add("$$Foo.$Bar$.Baz.$") + .add("$$Foo.$Bar$.Baz$$") + .add("$$Foo.$Bar$$Baz.$") + .add("$$Foo.$Bar$$Baz$$") + .add("$$Foo$.Bar.$Baz.$") + .add("$$Foo$.Bar.$Baz$$") + .add("$$Foo$.Bar$.Baz.$") + .add("$$Foo$.Bar$.Baz$$") + .add("$$Foo$.Bar$$Baz.$") + .add("$$Foo$.Bar$$Baz$$") + .add("$$Foo$$Bar.$Baz.$") + .add("$$Foo$$Bar.$Baz$$") + .add("$$Foo$$Bar$.Baz.$") + .add("$$Foo$$Bar$.Baz$$") + .add("$$Foo$$Bar$$Baz.$") + .add("$$Foo$$Bar$$Baz$$") + .build(); + assertEquals(expectedAttempts, resolveAttempts); + + Mockito.validateMockitoUsage(); + } + + +} diff --git a/compiler/src/test/java/dagger/tests/integration/codegen/InjectAdapterGenerationTest.java b/compiler/src/test/java/dagger/tests/integration/codegen/InjectAdapterGenerationTest.java index fb2e566b949..621b7ca0e2b 100644 --- a/compiler/src/test/java/dagger/tests/integration/codegen/InjectAdapterGenerationTest.java +++ b/compiler/src/test/java/dagger/tests/integration/codegen/InjectAdapterGenerationTest.java @@ -35,7 +35,11 @@ public final class InjectAdapterGenerationTest { "import javax.inject.Inject;", "class Basic {", " static class A { @Inject A() { } }", - " @Module(injects = A.class)", + " static class Foo$Bar {", + " @Inject Foo$Bar() { }", + " static class Baz { @Inject Baz() { } }", + " }", + " @Module(injects = { A.class, Foo$Bar.class, Foo$Bar.Baz.class })", " static class AModule { }", "}")); @@ -47,7 +51,8 @@ public final class InjectAdapterGenerationTest { "import java.lang.String;", "public final class Basic$AModule$$ModuleAdapter", " extends ModuleAdapter {", - " private static final String[] INJECTS = {\"members/Basic$A\"};", + " private static final String[] INJECTS = {", + " \"members/Basic$A\", \"members/Basic$Foo$Bar\", \"members/Basic$Foo$Bar$Baz\"};", " private static final Class[] STATIC_INJECTIONS = {};", " private static final Class[] INCLUDES = {};", " public Basic$AModule$$ModuleAdapter() {", @@ -59,7 +64,7 @@ public final class InjectAdapterGenerationTest { " }", "}")); - JavaFileObject expectedInjectAdapter = + JavaFileObject expectedInjectAdapterA = JavaFileObjects.forSourceString("Basic$A$$InjectAdapter", Joiner.on("\n").join( "import dagger.internal.Binding;", "import java.lang.Override;", @@ -75,9 +80,44 @@ public final class InjectAdapterGenerationTest { " }", "}")); + JavaFileObject expectedInjectAdapterFooBar = + JavaFileObjects.forSourceString("Basic$Foo$Bar$$InjectAdapter", Joiner.on("\n").join( + "import dagger.internal.Binding;", + "import java.lang.Override;", + "import javax.inject.Provider;", + "public final class Basic$Foo$Bar$$InjectAdapter", + " extends Binding implements Provider {", + " public Basic$Foo$Bar$$InjectAdapter() {", + " super(\"Basic$Foo$Bar\", \"members/Basic$Foo$Bar\",", + " NOT_SINGLETON, Basic.Foo$Bar.class);", + " }", + " @Override public Basic.Foo$Bar get() {", + " Basic.Foo$Bar result = new Basic.Foo$Bar();", + " return result;", + " }", + "}")); + + JavaFileObject expectedInjectAdapterFooBarBaz = + JavaFileObjects.forSourceString("Basic$Foo$Bar$Baz$$InjectAdapter", Joiner.on("\n").join( + "import dagger.internal.Binding;", + "import java.lang.Override;", + "import javax.inject.Provider;", + "public final class Basic$Foo$Bar$Baz$$InjectAdapter", + " extends Binding implements Provider {", + " public Basic$Foo$Bar$Baz$$InjectAdapter() {", + " super(\"Basic$Foo$Bar$Baz\", \"members/Basic$Foo$Bar$Baz\",", + " NOT_SINGLETON, Basic.Foo$Bar.Baz.class);", + " }", + " @Override public Basic.Foo$Bar.Baz get() {", + " Basic.Foo$Bar.Baz result = new Basic.Foo$Bar.Baz();", + " return result;", + " }", + "}")); + ASSERT.about(javaSource()).that(sourceFile).processedWith(daggerProcessors()) .compilesWithoutError().and() - .generatesSources(expectedModuleAdapter, expectedInjectAdapter); + .generatesSources(expectedModuleAdapter, expectedInjectAdapterA, + expectedInjectAdapterFooBar, expectedInjectAdapterFooBarBaz); } } diff --git a/core/src/main/java/dagger/internal/Memoizer.java b/core/src/main/java/dagger/internal/Memoizer.java index 04cdc104c70..a168aabfc1a 100644 --- a/core/src/main/java/dagger/internal/Memoizer.java +++ b/core/src/main/java/dagger/internal/Memoizer.java @@ -22,7 +22,9 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; /** - * Represents an operation to be + * Represents an operation whose results are memoized. Results returned by invocations of + * {@link #create(Object)} are memoized so that the same object is returned for multiple invocations + * of {@link #get(Object)} for the same key. */ abstract class Memoizer { private final Map map; diff --git a/core/src/test/java/dagger/internal/FailoverLoaderTest.java b/core/src/test/java/dagger/internal/FailoverLoaderTest.java index cb17ef68528..5cde6a66092 100644 --- a/core/src/test/java/dagger/internal/FailoverLoaderTest.java +++ b/core/src/test/java/dagger/internal/FailoverLoaderTest.java @@ -33,7 +33,7 @@ @RunWith(JUnit4.class) public final class FailoverLoaderTest { - @Module(injects = EntryPoint.class) + @Module(injects = Entry$Point.class) static class TestModule { @Provides String aString() { return "a"; } } @@ -45,12 +45,12 @@ static final class TestModule$$ModuleAdapter extends TestingModuleAdapter Date: Thu, 10 Dec 2015 15:28:35 -0500 Subject: [PATCH 2/2] work-around bug in javac in Java 7 --- .../internal/codegen/GraphAnalysisLoader.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java index 925eb729b29..fe81458de39 100644 --- a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java +++ b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisLoader.java @@ -64,7 +64,7 @@ public GraphAnalysisLoader(ProcessingEnvironment processingEnv) { @VisibleForTesting static TypeElement resolveType(Elements elements, String className) { int index = nextDollar(className, className, 0); if (index == -1) { - return elements.getTypeElement(className); + return getTypeElement(elements, className); } // have to test various possibilities of replacing '$' with '.' since '.' in a canonical name // of a nested type is replaced with '$' in the binary name. @@ -87,7 +87,7 @@ private static TypeElement resolveType(Elements elements, String className, Stri sb.setCharAt(index, '.'); int nextIndex = nextDollar(className, sb, index + 1); TypeElement type = nextIndex == -1 - ? elements.getTypeElement(sb) + ? getTypeElement(elements, sb) : resolveType(elements, className, sb, nextIndex); if (type != null) { return type; @@ -97,7 +97,7 @@ private static TypeElement resolveType(Elements elements, String className, Stri sb.setCharAt(index, '$'); nextIndex = nextDollar(className, sb, index + 1); return nextIndex == -1 - ? elements.getTypeElement(sb) + ? getTypeElement(elements, sb) : resolveType(elements, className, sb, nextIndex); } @@ -122,6 +122,18 @@ private static int nextDollar(String className, CharSequence current, int search } } + private static TypeElement getTypeElement(Elements elements, CharSequence className) { + try { + return elements.getTypeElement(className); + } catch (ClassCastException e) { + // work-around issue in javac in Java 7 where querying for non-existent type can + // throw a ClassCastException + // TODO(jh): refer to Oracle Bug ID if/when one is assigned to bug report + // (Review ID: JI-9027367) + return null; + } + } + @Override public ModuleAdapter getModuleAdapter(Class moduleClass) { throw new UnsupportedOperationException(); }