ValidationTool doesn't flag abstract classes as non-default-instantiable Contributed by tbroyer Fixes issue 7414 Review at: https://gwt-code-reviews.appspot.com/1729805/ git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11051 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/build.xml b/build.xml index ad7b34a..3e9f299 100755 --- a/build.xml +++ b/build.xml
@@ -54,7 +54,6 @@ <target name="dev" description="[subdir] Builds (or runs ${target} if set) all the dev libraries"> <call-subproject subproject="buildtools" subtarget="build" /> <gwt.ant dir="dev" /> - <gwt.ant dir="dev/codeserver" /> </target> <target name="codeserver" description="[subdir] Builds (or runs ${target} if set) only the SuperDevMode">
diff --git a/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java b/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java index 8bb1eb5..4fcec55 100644 --- a/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java +++ b/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
@@ -26,9 +26,11 @@ import java.util.Iterator; import java.util.List; +import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; import javax.lang.model.element.Name; +import javax.lang.model.element.NestingKind; import javax.lang.model.element.TypeElement; import javax.lang.model.type.ExecutableType; import javax.lang.model.type.MirroredTypeException; @@ -242,9 +244,9 @@ // Require domain property methods to be instance methods requireInstanceDomainMethods = true; if (!hasProxyLocator(clientTypeElement, state)) { - // Domain types without a Locator should have a no-arg constructor - if (!hasNoArgConstructor(domainElement)) { - state.warn(clientTypeElement, Messages.domainNoDefaultConstructor(domainElement + // Domain types without a Locator should be default-instantiable + if (!isDefaultInstantiable(domainElement)) { + state.warn(clientTypeElement, Messages.domainNotDefaultInstantiable(domainElement .getSimpleName(), clientTypeElement.getSimpleName(), state.requestContextType .asElement().getSimpleName())); } @@ -349,23 +351,6 @@ return returnType; } - /** - * Looks for a no-arg constructor or no constructors at all. Instance - * initializers are ignored. - */ - private boolean hasNoArgConstructor(TypeElement x) { - List<ExecutableElement> constructors = ElementFilter.constructorsIn(x.getEnclosedElements()); - if (constructors.isEmpty()) { - return true; - } - for (ExecutableElement constructor : constructors) { - if (constructor.getParameters().isEmpty()) { - return true; - } - } - return false; - } - private boolean hasProxyLocator(TypeElement x, State state) { ProxyFor proxyFor = x.getAnnotation(ProxyFor.class); if (proxyFor != null) { @@ -397,4 +382,32 @@ ServiceName serviceName = x.getAnnotation(ServiceName.class); return serviceName != null && !serviceName.locator().isEmpty(); } + + /** + * Looks for a no-arg constructor or no constructors at all. Instance + * initializers are ignored. + */ + private boolean isDefaultInstantiable(TypeElement x) { + if (x.getKind() != ElementKind.CLASS) { + return false; + } + if (x.getModifiers().contains(Modifier.ABSTRACT)) { + return false; + } + if (x.getNestingKind() == NestingKind.ANONYMOUS || x.getNestingKind() == NestingKind.LOCAL + || (x.getNestingKind() == NestingKind.MEMBER && !x.getModifiers().contains(Modifier.STATIC))) { + // anonymous and local shouldn't ever happen, but just in case... + return false; + } + List<ExecutableElement> constructors = ElementFilter.constructorsIn(x.getEnclosedElements()); + if (constructors.isEmpty()) { + return true; + } + for (ExecutableElement constructor : constructors) { + if (constructor.getParameters().isEmpty()) { + return true; + } + } + return false; + } }
diff --git a/user/src/com/google/web/bindery/requestfactory/apt/Messages.java b/user/src/com/google/web/bindery/requestfactory/apt/Messages.java index e2b8689..c925633 100644 --- a/user/src/com/google/web/bindery/requestfactory/apt/Messages.java +++ b/user/src/com/google/web/bindery/requestfactory/apt/Messages.java
@@ -94,13 +94,6 @@ return String.format("Could not find domain method similar to %s", description); } - public static String domainNoDefaultConstructor(Object domainName, Object proxyName, - Object requestContextName) { - return String.format("The domain type %s has no default constructor." - + " Calling %s.create(%s.class) will cause a server error.", domainName, - requestContextName, proxyName); - } - public static String domainNoGetId(Object domainType) { return String.format("Domain type %s does not have a getId() method", domainType); } @@ -109,6 +102,13 @@ return String.format("Domain type %s does not have a getVersion() method", domainType); } + public static String domainNotDefaultInstantiable(Object domainName, Object proxyName, + Object requestContextName) { + return String.format("The domain type %s is not default-instantiable." + + " Calling %s.create(%s.class) will cause a server error.", domainName, + requestContextName, proxyName); + } + public static String factoryMustBeAssignable(Object assignableTo) { return String.format("The return type of this method must return a %s", assignableTo); }
diff --git a/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java b/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java index 445beed..663e33d 100644 --- a/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java +++ b/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java
@@ -17,17 +17,31 @@ import com.google.web.bindery.requestfactory.shared.EntityProxy; import com.google.web.bindery.requestfactory.shared.ProxyFor; +import com.google.web.bindery.requestfactory.shared.ValueProxy; @Expected({ @Expect(method = "domainGetIdStatic"), @Expect(method = "domainGetVersionStatic"), @Expect(method = "domainFindNotStatic", args = "Domain"), - @Expect(method = "domainNoDefaultConstructor", args = { + @Expect(method = "domainNotDefaultInstantiable", args = { "Domain", "EntityProxyCheckDomainMapping", "RequestContext"}, warning = true)}) @ProxyFor(EntityProxyCheckDomainMapping.Domain.class) @SuppressWarnings("requestfactory") interface EntityProxyCheckDomainMapping extends EntityProxy { - public static class Domain { + class Domain { + + public static abstract class Abstract { + } + + public enum Enumeration { + } + + public class Inner { + } + + public interface Interface { + } + public static String getFoo() { return null; } @@ -47,14 +61,58 @@ return null; } + public Abstract getAbstract() { + return null; + } + + public Enumeration getEnum() { + return null; + } + + public Inner getInner() { + return null; + } + + public Interface getInterface() { + return null; + } + String getNotPublic() { return null; } } + @Expect(method = "domainNotDefaultInstantiable", args = { "Inner", "InnerProxy", "RequestContext" }, warning = true) + @ProxyFor(Domain.Inner.class) + interface InnerProxy extends ValueProxy { + } + + @Expect(method = "domainNotDefaultInstantiable", args = { "Abstract", "AbstractProxy", "RequestContext" }, warning = true) + @ProxyFor(Domain.Abstract.class) + interface AbstractProxy extends ValueProxy { + } + + @Expect(method = "domainNotDefaultInstantiable", args = { "Enumeration", "EnumProxy", "RequestContext" }, warning = true) + @ProxyFor(Domain.Enumeration.class) + interface EnumProxy extends ValueProxy { + } + + @Expect(method = "domainNotDefaultInstantiable", args = { "Interface", "InterfaceProxy", "RequestContext" }, warning = true) + @ProxyFor(Domain.Interface.class) + interface InterfaceProxy extends ValueProxy { + } + + AbstractProxy getAbstract(); + + EnumProxy getEnum(); + @Expect(method = "domainMethodWrongModifier", args = {"false", "getFoo"}) String getFoo(); + InnerProxy getInner(); + + InterfaceProxy getInterface(); + @Expect(method = "domainMissingMethod", args = "java.lang.String getMissingProperty()") String getMissingProperty();