ValidationTool now marks non-public domain methods as errors (issue 7220). Also switch domainMethodWrongModifier error to method level.

Review at: http://gwt-code-reviews.appspot.com/1653803/
Patch by: t.broyer

Review by: jasonhall@google.com

git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10947 8db76d5a-ed1c-0410-87a9-c151d255dfc7
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 5f5839f..8bb1eb5 100644
--- a/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
+++ b/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
@@ -193,8 +193,14 @@
       state.poison(clientMethodElement, Messages.domainMissingMethod(sb));
       return null;
     }
-    
-     /*
+
+    // Check if the method is public
+    if (!domainMethod.getModifiers().contains(Modifier.PUBLIC)) {
+      state.poison(clientMethodElement, Messages.domainMethodNotPublic(
+          domainMethod.getSimpleName()));
+    }
+
+    /*
      * Check the domain method for any requirements for it to be static.
      * InstanceRequests assume instance methods on the domain type.
      */
@@ -203,12 +209,12 @@
 
     if ((isInstanceRequest || requireInstanceDomainMethods)
         && domainMethod.getModifiers().contains(Modifier.STATIC)) {
-      state.poison(checkedElement, Messages.domainMethodWrongModifier(false, domainMethod
+      state.poison(clientMethodElement, Messages.domainMethodWrongModifier(false, domainMethod
           .getSimpleName()));
     }
     if (!isInstanceRequest && requireStaticDomainMethods
         && !domainMethod.getModifiers().contains(Modifier.STATIC)) {
-      state.poison(checkedElement, Messages.domainMethodWrongModifier(true, domainMethod
+      state.poison(clientMethodElement, Messages.domainMethodWrongModifier(true, domainMethod
           .getSimpleName()));
     }
 
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 1379596..e2b8689 100644
--- a/user/src/com/google/web/bindery/requestfactory/apt/Messages.java
+++ b/user/src/com/google/web/bindery/requestfactory/apt/Messages.java
@@ -74,6 +74,10 @@
     return "The domain type's getVersion() method must not be static";
   }
 
+  public static String domainMethodNotPublic(Object domainMethodName) {
+    return String.format("Domain method %s must be public", domainMethodName);
+  }
+
   public static String domainMethodWrongModifier(boolean expectStatic, Object domainMethodName) {
     return String.format("Found %s domain method %s when %s method required", expectStatic
         ? "instance" : "static", domainMethodName, expectStatic ? "static" : "instance");
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 6a1ee98..445beed 100644
--- a/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java
+++ b/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java
@@ -22,7 +22,6 @@
     @Expect(method = "domainGetIdStatic"),
     @Expect(method = "domainGetVersionStatic"),
     @Expect(method = "domainFindNotStatic", args = "Domain"),
-    @Expect(method = "domainMethodWrongModifier", args = {"false", "getFoo"}),
     @Expect(method = "domainNoDefaultConstructor", args = {
         "Domain", "EntityProxyCheckDomainMapping", "RequestContext"}, warning = true)})
 @ProxyFor(EntityProxyCheckDomainMapping.Domain.class)
@@ -47,13 +46,21 @@
     public Domain findDomain(@SuppressWarnings("unused") String id) {
       return null;
     }
+
+    String getNotPublic() {
+      return null;
+    }
   }
 
+  @Expect(method = "domainMethodWrongModifier", args = {"false", "getFoo"})
   String getFoo();
 
   @Expect(method = "domainMissingMethod", args = "java.lang.String getMissingProperty()")
   String getMissingProperty();
 
+  @Expect(method = "domainMethodNotPublic", args = "getNotPublic")
+  String getNotPublic();
+
   @Expected({
       @Expect(method = "methodNoDomainPeer", args = {"java.lang.Object", "false"}, warning = true),
       @Expect(method = "untransportableType", args = "java.lang.Object")})
diff --git a/user/test/com/google/web/bindery/requestfactory/apt/RequestContextWithMismatchedInstance.java b/user/test/com/google/web/bindery/requestfactory/apt/RequestContextWithMismatchedInstance.java
index 26c428d..cffeb65 100644
--- a/user/test/com/google/web/bindery/requestfactory/apt/RequestContextWithMismatchedInstance.java
+++ b/user/test/com/google/web/bindery/requestfactory/apt/RequestContextWithMismatchedInstance.java
@@ -26,9 +26,6 @@
  * Tests Request and InstanceRequest methods bound to methods with the wrong
  * static modifier.
  */
-@Expected({
-    @Expect(method = "domainMethodWrongModifier", args = {"true", "instanceMethod"}),
-    @Expect(method = "domainMethodWrongModifier", args = {"false", "staticMethod"})})
 @Service(RequestContextWithMismatchedInstance.Domain.class)
 interface RequestContextWithMismatchedInstance extends RequestContext {
   static class Domain {
@@ -55,7 +52,9 @@
   interface Proxy extends EntityProxy {
   }
 
+  @Expect(method = "domainMethodWrongModifier", args = {"true", "instanceMethod"})
   Request<Void> instanceMethod();
 
+  @Expect(method = "domainMethodWrongModifier", args = {"false", "staticMethod"})
   InstanceRequest<Proxy, Void> staticMethod();
 }