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
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
"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 @@
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 @@
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 @@
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