Even more lint tests.

Updated boolean set/get tests to handle isFoo() and hasFoo() style
methods.

When listeners are passed as method argument, they should come near
the end of the argument list.

Verify that resources are named consistently.

Slightly clearer message wording overall.

Change-Id: Id22947bd932d82222ce3e6c6e2012dade348fb56

1 files changed, 103 insertions, 47 deletions

diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py
index b8eaa2d..54525ad 100644
--- a/tools/apilint/apilint.py
+++ b/tools/apilint/apilint.py
@@ -253,7 +253,7 @@ def verify_constants(clazz):
     for f in clazz.fields:
         if "static" in f.split and "final" in f.split:
             if re.match("[A-Z0-9_]+", f.name) is None:
-                error(clazz, f, "C2", "Constant field names should be FOO_NAME")
+                error(clazz, f, "C2", "Constant field names must be FOO_NAME")
 
 
 def verify_enums(clazz):
@@ -269,7 +269,7 @@ def verify_class_names(clazz):
     if re.match("android\.R\.[a-z]+", clazz.fullname): return
 
     if re.search("[A-Z]{2,}", clazz.name) is not None:
-        warn(clazz, None, "S1", "Class name style should be Mtp not MTP")
+        warn(clazz, None, "S1", "Class names with acronyms should be Mtp not MTP")
     if re.match("[^A-Z]", clazz.name):
         error(clazz, None, "S1", "Class must start with uppercase char")
 
@@ -282,7 +282,7 @@ def verify_method_names(clazz):
 
     for m in clazz.methods:
         if re.search("[A-Z]{2,}", m.name) is not None:
-            warn(clazz, m, "S1", "Method name style should be getMtu() instead of getMTU()")
+            warn(clazz, m, "S1", "Method names with acronyms should be getMtu() instead of getMTU()")
         if re.match("[^a-z]", m.name):
             error(clazz, m, "S1", "Method name must start with lowercase char")
 
@@ -294,13 +294,13 @@ def verify_callbacks(clazz):
     if clazz.fullname == "android.speech.tts.SynthesisCallback": return
 
     if clazz.name.endswith("Callbacks"):
-        error(clazz, None, "L1", "Class name must not be plural")
+        error(clazz, None, "L1", "Callback class names should be singular")
     if clazz.name.endswith("Observer"):
         warn(clazz, None, "L1", "Class should be named FooCallback")
 
     if clazz.name.endswith("Callback"):
         if "interface" in clazz.split:
-            error(clazz, None, "CL3", "Callback must be abstract class to enable extension in future API levels")
+            error(clazz, None, "CL3", "Callbacks must be abstract class to enable extension in future API levels")
 
         for m in clazz.methods:
             if not re.match("on[A-Z][a-z]*", m.name):
@@ -316,7 +316,7 @@ def verify_listeners(clazz):
 
     if clazz.name.endswith("Listener"):
         if " abstract class " in clazz.raw:
-            error(clazz, None, "L1", "Listener should be an interface, otherwise renamed Callback")
+            error(clazz, None, "L1", "Listeners should be an interface, or otherwise renamed Callback")
 
         for m in clazz.methods:
             if not re.match("on[A-Z][a-z]*", m.name):
@@ -325,7 +325,7 @@ def verify_listeners(clazz):
         if len(clazz.methods) == 1 and clazz.name.startswith("On"):
             m = clazz.methods[0]
             if (m.name + "Listener").lower() != clazz.name.lower():
-                error(clazz, m, "L1", "Single listener method name should match class name")
+                error(clazz, m, "L1", "Single listener method name must match class name")
 
 
 def verify_actions(clazz):
@@ -412,10 +412,10 @@ def verify_protected(clazz):
     """Verify that no protected methods or fields are allowed."""
     for m in clazz.methods:
         if "protected" in m.split:
-            error(clazz, m, "M7", "No protected methods; must be public")
+            error(clazz, m, "M7", "Protected methods not allowed; must be public")
     for f in clazz.fields:
         if "protected" in f.split:
-            error(clazz, f, "M7", "No protected fields; must be public")
+            error(clazz, f, "M7", "Protected fields not allowed; must be public")
 
 
 def verify_fields(clazz):
@@ -449,10 +449,10 @@ def verify_fields(clazz):
 
         if not "static" in f.split:
             if not re.match("[a-z]([a-zA-Z]+)?", f.name):
-                error(clazz, f, "S1", "Non-static fields must be named with myField style")
+                error(clazz, f, "S1", "Non-static fields must be named using myField style")
 
         if re.match("[ms][A-Z]", f.name):
-            error(clazz, f, "F1", "Don't expose your internal objects")
+            error(clazz, f, "F1", "Internal objects must not be exposed")
 
         if re.match("[A-Z_]+", f.name):
             if "static" not in f.split or "final" not in f.split:
@@ -496,7 +496,7 @@ def verify_sync(clazz):
     """Verify synchronized methods aren't exposed."""
     for m in clazz.methods:
         if "synchronized" in m.split:
-            error(clazz, m, "M5", "Internal lock exposed")
+            error(clazz, m, "M5", "Internal locks must not be exposed")
 
 
 def verify_intent_builder(clazz):
@@ -508,7 +508,7 @@ def verify_intent_builder(clazz):
             if m.name.startswith("create") and m.name.endswith("Intent"):
                 pass
             else:
-                error(clazz, m, "FW1", "Methods creating an Intent should be named createFooIntent()")
+                error(clazz, m, "FW1", "Methods creating an Intent must be named createFooIntent()")
 
 
 def verify_helper_classes(clazz):
@@ -537,7 +537,7 @@ def verify_helper_classes(clazz):
             if f.name == "PROVIDER_INTERFACE":
                 found = True
                 if f.value != clazz.fullname:
-                    error(clazz, f, "C4", "Inconsistent interface name; expected %s" % (clazz.fullname))
+                    error(clazz, f, "C4", "Inconsistent interface constant; expected %s" % (clazz.fullname))
 
     if "extends android.content.BroadcastReceiver" in clazz.raw:
         test_methods = True
@@ -578,11 +578,11 @@ def verify_builder(clazz):
         if m.name.startswith("clear"): continue
 
         if m.name.startswith("with"):
-            warn(clazz, m, None, "Builder methods names should follow setFoo() style")
+            warn(clazz, m, None, "Builder methods names should use setFoo() style")
 
         if m.name.startswith("set"):
             if not m.typ.endswith(clazz.fullname):
-                warn(clazz, m, "M4", "Methods should return the builder")
+                warn(clazz, m, "M4", "Methods must return the builder object")
 
     if not has_build:
         warn(clazz, None, None, "Missing build() method")
@@ -591,13 +591,13 @@ def verify_builder(clazz):
 def verify_aidl(clazz):
     """Catch people exposing raw AIDL."""
     if "extends android.os.Binder" in clazz.raw or "implements android.os.IInterface" in clazz.raw:
-        error(clazz, None, None, "Exposing raw AIDL interface")
+        error(clazz, None, None, "Raw AIDL interfaces must not be exposed")
 
 
 def verify_internal(clazz):
     """Catch people exposing internal classes."""
     if clazz.pkg.name.startswith("com.android"):
-        error(clazz, None, None, "Exposing internal class")
+        error(clazz, None, None, "Internal classes must not be exposed")
 
 
 def verify_layering(clazz):
@@ -645,25 +645,43 @@ def verify_layering(clazz):
 
 
 def verify_boolean(clazz, api):
-    """Catches people returning boolean from getFoo() style methods.
-    Ignores when matching setFoo() is present."""
+    """Verifies that boolean accessors are named correctly.
+    For example, hasFoo() and setHasFoo()."""
 
-    methods = [ m.name for m in clazz.methods ]
+    def is_get(m): return len(m.args) == 0 and m.typ == "boolean"
+    def is_set(m): return len(m.args) == 1 and m.args[0] == "boolean"
+
+    gets = [ m for m in clazz.methods if is_get(m) ]
+    sets = [ m for m in clazz.methods if is_set(m) ]
 
-    builder = clazz.fullname + ".Builder"
-    builder_methods = []
-    if builder in api:
-        builder_methods = [ m.name for m in api[builder].methods ]
+    def error_if_exists(methods, trigger, expected, actual):
+        for m in methods:
+            if m.name == actual:
+                error(clazz, m, "M6", "Symmetric method for %s must be named %s" % (trigger, expected))
 
     for m in clazz.methods:
-        if m.typ == "boolean" and m.name.startswith("get") and m.name != "get" and len(m.args) == 0:
-            setter = "set" + m.name[3:]
-            if setter in methods:
-                pass
-            elif builder is not None and setter in builder_methods:
-                pass
-            else:
-                warn(clazz, m, None, "Methods returning boolean should be named isFoo, hasFoo, areFoo")
+        if is_get(m):
+            if re.match("is[A-Z]", m.name):
+                target = m.name[2:]
+                expected = "setIs" + target
+                error_if_exists(sets, m.name, expected, "setHas" + target)
+            elif re.match("has[A-Z]", m.name):
+                target = m.name[3:]
+                expected = "setHas" + target
+                error_if_exists(sets, m.name, expected, "setIs" + target)
+                error_if_exists(sets, m.name, expected, "set" + target)
+            elif re.match("get[A-Z]", m.name):
+                target = m.name[3:]
+                expected = "set" + target
+                error_if_exists(sets, m.name, expected, "setIs" + target)
+                error_if_exists(sets, m.name, expected, "setHas" + target)
+
+        if is_set(m):
+            if re.match("set[A-Z]", m.name):
+                target = m.name[3:]
+                expected = "get" + target
+                error_if_exists(sets, m.name, expected, "is" + target)
+                error_if_exists(sets, m.name, expected, "has" + target)
 
 
 def verify_collections(clazz):
@@ -674,10 +692,10 @@ def verify_collections(clazz):
            "java.util.HashMap", "java.util.HashSet", "android.util.ArraySet", "android.util.ArrayMap"]
     for m in clazz.methods:
         if m.typ in bad:
-            error(clazz, m, "CL2", "Return type is concrete collection; should be interface")
+            error(clazz, m, "CL2", "Return type is concrete collection; must be higher-level interface")
         for arg in m.args:
             if arg in bad:
-                error(clazz, m, "CL2", "Argument is concrete collection; should be interface")
+                error(clazz, m, "CL2", "Argument is concrete collection; must be higher-level interface")
 
 
 def verify_flags(clazz):
@@ -740,7 +758,7 @@ def verify_manager(clazz):
     if not clazz.name.endswith("Manager"): return
 
     for c in clazz.ctors:
-        error(clazz, c, None, "Managers should always be obtained from Context")
+        error(clazz, c, None, "Managers must always be obtained from Context")
 
 
 def verify_boxed(clazz):
@@ -751,18 +769,18 @@ def verify_boxed(clazz):
     for c in clazz.ctors:
         for arg in c.args:
             if arg in boxed:
-                error(clazz, c, None, "Must avoid boxed primitives")
+                error(clazz, c, "M11", "Must avoid boxed primitives")
 
     for f in clazz.fields:
         if f.typ in boxed:
-            error(clazz, f, None, "Must avoid boxed primitives")
+            error(clazz, f, "M11", "Must avoid boxed primitives")
 
     for m in clazz.methods:
         if m.typ in boxed:
-            error(clazz, m, None, "Must avoid boxed primitives")
+            error(clazz, m, "M11", "Must avoid boxed primitives")
         for arg in m.args:
             if arg in boxed:
-                error(clazz, m, None, "Must avoid boxed primitives")
+                error(clazz, m, "M11", "Must avoid boxed primitives")
 
 
 def verify_static_utils(clazz):
@@ -875,20 +893,56 @@ def verify_callback_handlers(clazz):
             if "android.os.Handler" in m.args:
                 takes_handler = True
         if not takes_handler:
-            error(clazz, f, "L1", "Registration methods should have overload that accepts delivery Handler")
+            warn(clazz, f, "L1", "Registration methods should have overload that accepts delivery Handler")
 
 
 def verify_context_first(clazz):
     """Verifies that methods accepting a Context keep it the first argument."""
-    for m in clazz.ctors:
-        if len(m.args) > 1:
+    examine = clazz.ctors + clazz.methods
+    for m in examine:
+        if len(m.args) > 1 and m.args[0] != "android.content.Context":
             if "android.content.Context" in m.args[1:]:
                 error(clazz, m, "M3", "Context is distinct, so it must be the first argument")
 
-    for m in clazz.methods:
-        if len(m.args) > 1:
-            if "android.content.Context" in m.args[1:]:
-                error(clazz, m, "M3", "Context is distinct, so it must be the first argument")
+
+def verify_listener_last(clazz):
+    """Verifies that methods accepting a Listener or Callback keep them as last arguments."""
+    examine = clazz.ctors + clazz.methods
+    for m in examine:
+        if "Listener" in m.name or "Callback" in m.name: continue
+        found = False
+        for a in m.args:
+            if a.endswith("Callback") or a.endswith("Callbacks") or a.endswith("Listener"):
+                found = True
+            elif found and a != "android.os.Handler":
+                warn(clazz, m, "M3", "Listeners should always be at end of argument list")
+
+
+def verify_resource_names(clazz):
+    """Verifies that resource names have consistent case."""
+    if not re.match("android\.R\.[a-z]+", clazz.fullname): return
+
+    # Resources defined by files are foo_bar_baz
+    if clazz.name in ["anim","animator","color","dimen","drawable","interpolator","layout","transition","menu","mipmap","string","plurals","raw","xml"]:
+        for f in clazz.fields:
+            if re.match("[a-z1-9_]+$", f.name): continue
+            error(clazz, f, None, "Expected resource name in this class to be foo_bar_baz style")
+
+    # Resources defined inside files are fooBarBaz
+    if clazz.name in ["array","attr","id","bool","fraction","integer"]:
+        for f in clazz.fields:
+            if re.match("config_[a-z][a-zA-Z1-9]*$", f.name): continue
+            if re.match("layout_[a-z][a-zA-Z1-9]*$", f.name): continue
+            if re.match("state_[a-z_]*$", f.name): continue
+
+            if re.match("[a-z][a-zA-Z1-9]*$", f.name): continue
+            error(clazz, f, "C7", "Expected resource name in this class to be fooBarBaz style")
+
+    # Styles are FooBar_Baz
+    if clazz.name in ["style"]:
+        for f in clazz.fields:
+            if re.match("[A-Z][A-Za-z1-9]+(_[A-Z][A-Za-z1-9]+?)*$", f.name): continue
+            error(clazz, f, "C7", "Expected resource name in this class to be FooBar_Baz style")
 
 
 def verify_style(api):
@@ -938,6 +992,8 @@ def verify_style(api):
         verify_overload_args(clazz)
         verify_callback_handlers(clazz)
         verify_context_first(clazz)
+        verify_listener_last(clazz)
+        verify_resource_names(clazz)
 
     return failures