diff options
Diffstat (limited to 'tools/apilint/apilint.py')
| -rw-r--r-- | tools/apilint/apilint.py | 260 |
1 files changed, 225 insertions, 35 deletions
diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 77c1c24b17eb..399b0c63e113 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -72,6 +72,9 @@ class Field(): self.ident = self.raw.replace(" deprecated ", " ") + def __hash__(self): + return hash(self.raw) + def __repr__(self): return self.raw @@ -97,9 +100,11 @@ class Method(): self.typ = raw[0] self.name = raw[1] self.args = [] + self.throws = [] + target = self.args for r in raw[2:]: - if r == "throws": break - self.args.append(r) + if r == "throws": target = self.throws + else: target.append(r) # identity for compat purposes ident = self.raw @@ -110,6 +115,9 @@ class Method(): ident = ident[:ident.index(" throws ")] self.ident = ident + def __hash__(self): + return hash(self.raw) + def __repr__(self): return self.raw @@ -145,6 +153,9 @@ class Class(): self.name = self.fullname[self.fullname.rindex(".")+1:] + def __hash__(self): + return hash((self.raw, tuple(self.ctors), tuple(self.fields), tuple(self.methods))) + def __repr__(self): return self.raw @@ -256,6 +267,14 @@ def error(clazz, detail, rule, msg): _fail(clazz, detail, True, rule, msg) +noticed = {} + +def notice(clazz): + global noticed + + noticed[clazz.fullname] = hash(clazz) + + def verify_constants(clazz): """All static final constants must be FOO_NAME style.""" if re.match("android\.R\.[a-z]+", clazz.fullname): return @@ -374,7 +393,7 @@ def verify_actions(clazz): prefix = clazz.pkg.name + ".action" expected = prefix + "." + f.name[7:] if f.value != expected: - error(clazz, f, "C4", "Inconsistent action value; expected %s" % (expected)) + error(clazz, f, "C4", "Inconsistent action value; expected '%s'" % (expected)) def verify_extras(clazz): @@ -404,7 +423,7 @@ def verify_extras(clazz): prefix = clazz.pkg.name + ".extra" expected = prefix + "." + f.name[6:] if f.value != expected: - error(clazz, f, "C4", "Inconsistent extra value; expected %s" % (expected)) + error(clazz, f, "C4", "Inconsistent extra value; expected '%s'" % (expected)) def verify_equals(clazz): @@ -433,6 +452,10 @@ def verify_parcelable(clazz): (" final deprecated class " not in clazz.raw)): error(clazz, None, "FW8", "Parcelable classes must be final") + for c in clazz.ctors: + if c.args == ["android.os.Parcel"]: + error(clazz, c, "FW3", "Parcelable inflation is exposed through CREATOR, not raw constructors") + def verify_protected(clazz): """Verify that no protected methods or fields are allowed.""" @@ -555,7 +578,7 @@ def verify_helper_classes(clazz): if f.name == "SERVICE_INTERFACE": found = True if f.value != clazz.fullname: - error(clazz, f, "C4", "Inconsistent interface constant; expected %s" % (clazz.fullname)) + error(clazz, f, "C4", "Inconsistent interface constant; expected '%s'" % (clazz.fullname)) if "extends android.content.ContentProvider" in clazz.raw: test_methods = True @@ -567,7 +590,7 @@ def verify_helper_classes(clazz): if f.name == "PROVIDER_INTERFACE": found = True if f.value != clazz.fullname: - error(clazz, f, "C4", "Inconsistent interface constant; 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 @@ -747,15 +770,19 @@ def verify_flags(clazz): 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") + for t in m.throws: + if t in ["java.lang.Exception", "java.lang.Throwable", "java.lang.Error"]: + error(clazz, m, "S1", "Methods must not throw generic exceptions") + + if t in ["android.os.RemoteException"]: + if clazz.name == "android.content.ContentProviderClient": continue + if clazz.name == "android.os.Binder": continue + if clazz.name == "android.os.IBinder": continue - if "throws android.os.RemoteException" in m.raw: - if clazz.name == "android.content.ContentProviderClient": continue - if clazz.name == "android.os.Binder": continue - if clazz.name == "android.os.IBinder": continue + error(clazz, m, "FW9", "Methods calling into system server should rethrow RemoteException as RuntimeException") - error(clazz, m, "FW9", "Methods calling into system server should rethrow RemoteException as RuntimeException") + if len(m.args) == 0 and t in ["java.lang.IllegalArgumentException", "java.lang.NullPointerException"]: + warn(clazz, m, "S1", "Methods taking no arguments should throw IllegalStateException") def verify_google(clazz): @@ -910,7 +937,8 @@ def verify_callback_handlers(clazz): found = {} by_name = collections.defaultdict(list) - for m in clazz.methods: + examine = clazz.ctors + clazz.methods + for m in examine: if m.name.startswith("unregister"): continue if m.name.startswith("remove"): continue if re.match("on[A-Z]+", m.name): continue @@ -923,11 +951,14 @@ def verify_callback_handlers(clazz): for f in found.values(): takes_handler = False + takes_exec = False for m in by_name[f.name]: if "android.os.Handler" in m.args: takes_handler = True - if not takes_handler: - warn(clazz, f, "L1", "Registration methods should have overload that accepts delivery Handler") + if "java.util.concurrent.Executor" in m.args: + takes_exec = True + if not takes_exec: + warn(clazz, f, "L1", "Registration methods should have overload that accepts delivery Executor") def verify_context_first(clazz): @@ -951,7 +982,7 @@ def verify_listener_last(clazz): 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": + elif found: warn(clazz, m, "M3", "Listeners should always be at end of argument list") @@ -1058,16 +1089,11 @@ def verify_runtime_exceptions(clazz): "java.nio.BufferOverflowException", ] - test = [] - test.extend(clazz.ctors) - test.extend(clazz.methods) - - for t in test: - if " throws " not in t.raw: continue - throws = t.raw[t.raw.index(" throws "):] - for b in banned: - if b in throws: - error(clazz, t, None, "Methods must not mention RuntimeException subclasses in throws clauses") + examine = clazz.ctors + clazz.methods + for m in examine: + for t in m.throws: + if t in banned: + error(clazz, m, None, "Methods must not mention RuntimeException subclasses in throws clauses") def verify_error(clazz): @@ -1127,8 +1153,149 @@ def verify_closable(clazz): return +def verify_member_name_not_kotlin_keyword(clazz): + """Prevent method names which are keywords in Kotlin.""" + + # https://kotlinlang.org/docs/reference/keyword-reference.html#hard-keywords + # This list does not include Java keywords as those are already impossible to use. + keywords = [ + 'as', + 'fun', + 'in', + 'is', + 'object', + 'typealias', + 'val', + 'var', + 'when', + ] + + for m in clazz.methods: + if m.name in keywords: + error(clazz, m, None, "Method name must not be a Kotlin keyword") + for f in clazz.fields: + if f.name in keywords: + error(clazz, f, None, "Field name must not be a Kotlin keyword") + + +def verify_method_name_not_kotlin_operator(clazz): + """Warn about method names which become operators in Kotlin.""" + + binary = set() + + def unique_binary_op(m, op): + if op in binary: + error(clazz, m, None, "Only one of '{0}' and '{0}Assign' methods should be present for Kotlin".format(op)) + binary.add(op) + + for m in clazz.methods: + if 'static' in m.split: + continue + + # https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators + if m.name in ['unaryPlus', 'unaryMinus', 'not'] and len(m.args) == 0: + warn(clazz, m, None, "Method can be invoked as a unary operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#increments-and-decrements + if m.name in ['inc', 'dec'] and len(m.args) == 0 and m.typ != 'void': + # This only applies if the return type is the same or a subtype of the enclosing class, but we have no + # practical way of checking that relationship here. + warn(clazz, m, None, "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#arithmetic + if m.name in ['plus', 'minus', 'times', 'div', 'rem', 'mod', 'rangeTo'] and len(m.args) == 1: + warn(clazz, m, None, "Method can be invoked as a binary operator from Kotlin") + unique_binary_op(m, m.name) + + # https://kotlinlang.org/docs/reference/operator-overloading.html#in + if m.name == 'contains' and len(m.args) == 1 and m.typ == 'boolean': + warn(clazz, m, None, "Method can be invoked as a 'in' operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#indexed + if (m.name == 'get' and len(m.args) > 0) or (m.name == 'set' and len(m.args) > 1): + warn(clazz, m, None, "Method can be invoked with an indexing operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#invoke + if m.name == 'invoke': + warn(clazz, m, None, "Method can be invoked with function call syntax from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#assignments + if m.name in ['plusAssign', 'minusAssign', 'timesAssign', 'divAssign', 'remAssign', 'modAssign'] \ + and len(m.args) == 1 \ + and m.typ == 'void': + warn(clazz, m, None, "Method can be invoked as a compound assignment operator from Kotlin") + unique_binary_op(m, m.name[:-6]) # Remove 'Assign' suffix + + +def verify_collections_over_arrays(clazz): + """Warn that [] should be Collections.""" + + safe = ["java.lang.String[]","byte[]","short[]","int[]","long[]","float[]","double[]","boolean[]","char[]"] + for m in clazz.methods: + if m.typ.endswith("[]") and m.typ not in safe: + warn(clazz, m, None, "Method should return Collection<> (or subclass) instead of raw array") + for arg in m.args: + if arg.endswith("[]") and arg not in safe: + warn(clazz, m, None, "Method argument should be Collection<> (or subclass) instead of raw array") + + +def verify_user_handle(clazz): + """Methods taking UserHandle should be ForUser or AsUser.""" + if clazz.name.endswith("Listener") or clazz.name.endswith("Callback") or clazz.name.endswith("Callbacks"): return + if clazz.fullname == "android.app.admin.DeviceAdminReceiver": return + if clazz.fullname == "android.content.pm.LauncherApps": return + if clazz.fullname == "android.os.UserHandle": return + if clazz.fullname == "android.os.UserManager": return + + for m in clazz.methods: + if m.name.endswith("AsUser") or m.name.endswith("ForUser"): continue + if re.match("on[A-Z]+", m.name): continue + if "android.os.UserHandle" in m.args: + warn(clazz, m, None, "Method taking UserHandle should be named 'doFooAsUser' or 'queryFooForUser'") + + +def verify_params(clazz): + """Parameter classes should be 'Params'.""" + if clazz.name.endswith("Params"): return + if clazz.fullname == "android.app.ActivityOptions": return + if clazz.fullname == "android.app.BroadcastOptions": return + if clazz.fullname == "android.os.Bundle": return + if clazz.fullname == "android.os.BaseBundle": return + if clazz.fullname == "android.os.PersistableBundle": return + + bad = ["Param","Parameter","Parameters","Args","Arg","Argument","Arguments","Options","Bundle"] + for b in bad: + if clazz.name.endswith(b): + error(clazz, None, None, "Classes holding a set of parameters should be called 'FooParams'") + + +def verify_services(clazz): + """Service name should be FOO_BAR_SERVICE = 'foo_bar'.""" + if clazz.fullname != "android.content.Context": return + + for f in clazz.fields: + if f.typ != "java.lang.String": continue + found = re.match(r"([A-Z_]+)_SERVICE", f.name) + if found: + expected = found.group(1).lower() + if f.value != expected: + error(clazz, f, "C4", "Inconsistent service value; expected '%s'" % (expected)) + + +def verify_tense(clazz): + """Verify tenses of method names.""" + if clazz.fullname.startswith("android.opengl"): return + + for m in clazz.methods: + if m.name.endswith("Enable"): + warn(clazz, m, None, "Unexpected tense; probably meant 'enabled'") + + def examine_clazz(clazz): """Find all style issues in the given class.""" + + notice(clazz) + if clazz.pkg.name.startswith("java"): return if clazz.pkg.name.startswith("junit"): return if clazz.pkg.name.startswith("org.apache"): return @@ -1166,7 +1333,7 @@ def examine_clazz(clazz): verify_manager(clazz) verify_boxed(clazz) verify_static_utils(clazz) - verify_overload_args(clazz) + # verify_overload_args(clazz) verify_callback_handlers(clazz) verify_context_first(clazz) verify_listener_last(clazz) @@ -1178,14 +1345,22 @@ def examine_clazz(clazz): verify_error(clazz) verify_units(clazz) verify_closable(clazz) + verify_member_name_not_kotlin_keyword(clazz) + verify_method_name_not_kotlin_operator(clazz) + verify_collections_over_arrays(clazz) + verify_user_handle(clazz) + verify_params(clazz) + verify_services(clazz) + verify_tense(clazz) def examine_stream(stream): """Find all style issues in the given API stream.""" - global failures + global failures, noticed failures = {} + noticed = {} _parse_stream(stream, examine_clazz) - return failures + return (failures, noticed) def examine_api(api): @@ -1262,6 +1437,8 @@ if __name__ == "__main__": help="Disable terminal colors") parser.add_argument("--allow-google", action='store_const', const=True, help="Allow references to Google") + parser.add_argument("--show-noticed", action='store_const', const=True, + help="Show API changes noticed") args = vars(parser.parse_args()) if args['no_color']: @@ -1274,16 +1451,21 @@ if __name__ == "__main__": previous_file = args['previous.txt'] with current_file as f: - cur_fail = examine_stream(f) + cur_fail, cur_noticed = examine_stream(f) if not previous_file is None: with previous_file as f: - prev_fail = examine_stream(f) + prev_fail, prev_noticed = examine_stream(f) # ignore errors from previous API level for p in prev_fail: if p in cur_fail: del cur_fail[p] + # ignore classes unchanged from previous API level + for k, v in prev_noticed.iteritems(): + if k in cur_noticed and v == cur_noticed[k]: + del cur_noticed[k] + """ # NOTE: disabled because of memory pressure # look for compatibility issues @@ -1295,7 +1477,15 @@ if __name__ == "__main__": print """ - print "%s API style issues %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))) - for f in sorted(cur_fail): - print cur_fail[f] + if args['show_noticed'] and len(cur_noticed) != 0: + print "%s API changes noticed %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))) + for f in sorted(cur_noticed.keys()): + print f print + + if len(cur_fail) != 0: + print "%s API style issues %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))) + for f in sorted(cur_fail): + print cur_fail[f] + print + sys.exit(77) |