Permalink
Browse files

Merge pull request #196 from square/jw/composition

Forbid modules who extend from non-Object class types.
  • Loading branch information...
2 parents 6f2413a + 54079ae commit 079a3dc521c0a5bf3f818933d223ce6a11449062 @JakeWharton JakeWharton committed Mar 27, 2013
@@ -0,0 +1 @@
+invoker.buildResult=failure
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Copyright (C) 2013 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.
+-->
+<project
+ xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>com.example.dagger.tests</groupId>
+ <artifactId>module-type-validation</artifactId>
+ <version>HEAD-SNAPSHOT</version>
+ <dependencies>
+ <dependency>
+ <groupId>@dagger.groupId@</groupId>
+ <artifactId>dagger</artifactId>
+ <version>@dagger.version@</version>
+ </dependency>
+ </dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.0</version>
+ <configuration>
+ <source>1.5</source>
+ <target>1.5</target>
+ <!-- workaround for http://jira.codehaus.org/browse/MCOMPILER-202 -->
+ <forceJavacCompilerUse>true</forceJavacCompilerUse>
+ </configuration>
+ <dependencies>
+ <dependency>
+ <groupId>@dagger.groupId@</groupId>
+ <artifactId>dagger-compiler</artifactId>
+ <version>@dagger.version@</version>
+ </dependency>
+ </dependencies>
+ </plugin>
+ </plugins>
+ </build>
+</project>
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2013 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 test;
+
+import dagger.Module;
+
+@Module
+class ThreadModule extends Thread {
+}
+
+@Module
+enum EnumModule {
+}
+
+@Module
+interface InterfaceModule {
+}
@@ -0,0 +1,10 @@
+import dagger.testing.it.BuildLogValidator;
+import java.io.File;
+
+File buildLog = new File(basedir, "build.log");
+new BuildLogValidator().assertHasText(buildLog, new String[]{
+ "Modules must not extend from other classes: test.ThreadModule"});
+new BuildLogValidator().assertHasText(buildLog, new String[]{
+ "Modules must be classes: test.EnumModule"});
+new BuildLogValidator().assertHasText(buildLog, new String[]{
+ "Modules must be classes: test.InterfaceModule"});
@@ -45,6 +45,7 @@
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror;
+import javax.lang.model.util.Elements;
import javax.tools.Diagnostic;
import javax.tools.JavaFileObject;
@@ -71,7 +72,6 @@
return SourceVersion.latestSupported();
}
- // TODO: include @Provides methods from the superclass
@Override public boolean process(Set<? extends TypeElement> types, RoundEnvironment env) {
remainingTypes.putAll(providerMethodsByClass(env));
for (Iterator<String> i = remainingTypes.keySet().iterator(); i.hasNext();) {
@@ -148,14 +148,27 @@ private void error(String msg, Element element) {
methods.add(providerMethodAsExecutable);
}
+ Elements elementUtils = processingEnv.getElementUtils();
+ TypeMirror objectType = elementUtils.getTypeElement("java.lang.Object").asType();
+
// Catch any stray modules without @Provides since their entry points
// should still be registered and a ModuleAdapter should still be written.
- for (Element type : env.getElementsAnnotatedWith(Module.class)) {
- if (type.getKind().equals(ElementKind.CLASS)) {
- String moduleType = ((TypeElement) type).getQualifiedName().toString();
- if (result.containsKey(moduleType)) continue;
- result.put(moduleType, new ArrayList<ExecutableElement>());
+ for (Element module : env.getElementsAnnotatedWith(Module.class)) {
+ if (!module.getKind().equals(ElementKind.CLASS)) {
+ error("Modules must be classes: " + module, module);
+ continue;
+ }
+
+ TypeElement moduleType = (TypeElement) module;
+
+ // Verify that all modules do not extend from non-Object types.
+ if (!moduleType.getSuperclass().equals(objectType)) {
+ error("Modules must not extend from other classes: " + module, module);
}
+
+ String moduleName = moduleType.getQualifiedName().toString();
+ if (result.containsKey(moduleName)) continue;
+ result.put(moduleName, new ArrayList<ExecutableElement>());
}
return result;
}
@@ -52,6 +52,10 @@
if (annotation == null) {
throw new IllegalArgumentException("No @Module on " + moduleClass.getName());
}
+ if (moduleClass.getSuperclass() != Object.class) {
+ throw new IllegalArgumentException(
+ "Modules must not extend from other classes: " + moduleClass.getName());
+ }
return (ModuleAdapter<T>) new ReflectiveModuleAdapter(moduleClass, annotation);
}
@@ -24,7 +24,7 @@
import static org.junit.Assert.fail;
@RunWith(JUnit4.class)
-public final class ModuleIncludesTest {
+public final class ModuleTest {
static class TestEntryPoint {
@Inject String s;
}
@@ -181,4 +181,16 @@
public void childModuleMissingModuleAnnotation() {
ObjectGraph.create(new ChildModuleMissingModuleAnnotation());
}
+
+ @Module
+ static class ThreadModule extends Thread {}
+
+ @Test public void moduleExtendingClassThrowsException() {
+ try {
+ ObjectGraph.create(new ThreadModule());
+ fail();
+ } catch (IllegalArgumentException e) {
+ assertThat(e.getMessage()).startsWith("Modules must not extend from other classes: ");
+ }
+ }
}

0 comments on commit 079a3dc

Please sign in to comment.