Reference lint rule numbers, more rules.

When reporting lint errors/warnings, reference the relevant
underlying rule.  Also adds a few more lint rules, and removes a few
aggressive checks.

Change-Id: I1bdadf5fd4df9cd28bb7dfe1c7bb1f9055398315

1 files changed, 162 insertions, 79 deletions

diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py
index 1330c28..fefea6b 100644
--- a/tools/apilint/apilint.py
+++ b/tools/apilint/apilint.py
@@ -194,17 +194,20 @@ def parse_api_file(fn):
 
 
 class Failure():
-    def __init__(self, sig, clazz, detail, error, msg):
+    def __init__(self, sig, clazz, detail, error, rule, msg):
         self.sig = sig
         self.clazz = clazz
         self.detail = detail
         self.error = error
+        self.rule = rule
         self.msg = msg
 
         if error:
-            dump = "%sError:%s %s" % (format(fg=RED, bg=BLACK, bold=True), format(reset=True), msg)
+            self.head = "Error %s" % (rule) if rule else "Error"
+            dump = "%s%s:%s %s" % (format(fg=RED, bg=BLACK, bold=True), self.head, format(reset=True), msg)
         else:
-            dump = "%sWarning:%s %s" % (format(fg=YELLOW, bg=BLACK, bold=True), format(reset=True), msg)
+            self.head = "Warning %s" % (rule) if rule else "Warning"
+            dump = "%s%s:%s %s" % (format(fg=YELLOW, bg=BLACK, bold=True), self.head, format(reset=True), msg)
 
         self.line = clazz.line
         blame = clazz.blame
@@ -226,21 +229,21 @@ class Failure():
 
 failures = {}
 
-def _fail(clazz, detail, error, msg):
+def _fail(clazz, detail, error, rule, msg):
     """Records an API failure to be processed later."""
     global failures
 
     sig = "%s-%s-%s" % (clazz.fullname, repr(detail), msg)
     sig = sig.replace(" deprecated ", " ")
 
-    failures[sig] = Failure(sig, clazz, detail, error, msg)
+    failures[sig] = Failure(sig, clazz, detail, error, rule, msg)
 
 
-def warn(clazz, detail, msg):
-    _fail(clazz, detail, False, msg)
+def warn(clazz, detail, rule, msg):
+    _fail(clazz, detail, False, rule, msg)
 
-def error(clazz, detail, msg):
-    _fail(clazz, detail, True, msg)
+def error(clazz, detail, rule, msg):
+    _fail(clazz, detail, True, rule, msg)
 
 
 def verify_constants(clazz):
@@ -250,13 +253,13 @@ 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, "Constant field names should be FOO_NAME")
+                error(clazz, f, "C2", "Constant field names should be FOO_NAME")
 
 
 def verify_enums(clazz):
     """Enums are bad, mmkay?"""
     if "extends java.lang.Enum" in clazz.raw:
-        error(clazz, None, "Enums are not allowed")
+        error(clazz, None, "F5", "Enums are not allowed")
 
 
 def verify_class_names(clazz):
@@ -266,9 +269,9 @@ 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, "Class name style should be Mtp not MTP")
+        warn(clazz, None, "S1", "Class name style should be Mtp not MTP")
     if re.match("[^A-Z]", clazz.name):
-        error(clazz, None, "Class must start with uppercase char")
+        error(clazz, None, "S1", "Class must start with uppercase char")
 
 
 def verify_method_names(clazz):
@@ -279,9 +282,9 @@ def verify_method_names(clazz):
 
     for m in clazz.methods:
         if re.search("[A-Z]{2,}", m.name) is not None:
-            warn(clazz, m, "Method name style should be getMtu() instead of getMTU()")
+            warn(clazz, m, "S1", "Method name style should be getMtu() instead of getMTU()")
         if re.match("[^a-z]", m.name):
-            error(clazz, m, "Method name must start with lowercase char")
+            error(clazz, m, "S1", "Method name must start with lowercase char")
 
 
 def verify_callbacks(clazz):
@@ -291,17 +294,17 @@ def verify_callbacks(clazz):
     if clazz.fullname == "android.speech.tts.SynthesisCallback": return
 
     if clazz.name.endswith("Callbacks"):
-        error(clazz, None, "Class name must not be plural")
+        error(clazz, None, "L1", "Class name must not be plural")
     if clazz.name.endswith("Observer"):
-        warn(clazz, None, "Class should be named FooCallback")
+        warn(clazz, None, "L1", "Class should be named FooCallback")
 
     if clazz.name.endswith("Callback"):
         if "interface" in clazz.split:
-            error(clazz, None, "Callback must be abstract class to enable extension in future API levels")
+            error(clazz, None, "CL3", "Callback 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):
-                error(clazz, m, "Callback method names must be onFoo() style")
+                error(clazz, m, "L1", "Callback method names must be onFoo() style")
 
 
 def verify_listeners(clazz):
@@ -313,16 +316,16 @@ def verify_listeners(clazz):
 
     if clazz.name.endswith("Listener"):
         if " abstract class " in clazz.raw:
-            error(clazz, None, "Listener should be an interface, otherwise renamed Callback")
+            error(clazz, None, "L1", "Listener should be an interface, otherwise renamed Callback")
 
         for m in clazz.methods:
             if not re.match("on[A-Z][a-z]*", m.name):
-                error(clazz, m, "Listener method names must be onFoo() style")
+                error(clazz, m, "L1", "Listener method names must be onFoo() style")
 
         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, "Single listener method name should match class name")
+                error(clazz, m, "L1", "Single listener method name should match class name")
 
 
 def verify_actions(clazz):
@@ -340,7 +343,7 @@ def verify_actions(clazz):
         if "static" in f.split and "final" in f.split and f.typ == "java.lang.String":
             if "_ACTION" in f.name or "ACTION_" in f.name or ".action." in f.value.lower():
                 if not f.name.startswith("ACTION_"):
-                    error(clazz, f, "Intent action constant name must be ACTION_FOO")
+                    error(clazz, f, "C3", "Intent action constant name must be ACTION_FOO")
                 else:
                     if clazz.fullname == "android.content.Intent":
                         prefix = "android.intent.action"
@@ -352,7 +355,7 @@ def verify_actions(clazz):
                         prefix = clazz.pkg.name + ".action"
                     expected = prefix + "." + f.name[7:]
                     if f.value != expected:
-                        error(clazz, f, "Inconsistent action value; expected %s" % (expected))
+                        error(clazz, f, "C4", "Inconsistent action value; expected %s" % (expected))
 
 
 def verify_extras(clazz):
@@ -372,7 +375,7 @@ def verify_extras(clazz):
         if "static" in f.split and "final" in f.split and f.typ == "java.lang.String":
             if "_EXTRA" in f.name or "EXTRA_" in f.name or ".extra" in f.value.lower():
                 if not f.name.startswith("EXTRA_"):
-                    error(clazz, f, "Intent extra must be EXTRA_FOO")
+                    error(clazz, f, "C3", "Intent extra must be EXTRA_FOO")
                 else:
                     if clazz.pkg.name == "android.content" and clazz.name == "Intent":
                         prefix = "android.intent.extra"
@@ -382,7 +385,7 @@ def verify_extras(clazz):
                         prefix = clazz.pkg.name + ".extra"
                     expected = prefix + "." + f.name[6:]
                     if f.value != expected:
-                        error(clazz, f, "Inconsistent extra value; expected %s" % (expected))
+                        error(clazz, f, "C4", "Inconsistent extra value; expected %s" % (expected))
 
 
 def verify_equals(clazz):
@@ -391,7 +394,7 @@ def verify_equals(clazz):
     eq = "equals" in methods
     hc = "hashCode" in methods
     if eq != hc:
-        error(clazz, None, "Must override both equals and hashCode; missing one")
+        error(clazz, None, "M8", "Must override both equals and hashCode; missing one")
 
 
 def verify_parcelable(clazz):
@@ -402,17 +405,17 @@ def verify_parcelable(clazz):
         describe = [ i for i in clazz.methods if i.name == "describeContents" ]
 
         if len(creator) == 0 or len(write) == 0 or len(describe) == 0:
-            error(clazz, None, "Parcelable requires CREATOR, writeToParcel, and describeContents; missing one")
+            error(clazz, None, "FW3", "Parcelable requires CREATOR, writeToParcel, and describeContents; missing one")
 
 
 def verify_protected(clazz):
     """Verify that no protected methods are allowed."""
     for m in clazz.methods:
         if "protected" in m.split:
-            error(clazz, m, "No protected methods; must be public")
+            error(clazz, m, "M7", "No protected methods; must be public")
     for f in clazz.fields:
         if "protected" in f.split:
-            error(clazz, f, "No protected fields; must be public")
+            error(clazz, f, "M7", "No protected fields; must be public")
 
 
 def verify_fields(clazz):
@@ -442,18 +445,18 @@ def verify_fields(clazz):
             elif clazz.fullname.startswith("android.util.Mutable"):
                 pass
             else:
-                error(clazz, f, "Bare fields must be marked final; consider adding accessors")
+                error(clazz, f, "F2", "Bare fields must be marked final, or add accessors if mutable")
 
         if not "static" in f.split:
             if not re.match("[a-z]([a-zA-Z]+)?", f.name):
-                error(clazz, f, "Non-static fields must be named with myField style")
+                error(clazz, f, "S1", "Non-static fields must be named with myField style")
 
         if re.match("[ms][A-Z]", f.name):
-            error(clazz, f, "Don't expose your internal objects")
+            error(clazz, f, "F1", "Don't expose your internal objects")
 
         if re.match("[A-Z_]+", f.name):
             if "static" not in f.split or "final" not in f.split:
-                error(clazz, f, "Constants must be marked static final")
+                error(clazz, f, "C2", "Constants must be marked static final")
 
 
 def verify_register(clazz):
@@ -466,34 +469,34 @@ def verify_register(clazz):
             if m.name.startswith("register"):
                 other = "unregister" + m.name[8:]
                 if other not in methods:
-                    error(clazz, m, "Missing unregister method")
+                    error(clazz, m, "L2", "Missing unregister method")
             if m.name.startswith("unregister"):
                 other = "register" + m.name[10:]
                 if other not in methods:
-                    error(clazz, m, "Missing register method")
+                    error(clazz, m, "L2", "Missing register method")
 
             if m.name.startswith("add") or m.name.startswith("remove"):
-                error(clazz, m, "Callback methods should be named register/unregister")
+                error(clazz, m, "L3", "Callback methods should be named register/unregister")
 
         if "Listener" in m.raw:
             if m.name.startswith("add"):
                 other = "remove" + m.name[3:]
                 if other not in methods:
-                    error(clazz, m, "Missing remove method")
+                    error(clazz, m, "L2", "Missing remove method")
             if m.name.startswith("remove") and not m.name.startswith("removeAll"):
                 other = "add" + m.name[6:]
                 if other not in methods:
-                    error(clazz, m, "Missing add method")
+                    error(clazz, m, "L2", "Missing add method")
 
             if m.name.startswith("register") or m.name.startswith("unregister"):
-                error(clazz, m, "Listener methods should be named add/remove")
+                error(clazz, m, "L3", "Listener methods should be named add/remove")
 
 
 def verify_sync(clazz):
     """Verify synchronized methods aren't exposed."""
     for m in clazz.methods:
         if "synchronized" in m.split:
-            error(clazz, m, "Internal lock exposed")
+            error(clazz, m, "M5", "Internal lock exposed")
 
 
 def verify_intent_builder(clazz):
@@ -505,7 +508,7 @@ def verify_intent_builder(clazz):
             if m.name.startswith("create") and m.name.endswith("Intent"):
                 pass
             else:
-                error(clazz, m, "Methods creating an Intent should be named createFooIntent()")
+                error(clazz, m, "FW1", "Methods creating an Intent should be named createFooIntent()")
 
 
 def verify_helper_classes(clazz):
@@ -515,57 +518,45 @@ def verify_helper_classes(clazz):
     if "extends android.app.Service" in clazz.raw:
         test_methods = True
         if not clazz.name.endswith("Service"):
-            error(clazz, None, "Inconsistent class name; should be FooService")
+            error(clazz, None, "CL4", "Inconsistent class name; should be FooService")
 
         found = False
         for f in clazz.fields:
             if f.name == "SERVICE_INTERFACE":
                 found = True
                 if f.value != clazz.fullname:
-                    error(clazz, f, "Inconsistent interface constant; expected %s" % (clazz.fullname))
-
-        if not found:
-            warn(clazz, None, "Missing SERVICE_INTERFACE constant")
-
-        if "abstract" in clazz.split and not clazz.fullname.startswith("android.service."):
-            warn(clazz, None, "Services extended by developers should be under android.service")
+                    error(clazz, f, "C4", "Inconsistent interface constant; expected %s" % (clazz.fullname))
 
     if "extends android.content.ContentProvider" in clazz.raw:
         test_methods = True
         if not clazz.name.endswith("Provider"):
-            error(clazz, None, "Inconsistent class name; should be FooProvider")
+            error(clazz, None, "CL4", "Inconsistent class name; should be FooProvider")
 
         found = False
         for f in clazz.fields:
             if f.name == "PROVIDER_INTERFACE":
                 found = True
                 if f.value != clazz.fullname:
-                    error(clazz, f, "Inconsistent interface name; expected %s" % (clazz.fullname))
-
-        if not found:
-            warn(clazz, None, "Missing PROVIDER_INTERFACE constant")
-
-        if "abstract" in clazz.split and not clazz.fullname.startswith("android.provider."):
-            warn(clazz, None, "Providers extended by developers should be under android.provider")
+                    error(clazz, f, "C4", "Inconsistent interface name; expected %s" % (clazz.fullname))
 
     if "extends android.content.BroadcastReceiver" in clazz.raw:
         test_methods = True
         if not clazz.name.endswith("Receiver"):
-            error(clazz, None, "Inconsistent class name; should be FooReceiver")
+            error(clazz, None, "CL4", "Inconsistent class name; should be FooReceiver")
 
     if "extends android.app.Activity" in clazz.raw:
         test_methods = True
         if not clazz.name.endswith("Activity"):
-            error(clazz, None, "Inconsistent class name; should be FooActivity")
+            error(clazz, None, "CL4", "Inconsistent class name; should be FooActivity")
 
     if test_methods:
         for m in clazz.methods:
             if "final" in m.split: continue
             if not re.match("on[A-Z]", m.name):
                 if "abstract" in m.split:
-                    error(clazz, m, "Methods implemented by developers must be named onFoo()")
+                    error(clazz, m, None, "Methods implemented by developers must be named onFoo()")
                 else:
-                    warn(clazz, m, "If implemented by developer, should be named onFoo(); otherwise consider marking final")
+                    warn(clazz, m, None, "If implemented by developer, should be named onFoo(); otherwise consider marking final")
 
 
 def verify_builder(clazz):
@@ -575,7 +566,7 @@ def verify_builder(clazz):
     if not clazz.name.endswith("Builder"): return
 
     if clazz.name != "Builder":
-        warn(clazz, None, "Builder should be defined as inner class")
+        warn(clazz, None, None, "Builder should be defined as inner class")
 
     has_build = False
     for m in clazz.methods:
@@ -587,26 +578,26 @@ def verify_builder(clazz):
         if m.name.startswith("clear"): continue
 
         if m.name.startswith("with"):
-            error(clazz, m, "Builder methods names must follow setFoo() style")
+            error(clazz, m, None, "Builder methods names must follow setFoo() style")
 
         if m.name.startswith("set"):
             if not m.typ.endswith(clazz.fullname):
-                warn(clazz, m, "Methods should return the builder")
+                warn(clazz, m, "M4", "Methods should return the builder")
 
     if not has_build:
-        warn(clazz, None, "Missing build() method")
+        warn(clazz, None, None, "Missing build() method")
 
 
 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, "Exposing raw AIDL interface")
+        error(clazz, None, None, "Exposing raw AIDL interface")
 
 
 def verify_internal(clazz):
     """Catch people exposing internal classes."""
     if clazz.pkg.name.startswith("com.android"):
-        error(clazz, None, "Exposing internal class")
+        error(clazz, None, None, "Exposing internal class")
 
 
 def verify_layering(clazz):
@@ -641,16 +632,16 @@ def verify_layering(clazz):
     for f in clazz.fields:
         ir = rank(f.typ)
         if ir and ir < cr:
-            warn(clazz, f, "Field type violates package layering")
+            warn(clazz, f, "FW6", "Field type violates package layering")
 
     for m in clazz.methods:
         ir = rank(m.typ)
         if ir and ir < cr:
-            warn(clazz, m, "Method return type violates package layering")
+            warn(clazz, m, "FW6", "Method return type violates package layering")
         for arg in m.args:
             ir = rank(arg)
             if ir and ir < cr:
-                warn(clazz, m, "Method argument type violates package layering")
+                warn(clazz, m, "FW6", "Method argument type violates package layering")
 
 
 def verify_boolean(clazz, api):
@@ -672,7 +663,7 @@ def verify_boolean(clazz, api):
             elif builder is not None and setter in builder_methods:
                 pass
             else:
-                warn(clazz, m, "Methods returning boolean should be named isFoo, hasFoo, areFoo")
+                warn(clazz, m, None, "Methods returning boolean should be named isFoo, hasFoo, areFoo")
 
 
 def verify_collections(clazz):
@@ -683,10 +674,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, "Return type is concrete collection; should be interface")
+            error(clazz, m, "CL2", "Return type is concrete collection; should be interface")
         for arg in m.args:
             if arg in bad:
-                error(clazz, m, "Argument is concrete collection; should be interface")
+                error(clazz, m, "CL2", "Argument is concrete collection; should be interface")
 
 
 def verify_flags(clazz):
@@ -701,10 +692,96 @@ def verify_flags(clazz):
 
             scope = f.name[0:f.name.index("FLAG_")]
             if val & known[scope]:
-                warn(clazz, f, "Found overlapping flag constant value")
+                error(clazz, f, "C1", "Found overlapping flag constant value")
             known[scope] |= val
 
 
+def verify_exception(clazz):
+    """Verifies that methods don't throw generic exceptions."""
+    for m in clazz.methods:
+        if "throws java.lang.Exception" in m.raw or "throws java.lang.Throwable" in m.raw or "throws java.lang.Error" in m.raw:
+            error(clazz, m, "S1", "Methods must not throw generic exceptions")
+
+
+def verify_google(clazz):
+    """Verifies that APIs never reference Google."""
+
+    if re.search("google", clazz.raw, re.IGNORECASE):
+        error(clazz, None, None, "Must never reference Google")
+
+    test = []
+    test.extend(clazz.ctors)
+    test.extend(clazz.fields)
+    test.extend(clazz.methods)
+
+    for t in test:
+        if re.search("google", t.raw, re.IGNORECASE):
+            error(clazz, t, None, "Must never reference Google")
+
+
+def verify_bitset(clazz):
+    """Verifies that we avoid using heavy BitSet."""
+
+    for f in clazz.fields:
+        if f.typ == "java.util.BitSet":
+            error(clazz, f, None, "Field type must not be heavy BitSet")
+
+    for m in clazz.methods:
+        if m.typ == "java.util.BitSet":
+            error(clazz, m, None, "Return type must not be heavy BitSet")
+        for arg in m.args:
+            if arg == "java.util.BitSet":
+                error(clazz, m, None, "Argument type must not be heavy BitSet")
+
+
+def verify_manager(clazz):
+    """Verifies that FooManager is only obtained from Context."""
+
+    if not clazz.name.endswith("Manager"): return
+
+    for c in clazz.ctors:
+        error(clazz, c, None, "Managers must always be obtained from Context")
+
+
+def verify_boxed(clazz):
+    """Verifies that methods avoid boxed primitives."""
+
+    boxed = ["java.lang.Number", "java.lang.Byte","java.lang.Double","java.lang.Float","java.lang.Integer","java.lang.Long","java.lang.Short"]
+
+    for c in clazz.ctors:
+        for arg in c.args:
+            if arg in boxed:
+                error(clazz, c, None, "Must avoid boxed primitives")
+
+    for f in clazz.fields:
+        if f.typ in boxed:
+            error(clazz, f, None, "Must avoid boxed primitives")
+
+    for m in clazz.methods:
+        if m.typ in boxed:
+            error(clazz, m, None, "Must avoid boxed primitives")
+        for arg in m.args:
+            if arg in boxed:
+                error(clazz, m, None, "Must avoid boxed primitives")
+
+
+def verify_static_utils(clazz):
+    """Verifies that helper classes can't be constructed."""
+    if clazz.fullname.startswith("android.opengl"): return
+    if re.match("android\.R\.[a-z]+", clazz.fullname): return
+
+    if len(clazz.fields) > 0: return
+    if len(clazz.methods) == 0: return
+
+    for m in clazz.methods:
+        if "static" not in m.split:
+            return
+
+    # At this point, we have no fields, and all methods are static
+    if len(clazz.ctors) > 0:
+        error(clazz, None, None, "Fully-static utility classes must not have constructor")
+
+
 def verify_style(api):
     """Find all style issues in the given API level."""
     global failures
@@ -743,6 +820,12 @@ def verify_style(api):
         verify_boolean(clazz, api)
         verify_collections(clazz)
         verify_flags(clazz)
+        verify_exception(clazz)
+        verify_google(clazz)
+        verify_bitset(clazz)
+        verify_manager(clazz)
+        verify_boxed(clazz)
+        verify_static_utils(clazz)
 
     return failures
 
@@ -781,23 +864,23 @@ def verify_compat(cur, prev):
         prev_clazz = prev[key]
 
         if not class_exists(cur, prev_clazz):
-            error(prev_clazz, None, "Class removed or incompatible change")
+            error(prev_clazz, None, None, "Class removed or incompatible change")
             continue
 
         cur_clazz = cur[key]
 
         for test in prev_clazz.ctors:
             if not ctor_exists(cur, cur_clazz, test):
-                error(prev_clazz, prev_ctor, "Constructor removed or incompatible change")
+                error(prev_clazz, prev_ctor, None, "Constructor removed or incompatible change")
 
         methods = all_methods(prev, prev_clazz)
         for test in methods:
             if not method_exists(cur, cur_clazz, test):
-                error(prev_clazz, test, "Method removed or incompatible change")
+                error(prev_clazz, test, None, "Method removed or incompatible change")
 
         for test in prev_clazz.fields:
             if not field_exists(cur, cur_clazz, test):
-                error(prev_clazz, test, "Field removed or incompatible change")
+                error(prev_clazz, test, None, "Field removed or incompatible change")
 
     return failures