From fe5ee6e74b45553e408b1f5c47db773fd971ea7e Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 20 Apr 2018 11:26:16 -0600 Subject: [PATCH 01/13] Extend lint script to emit API statistics. Bug: 77588754 Test: manual Change-Id: I240dba5fae1a8635a4265a1af903517f00dec54c (cherry picked from commit daa7cf3a365c24e3a81d0df178c188b12e6d3a91) --- tools/apilint/apilint.py | 64 ++++++++++++++++++++++++++++++---- tools/apilint/apilint_stats.sh | 7 ++++ 2 files changed, 64 insertions(+), 7 deletions(-) create mode 100755 tools/apilint/apilint_stats.sh diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 70a47cf427553..9db3f0271598a 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -1334,18 +1334,25 @@ def verify_clone(clazz): error(clazz, m, None, "Provide an explicit copy constructor instead of implementing clone()") +def is_interesting(clazz): + """Test if given class is interesting from an Android PoV.""" + + if clazz.pkg.name.startswith("java"): return False + if clazz.pkg.name.startswith("junit"): return False + if clazz.pkg.name.startswith("org.apache"): return False + if clazz.pkg.name.startswith("org.xml"): return False + if clazz.pkg.name.startswith("org.json"): return False + if clazz.pkg.name.startswith("org.w3c"): return False + if clazz.pkg.name.startswith("android.icu."): return False + return True + + 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 - if clazz.pkg.name.startswith("org.xml"): return - if clazz.pkg.name.startswith("org.json"): return - if clazz.pkg.name.startswith("org.w3c"): return - if clazz.pkg.name.startswith("android.icu."): return + if not is_interesting(clazz): return verify_constants(clazz) verify_enums(clazz) @@ -1479,6 +1486,7 @@ def show_deprecations_at_birth(cur, prev): # Remove all existing things so we're left with new for prev_clazz in prev.values(): cur_clazz = cur[prev_clazz.fullname] + if not is_interesting(cur_clazz): continue sigs = { i.ident: i for i in prev_clazz.ctors } cur_clazz.ctors = [ i for i in cur_clazz.ctors if i.ident not in sigs ] @@ -1506,6 +1514,38 @@ def show_deprecations_at_birth(cur, prev): print +def show_stats(cur, prev): + """Show API stats.""" + + stats = collections.defaultdict(int) + for cur_clazz in cur.values(): + if not is_interesting(cur_clazz): continue + + if cur_clazz.fullname not in prev: + stats['new_classes'] += 1 + stats['new_ctors'] += len(cur_clazz.ctors) + stats['new_methods'] += len(cur_clazz.methods) + stats['new_fields'] += len(cur_clazz.fields) + else: + prev_clazz = prev[cur_clazz.fullname] + + sigs = { i.ident: i for i in prev_clazz.ctors } + ctors = len([ i for i in cur_clazz.ctors if i.ident not in sigs ]) + sigs = { i.ident: i for i in prev_clazz.methods } + methods = len([ i for i in cur_clazz.methods if i.ident not in sigs ]) + sigs = { i.ident: i for i in prev_clazz.fields } + fields = len([ i for i in cur_clazz.fields if i.ident not in sigs ]) + + if ctors + methods + fields > 0: + stats['extend_classes'] += 1 + stats['extend_ctors'] += ctors + stats['extend_methods'] += methods + stats['extend_fields'] += fields + + print "#", "".join([ k.ljust(20) for k in sorted(stats.keys()) ]) + print " ", "".join([ str(stats[k]).ljust(20) for k in sorted(stats.keys()) ]) + + if __name__ == "__main__": parser = argparse.ArgumentParser(description="Enforces common Android public API design \ patterns. It ignores lint messages from a previous API level, if provided.") @@ -1520,6 +1560,8 @@ if __name__ == "__main__": help="Show API changes noticed") parser.add_argument("--show-deprecations-at-birth", action='store_const', const=True, help="Show API deprecations at birth") + parser.add_argument("--show-stats", action='store_const', const=True, + help="Show API stats") args = vars(parser.parse_args()) if args['no_color']: @@ -1539,6 +1581,14 @@ if __name__ == "__main__": show_deprecations_at_birth(cur, prev) sys.exit() + if args['show_stats']: + with current_file as f: + cur = _parse_stream(f) + with previous_file as f: + prev = _parse_stream(f) + show_stats(cur, prev) + sys.exit() + with current_file as f: cur_fail, cur_noticed = examine_stream(f) if not previous_file is None: diff --git a/tools/apilint/apilint_stats.sh b/tools/apilint/apilint_stats.sh new file mode 100755 index 0000000000000..052d9a5265fee --- /dev/null +++ b/tools/apilint/apilint_stats.sh @@ -0,0 +1,7 @@ +#!/bin/bash +API=28 +while [ $API -gt 14 ]; do + echo "# Changes in API $((API))" + python tools/apilint/apilint.py --show-stats ../../prebuilts/sdk/$((API))/public/api/android.txt ../../prebuilts/sdk/$((API-1))/public/api/android.txt + let API=API-1 +done From 40d67f4b6dcad3f5728f9b3542e350028bbe8b8f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 17 Jul 2018 13:29:40 -0600 Subject: [PATCH 02/13] Handle new current.txt format. We're starting to see "@interface" show up, so handle them like any other interface. We're also seeing more details argument lists with names and annotations; ignore them for now, since all our existing lint checks work on the "real" data type. Verified that it handles new support library current.txt files without causing any regressions against existing framework current.txt files. Test: manual inspection Bug: 111555356 Change-Id: Id11c3561edd317e4ba1a9b43993fd96d8243e00d (cherry picked from commit bd2611916990b0d18a36483060365207fdd94c13) --- tools/apilint/apilint.py | 43 ++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 9db3f0271598a..018e9c9d5f570 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -69,12 +69,19 @@ class Field(): self.raw = raw.strip(" {;") self.blame = blame + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) + raw = raw.split() self.split = list(raw) for r in ["field", "volatile", "transient", "public", "protected", "static", "final", "deprecated"]: while r in raw: raw.remove(r) + # ignore annotations for now + raw = [ r for r in raw if not r.startswith("@") ] + self.typ = raw[0] self.name = raw[1].strip(";") if len(raw) >= 4 and raw[2] == "=": @@ -97,25 +104,39 @@ class Method(): self.raw = raw.strip(" {;") self.blame = blame - # drop generics for now - raw = re.sub("<.+?>", "", raw) + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) - raw = re.split("[\s(),;]+", raw) + # handle each clause differently + raw_prefix, raw_args, _, raw_throws = re.match(r"(.*?)\((.*?)\)( throws )?(.*?);$", raw).groups() + + # parse prefixes + raw = re.split("[\s]+", raw_prefix) for r in ["", ";"]: while r in raw: raw.remove(r) self.split = list(raw) - for r in ["method", "public", "protected", "static", "final", "deprecated", "abstract", "default"]: + for r in ["method", "public", "protected", "static", "final", "deprecated", "abstract", "default", "operator"]: while r in raw: raw.remove(r) self.typ = raw[0] self.name = raw[1] + + # parse args self.args = [] + for arg in re.split(",\s*", raw_args): + arg = re.split("\s", arg) + # ignore annotations for now + arg = [ a for a in arg if not a.startswith("@") ] + if len(arg[0]) > 0: + self.args.append(arg[0]) + + # parse throws self.throws = [] - target = self.args - for r in raw[2:]: - if r == "throws": target = self.throws - else: target.append(r) + for throw in re.split(",\s*", raw_throws): + self.throws.append(throw) + self.ident = ident(self.raw) def __hash__(self): @@ -135,12 +156,18 @@ class Class(): self.fields = [] self.methods = [] + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) + raw = raw.split() self.split = list(raw) if "class" in raw: self.fullname = raw[raw.index("class")+1] elif "interface" in raw: self.fullname = raw[raw.index("interface")+1] + elif "@interface" in raw: + self.fullname = raw[raw.index("@interface")+1] else: raise ValueError("Funky class type %s" % (self.raw)) From 0a2e15dd6eb16d0796a871d55710048c05c54d2f Mon Sep 17 00:00:00 2001 From: Siyamed Sinir Date: Thu, 13 Sep 2018 16:06:59 -0700 Subject: [PATCH 03/13] Update lint rules for graphics/text package layering Test: N/A Bug: 77347886 Change-Id: I907d99b4a1dee0c69b914e55a033d3d9c4eebe56 (cherry picked from commit e23aeb802409cb337ad66454049c9e3b6af0db9a) --- tools/apilint/apilint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 018e9c9d5f570..934847f6eec20 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -697,8 +697,8 @@ def verify_layering(clazz): "android.provider", ["android.content","android.graphics.drawable"], "android.database", - "android.graphics", "android.text", + "android.graphics", "android.os", "android.util" ] From eff9e228c12e1f75feab9c59a3edf546050cc1bc Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 27 Sep 2018 16:29:25 -0600 Subject: [PATCH 04/13] Moar lint rulez! Bug: 37534642, 116675691, 116798271, 72059458, 111790177 Test: manual Change-Id: Ib079ae580a827f225be08f90dbdddeee7d341c48 (cherry picked from commit daac37f229f84ed844adcf1ffd3432b235524d9b) --- tools/apilint/apilint.py | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 934847f6eec20..91cd1cba964d6 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -492,6 +492,7 @@ def verify_parcelable(clazz): def verify_protected(clazz): """Verify that no protected methods or fields are allowed.""" for m in clazz.methods: + if m.name == "finalize": continue if "protected" in m.split: error(clazz, m, "M7", "Protected methods not allowed; must be public") for f in clazz.fields: @@ -1025,6 +1026,10 @@ def verify_resource_names(clazz): # 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("config_[a-z][a-zA-Z1-9]*$", f.name): continue + if f.name.startswith("config_"): + error(clazz, f, None, "Expected config name to be config_fooBarBaz style") + 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") @@ -1361,6 +1366,60 @@ def verify_clone(clazz): error(clazz, m, None, "Provide an explicit copy constructor instead of implementing clone()") +def verify_pfd(clazz): + """Verify that android APIs use PFD over FD.""" + examine = clazz.ctors + clazz.methods + for m in examine: + if m.typ == "java.io.FileDescriptor": + error(clazz, m, "FW11", "Must use ParcelFileDescriptor") + if m.typ == "int": + if "Fd" in m.name or "FD" in m.name or "FileDescriptor" in m.name: + error(clazz, m, "FW11", "Must use ParcelFileDescriptor") + for arg in m.args: + if arg == "java.io.FileDescriptor": + error(clazz, m, "FW11", "Must use ParcelFileDescriptor") + + for f in clazz.fields: + if f.typ == "java.io.FileDescriptor": + error(clazz, f, "FW11", "Must use ParcelFileDescriptor") + + +def verify_numbers(clazz): + """Discourage small numbers types like short and byte.""" + + discouraged = ["short","byte"] + + for c in clazz.ctors: + for arg in c.args: + if arg in discouraged: + warn(clazz, c, "FW12", "Should avoid odd sized primitives; use int instead") + + for f in clazz.fields: + if f.typ in discouraged: + warn(clazz, f, "FW12", "Should avoid odd sized primitives; use int instead") + + for m in clazz.methods: + if m.typ in discouraged: + warn(clazz, m, "FW12", "Should avoid odd sized primitives; use int instead") + for arg in m.args: + if arg in discouraged: + warn(clazz, m, "FW12", "Should avoid odd sized primitives; use int instead") + + +def verify_singleton(clazz): + """Catch singleton objects with constructors.""" + + singleton = False + for m in clazz.methods: + if m.name.startswith("get") and m.name.endswith("Instance") and " static " in m.raw: + singleton = True + + if singleton: + for c in clazz.ctors: + error(clazz, c, None, "Singleton classes should use getInstance() methods") + + + def is_interesting(clazz): """Test if given class is interesting from an Android PoV.""" @@ -1431,6 +1490,9 @@ def examine_clazz(clazz): verify_tense(clazz) verify_icu(clazz) verify_clone(clazz) + verify_pfd(clazz) + verify_numbers(clazz) + verify_singleton(clazz) def examine_stream(stream): From a8e5df06a010406b20a081c780a6e9854a2bad31 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 27 Nov 2018 17:33:42 -0700 Subject: [PATCH 05/13] Guide towards Context.createPackageContextAsUser(). It's a better alternative that should be used instead of adding new "ForUser" or "AsUser" methods. Bug: 115654727 Test: manual Change-Id: I8742c2ef42d743ef69f8f7a91378f498fdc81e43 (cherry picked from commit 86445841ac90e04941dbc8dad34f2a893a2e0f8b) --- tools/apilint/apilint.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 91cd1cba964d6..cb8fef946baf5 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -1285,10 +1285,19 @@ def verify_user_handle(clazz): 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'") + + has_arg = "android.os.UserHandle" in m.args + has_name = m.name.endswith("AsUser") or m.name.endswith("ForUser") + + if clazz.fullname.endswith("Manager") and has_arg: + warn(clazz, m, None, "When a method overload is needed to target a specific " + "UserHandle, callers should be directed to use " + "Context.createPackageContextAsUser() and re-obtain the relevant " + "Manager, and no new API should be added") + elif has_arg and not has_name: + warn(clazz, m, None, "Method taking UserHandle should be named 'doFooAsUser' " + "or 'queryFooForUser'") def verify_params(clazz): From 6eb57b0f4a51255c656c71337fe41b81ff96a3dd Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Thu, 13 Dec 2018 22:08:29 +0100 Subject: [PATCH 06/13] API Lint: Add support for base current.txt Allows specifying a base current.txt and previous.txt file when linting system-current.txt and test-current.txt to avoid false positive error messages due to public API members not being duplicated in the respective non-public APIs Test: python tools/apilint/apilint.py --base-current=api/current.txt api/system-current.txt Change-Id: I306a99b1423584ef3fcdc9272a83cb5eacc37227 (cherry picked from commit 7690d0d4eea0ffa429351b0b1caa34cdb3e0d37f) --- tools/apilint/apilint.py | 87 ++++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 16 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index cb8fef946baf5..b5a990e6e3cf2 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -183,6 +183,11 @@ class Class(): self.name = self.fullname[self.fullname.rindex(".")+1:] + def merge_from(self, other): + self.ctors.extend(other.ctors) + self.fields.extend(other.fields) + self.methods.extend(other.methods) + def __hash__(self): return hash((self.raw, tuple(self.ctors), tuple(self.fields), tuple(self.methods))) @@ -204,9 +209,28 @@ class Package(): return self.raw -def _parse_stream(f, clazz_cb=None): - line = 0 +def _parse_stream(f, clazz_cb=None, base_f=None): api = {} + + if base_f: + base_classes = _parse_stream_to_generator(base_f) + else: + base_classes = [] + + for clazz in _parse_stream_to_generator(f): + base_class = _parse_to_matching_class(base_classes, clazz) + if base_class: + clazz.merge_from(base_class) + + if clazz_cb: + clazz_cb(clazz) + else: # In callback mode, don't keep track of the full API + api[clazz.fullname] = clazz + + return api + +def _parse_stream_to_generator(f): + line = 0 pkg = None clazz = None blame = None @@ -225,26 +249,41 @@ def _parse_stream(f, clazz_cb=None): if raw.startswith("package"): pkg = Package(line, raw, blame) elif raw.startswith(" ") and raw.endswith("{"): - # When provided with class callback, we treat as incremental - # parse and don't build up entire API - if clazz and clazz_cb: - clazz_cb(clazz) clazz = Class(pkg, line, raw, blame) - if not clazz_cb: - api[clazz.fullname] = clazz elif raw.startswith(" ctor"): clazz.ctors.append(Method(clazz, line, raw, blame)) elif raw.startswith(" method"): clazz.methods.append(Method(clazz, line, raw, blame)) elif raw.startswith(" field"): clazz.fields.append(Field(clazz, line, raw, blame)) + elif raw.startswith(" }") and clazz: + while True: + retry = yield clazz + if not retry: + break + # send() was called, asking us to redeliver clazz on next(). Still need to yield + # a dummy value to the send() first though. + if (yield "Returning clazz on next()"): + raise TypeError("send() must be followed by next(), not send()") - # Handle last trailing class - if clazz and clazz_cb: - clazz_cb(clazz) - return api +def _parse_to_matching_class(classes, needle): + """Takes a classes generator and parses it until it returns the class we're looking for + This relies on classes being sorted by package and class name.""" + + for clazz in classes: + if clazz.pkg.name < needle.pkg.name: + # We haven't reached the right package yet + continue + if clazz.name < needle.name: + # We haven't reached the right class yet + continue + if clazz.fullname == needle.fullname: + return clazz + # We ran past the right class. Send it back into the generator, then report failure. + classes.send(clazz) + return None class Failure(): def __init__(self, sig, clazz, detail, error, rule, msg): @@ -1504,12 +1543,12 @@ def examine_clazz(clazz): verify_singleton(clazz) -def examine_stream(stream): +def examine_stream(stream, base_stream=None): """Find all style issues in the given API stream.""" global failures, noticed failures = {} noticed = {} - _parse_stream(stream, examine_clazz) + _parse_stream(stream, examine_clazz, base_f=base_stream) return (failures, noticed) @@ -1650,6 +1689,12 @@ if __name__ == "__main__": parser.add_argument("current.txt", type=argparse.FileType('r'), help="current.txt") parser.add_argument("previous.txt", nargs='?', type=argparse.FileType('r'), default=None, help="previous.txt") + parser.add_argument("--base-current", nargs='?', type=argparse.FileType('r'), default=None, + help="The base current.txt to use when examining system-current.txt or" + " test-current.txt") + parser.add_argument("--base-previous", nargs='?', type=argparse.FileType('r'), default=None, + help="The base previous.txt to use when examining system-previous.txt or" + " test-previous.txt") parser.add_argument("--no-color", action='store_const', const=True, help="Disable terminal colors") parser.add_argument("--allow-google", action='store_const', const=True, @@ -1669,7 +1714,9 @@ if __name__ == "__main__": ALLOW_GOOGLE = True current_file = args['current.txt'] + base_current_file = args['base_current'] previous_file = args['previous.txt'] + base_previous_file = args['base_previous'] if args['show_deprecations_at_birth']: with current_file as f: @@ -1688,10 +1735,18 @@ if __name__ == "__main__": sys.exit() with current_file as f: - cur_fail, cur_noticed = examine_stream(f) + if base_current_file: + with base_current_file as base_f: + cur_fail, cur_noticed = examine_stream(f, base_f) + else: + cur_fail, cur_noticed = examine_stream(f) if not previous_file is None: with previous_file as f: - prev_fail, prev_noticed = examine_stream(f) + if base_previous_file: + with base_previous_file as base_f: + prev_fail, prev_noticed = examine_stream(f, base_f) + else: + prev_fail, prev_noticed = examine_stream(f) # ignore errors from previous API level for p in prev_fail: From 5ed42b6a2e99ae75177cb5790c908b12c4bc47b9 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Wed, 19 Dec 2018 17:10:22 +0100 Subject: [PATCH 07/13] apilint: Fix API lint issues Fixes a bug where only the name instead of the fully qualified name was considered when looking for a class, which lead to faulty results for inner classes. Test: python tools/apilint/apilint_test.py Change-Id: Ib015669ed3faef21d2bdd16f1e27bc55c8669d70 (cherry picked from commit 2c5cacfd36128f43f5fab4f0665acf69ac049a44) --- tools/apilint/apilint.py | 16 ++++++- tools/apilint/apilint_sha_system.sh | 23 +++++++++ tools/apilint/apilint_test.py | 73 +++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100755 tools/apilint/apilint_sha_system.sh create mode 100644 tools/apilint/apilint_test.py diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index b5a990e6e3cf2..1eec218c7be38 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -267,6 +267,18 @@ def _parse_stream_to_generator(f): raise TypeError("send() must be followed by next(), not send()") +def _retry_iterator(it): + """Wraps an iterator, such that calling send(True) on it will redeliver the same element""" + for e in it: + while True: + retry = yield e + if not retry: + break + # send() was called, asking us to redeliver clazz on next(). Still need to yield + # a dummy value to the send() first though. + if (yield "Returning clazz on next()"): + raise TypeError("send() must be followed by next(), not send()") + def _parse_to_matching_class(classes, needle): """Takes a classes generator and parses it until it returns the class we're looking for @@ -276,8 +288,8 @@ def _parse_to_matching_class(classes, needle): if clazz.pkg.name < needle.pkg.name: # We haven't reached the right package yet continue - if clazz.name < needle.name: - # We haven't reached the right class yet + if clazz.pkg.name == needle.pkg.name and clazz.fullname < needle.fullname: + # We're in the right package, but not the right class yet continue if clazz.fullname == needle.fullname: return clazz diff --git a/tools/apilint/apilint_sha_system.sh b/tools/apilint/apilint_sha_system.sh new file mode 100755 index 0000000000000..8538a3d904f5f --- /dev/null +++ b/tools/apilint/apilint_sha_system.sh @@ -0,0 +1,23 @@ +#!/bin/bash + +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the 'License'); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an 'AS IS' BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +if git show --name-only --pretty=format: $1 | grep api/ > /dev/null; then + python tools/apilint/apilint.py \ + --base-current <(git show $1:api/current.txt) \ + --base-previous <(git show $1^:api/current.txt) \ + <(git show $1:api/system-current.txt) \ + <(git show $1^:api/system-current.txt) +fi diff --git a/tools/apilint/apilint_test.py b/tools/apilint/apilint_test.py new file mode 100644 index 0000000000000..59fc14d97bee4 --- /dev/null +++ b/tools/apilint/apilint_test.py @@ -0,0 +1,73 @@ +#!/usr/bin/env python + +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the 'License'); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an 'AS IS' BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +import apilint + +def cls(pkg, name): + return apilint.Class(apilint.Package(999, "package %s {" % pkg, None), 999, + "public final class %s {" % name, None) + +_ri = apilint._retry_iterator + +c1 = cls("android.app", "ActivityManager") +c2 = cls("android.app", "Notification") +c3 = cls("android.app", "Notification.Action") +c4 = cls("android.graphics", "Bitmap") + +class UtilTests(unittest.TestCase): + def test_retry_iterator(self): + it = apilint._retry_iterator([1, 2, 3, 4]) + self.assertEqual(it.next(), 1) + self.assertEqual(it.next(), 2) + self.assertEqual(it.next(), 3) + it.send("retry") + self.assertEqual(it.next(), 3) + self.assertEqual(it.next(), 4) + with self.assertRaises(StopIteration): + it.next() + + def test_retry_iterator_one(self): + it = apilint._retry_iterator([1]) + self.assertEqual(it.next(), 1) + it.send("retry") + self.assertEqual(it.next(), 1) + with self.assertRaises(StopIteration): + it.next() + + def test_retry_iterator_one(self): + it = apilint._retry_iterator([1]) + self.assertEqual(it.next(), 1) + it.send("retry") + self.assertEqual(it.next(), 1) + with self.assertRaises(StopIteration): + it.next() + + def test_skip_to_matching_class_found(self): + it = _ri([c1, c2, c3, c4]) + self.assertEquals(apilint._parse_to_matching_class(it, c3), + c3) + self.assertEqual(it.next(), c4) + + def test_skip_to_matching_class_not_found(self): + it = _ri([c1, c2, c3, c4]) + self.assertEquals(apilint._parse_to_matching_class(it, cls("android.content", "ContentProvider")), + None) + self.assertEqual(it.next(), c4) + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From 038a02992abc2c6bd36f5461529216d9aef3eeb1 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Wed, 19 Dec 2018 17:11:21 +0100 Subject: [PATCH 08/13] apilint: Fix API lint issues 2/2 Fixes false positives that occur when a class in current.txt is faulty, and an entry for that class is then added to system-current.txt. This was so because when parsing the previous revison's system-current.txt, we did not know about the class and thus didn't look for it in current.txt, and thus never recorded that the error is preexisting. To avoid that, we track all classes in system-current.txt with a matching entry in current.txt in the current revision, and later use that to look up all classes we may have missed when examining the previous revision. Test: python tools/apilint/apilint_test.py Change-Id: Ibe09f1159e351e56b35b8816ce0db760de4ef791 (cherry picked from commit 61e3730bc07e04181a01760d2eb1db834a854683) --- tools/apilint/apilint.py | 87 +++++++++++++++++++++++++---------- tools/apilint/apilint_test.py | 78 ++++++++++++++++++++++++++++++- 2 files changed, 139 insertions(+), 26 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 1eec218c7be38..6476abd8268e6 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -209,24 +209,42 @@ class Package(): return self.raw -def _parse_stream(f, clazz_cb=None, base_f=None): +def _parse_stream(f, clazz_cb=None, base_f=None, out_classes_with_base=None, + in_classes_with_base=[]): api = {} + in_classes_with_base = _retry_iterator(in_classes_with_base) if base_f: - base_classes = _parse_stream_to_generator(base_f) + base_classes = _retry_iterator(_parse_stream_to_generator(base_f)) else: base_classes = [] - for clazz in _parse_stream_to_generator(f): - base_class = _parse_to_matching_class(base_classes, clazz) - if base_class: - clazz.merge_from(base_class) - + def handle_class(clazz): if clazz_cb: clazz_cb(clazz) else: # In callback mode, don't keep track of the full API api[clazz.fullname] = clazz + def handle_missed_classes_with_base(clazz): + for c in _yield_until_matching_class(in_classes_with_base, clazz): + base_class = _skip_to_matching_class(base_classes, c) + if base_class: + handle_class(base_class) + + for clazz in _parse_stream_to_generator(f): + # Before looking at clazz, let's see if there's some classes that were not present, but + # may have an entry in the base stream. + handle_missed_classes_with_base(clazz) + + base_class = _skip_to_matching_class(base_classes, clazz) + if base_class: + clazz.merge_from(base_class) + if out_classes_with_base is not None: + out_classes_with_base.append(clazz) + handle_class(clazz) + + handle_missed_classes_with_base(None) + return api def _parse_stream_to_generator(f): @@ -257,15 +275,7 @@ def _parse_stream_to_generator(f): elif raw.startswith(" field"): clazz.fields.append(Field(clazz, line, raw, blame)) elif raw.startswith(" }") and clazz: - while True: - retry = yield clazz - if not retry: - break - # send() was called, asking us to redeliver clazz on next(). Still need to yield - # a dummy value to the send() first though. - if (yield "Returning clazz on next()"): - raise TypeError("send() must be followed by next(), not send()") - + yield clazz def _retry_iterator(it): """Wraps an iterator, such that calling send(True) on it will redeliver the same element""" @@ -279,8 +289,8 @@ def _retry_iterator(it): if (yield "Returning clazz on next()"): raise TypeError("send() must be followed by next(), not send()") -def _parse_to_matching_class(classes, needle): - """Takes a classes generator and parses it until it returns the class we're looking for +def _skip_to_matching_class(classes, needle): + """Takes a classes iterator and consumes entries until it returns the class we're looking for This relies on classes being sorted by package and class name.""" @@ -297,6 +307,28 @@ def _parse_to_matching_class(classes, needle): classes.send(clazz) return None +def _yield_until_matching_class(classes, needle): + """Takes a class iterator and yields entries it until it reaches the class we're looking for. + + This relies on classes being sorted by package and class name.""" + + for clazz in classes: + if needle is None: + yield clazz + elif clazz.pkg.name < needle.pkg.name: + # We haven't reached the right package yet + yield clazz + elif clazz.pkg.name == needle.pkg.name and clazz.fullname < needle.fullname: + # We're in the right package, but not the right class yet + yield clazz + elif clazz.fullname == needle.fullname: + # Class found, abort. + return + else: + # We ran past the right class. Send it back into the iterator, then abort. + classes.send(clazz) + return + class Failure(): def __init__(self, sig, clazz, detail, error, rule, msg): self.sig = sig @@ -1555,12 +1587,14 @@ def examine_clazz(clazz): verify_singleton(clazz) -def examine_stream(stream, base_stream=None): +def examine_stream(stream, base_stream=None, in_classes_with_base=[], out_classes_with_base=None): """Find all style issues in the given API stream.""" global failures, noticed failures = {} noticed = {} - _parse_stream(stream, examine_clazz, base_f=base_stream) + _parse_stream(stream, examine_clazz, base_f=base_stream, + in_classes_with_base=in_classes_with_base, + out_classes_with_base=out_classes_with_base) return (failures, noticed) @@ -1746,19 +1780,24 @@ if __name__ == "__main__": show_stats(cur, prev) sys.exit() + classes_with_base = [] + with current_file as f: if base_current_file: with base_current_file as base_f: - cur_fail, cur_noticed = examine_stream(f, base_f) + cur_fail, cur_noticed = examine_stream(f, base_f, + out_classes_with_base=classes_with_base) else: - cur_fail, cur_noticed = examine_stream(f) + cur_fail, cur_noticed = examine_stream(f, out_classes_with_base=classes_with_base) + if not previous_file is None: with previous_file as f: if base_previous_file: with base_previous_file as base_f: - prev_fail, prev_noticed = examine_stream(f, base_f) + prev_fail, prev_noticed = examine_stream(f, base_f, + in_classes_with_base=classes_with_base) else: - prev_fail, prev_noticed = examine_stream(f) + prev_fail, prev_noticed = examine_stream(f, in_classes_with_base=classes_with_base) # ignore errors from previous API level for p in prev_fail: diff --git a/tools/apilint/apilint_test.py b/tools/apilint/apilint_test.py index 59fc14d97bee4..ece69a99f5792 100644 --- a/tools/apilint/apilint_test.py +++ b/tools/apilint/apilint_test.py @@ -59,15 +59,89 @@ class UtilTests(unittest.TestCase): def test_skip_to_matching_class_found(self): it = _ri([c1, c2, c3, c4]) - self.assertEquals(apilint._parse_to_matching_class(it, c3), + self.assertEquals(apilint._skip_to_matching_class(it, c3), c3) self.assertEqual(it.next(), c4) def test_skip_to_matching_class_not_found(self): it = _ri([c1, c2, c3, c4]) - self.assertEquals(apilint._parse_to_matching_class(it, cls("android.content", "ContentProvider")), + self.assertEquals(apilint._skip_to_matching_class(it, cls("android.content", "ContentProvider")), None) self.assertEqual(it.next(), c4) + def test_yield_until_matching_class_found(self): + it = _ri([c1, c2, c3, c4]) + self.assertEquals(list(apilint._yield_until_matching_class(it, c3)), + [c1, c2]) + self.assertEqual(it.next(), c4) + + def test_yield_until_matching_class_not_found(self): + it = _ri([c1, c2, c3, c4]) + self.assertEquals(list(apilint._yield_until_matching_class(it, cls("android.content", "ContentProvider"))), + [c1, c2, c3]) + self.assertEqual(it.next(), c4) + + def test_yield_until_matching_class_None(self): + it = _ri([c1, c2, c3, c4]) + self.assertEquals(list(apilint._yield_until_matching_class(it, None)), + [c1, c2, c3, c4]) + + +faulty_current_txt = """ +package android.app { + public final class Activity { + } + + public final class WallpaperColors implements android.os.Parcelable { + ctor public WallpaperColors(android.os.Parcel); + method public int describeContents(); + method public void writeToParcel(android.os.Parcel, int); + field public static final android.os.Parcelable.Creator CREATOR; + } +} +""".split('\n') + +ok_current_txt = """ +package android.app { + public final class Activity { + } + + public final class WallpaperColors implements android.os.Parcelable { + ctor public WallpaperColors(); + method public int describeContents(); + method public void writeToParcel(android.os.Parcel, int); + field public static final android.os.Parcelable.Creator CREATOR; + } +} +""".split('\n') + +system_current_txt = """ +package android.app { + public final class WallpaperColors implements android.os.Parcelable { + method public int getSomething(); + } +} +""".split('\n') + + + +class BaseFileTests(unittest.TestCase): + def test_base_file_avoids_errors(self): + failures, _ = apilint.examine_stream(system_current_txt, ok_current_txt) + self.assertEquals(failures, {}) + + def test_class_with_base_finds_same_errors(self): + failures_with_classes_with_base, _ = apilint.examine_stream("", faulty_current_txt, + in_classes_with_base=[cls("android.app", "WallpaperColors")]) + failures_with_system_txt, _ = apilint.examine_stream(system_current_txt, faulty_current_txt) + + self.assertEquals(failures_with_classes_with_base.keys(), failures_with_system_txt.keys()) + + def test_classes_with_base_is_emited(self): + classes_with_base = [] + _, _ = apilint.examine_stream(system_current_txt, faulty_current_txt, + out_classes_with_base=classes_with_base) + self.assertEquals(map(lambda x: x.fullname, classes_with_base), ["android.app.WallpaperColors"]) + if __name__ == "__main__": unittest.main() \ No newline at end of file From b787c183a2061c4deb0301c970c00c43fe72fed4 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Thu, 3 Jan 2019 18:54:33 +0100 Subject: [PATCH 09/13] ApiLint: Allow parsing 2.0 API signature files Test: tools/apilint/apilint_sha.sh HEAD Test: python tools/apilint/apilint_test.py Change-Id: Id2e1792392b3626746f1ec99f481d0cb27e523a1 (cherry picked from commit d170961b02bf67189ddd14f358c9f263009df786) --- tools/apilint/apilint.py | 500 +++++++++++++++++++++++++++------- tools/apilint/apilint_test.py | 130 ++++++++- 2 files changed, 527 insertions(+), 103 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 6476abd8268e6..847b43bdd5ae2 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -50,45 +50,37 @@ def format(fg=None, bg=None, bright=False, bold=False, dim=False, reset=False): return "\033[%sm" % (";".join(codes)) -def ident(raw): - """Strips superficial signature changes, giving us a strong key that - can be used to identify members across API levels.""" - raw = raw.replace(" deprecated ", " ") - raw = raw.replace(" synchronized ", " ") - raw = raw.replace(" final ", " ") - raw = re.sub("<.+?>", "", raw) - if " throws " in raw: - raw = raw[:raw.index(" throws ")] - return raw - - class Field(): - def __init__(self, clazz, line, raw, blame): + def __init__(self, clazz, line, raw, blame, sig_format = 1): self.clazz = clazz self.line = line self.raw = raw.strip(" {;") self.blame = blame - # drop generics for now; may need multiple passes - raw = re.sub("<[^<]+?>", "", raw) - raw = re.sub("<[^<]+?>", "", raw) + if sig_format == 2: + V2LineParser(raw).parse_into_field(self) + elif sig_format == 1: + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) - raw = raw.split() - self.split = list(raw) + raw = raw.split() + self.split = list(raw) - for r in ["field", "volatile", "transient", "public", "protected", "static", "final", "deprecated"]: - while r in raw: raw.remove(r) + for r in ["field", "volatile", "transient", "public", "protected", "static", "final", "deprecated"]: + while r in raw: raw.remove(r) - # ignore annotations for now - raw = [ r for r in raw if not r.startswith("@") ] + # ignore annotations for now + raw = [ r for r in raw if not r.startswith("@") ] - self.typ = raw[0] - self.name = raw[1].strip(";") - if len(raw) >= 4 and raw[2] == "=": - self.value = raw[3].strip(';"') - else: - self.value = None - self.ident = ident(self.raw) + self.typ = raw[0] + self.name = raw[1].strip(";") + if len(raw) >= 4 and raw[2] == "=": + self.value = raw[3].strip(';"') + else: + self.value = None + + self.ident = "-".join((self.typ, self.name, self.value or "")) def __hash__(self): return hash(self.raw) @@ -96,48 +88,55 @@ class Field(): def __repr__(self): return self.raw - class Method(): - def __init__(self, clazz, line, raw, blame): + def __init__(self, clazz, line, raw, blame, sig_format = 1): self.clazz = clazz self.line = line self.raw = raw.strip(" {;") self.blame = blame - # drop generics for now; may need multiple passes - raw = re.sub("<[^<]+?>", "", raw) - raw = re.sub("<[^<]+?>", "", raw) + if sig_format == 2: + V2LineParser(raw).parse_into_method(self) + elif sig_format == 1: + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) - # handle each clause differently - raw_prefix, raw_args, _, raw_throws = re.match(r"(.*?)\((.*?)\)( throws )?(.*?);$", raw).groups() + # handle each clause differently + raw_prefix, raw_args, _, raw_throws = re.match(r"(.*?)\((.*?)\)( throws )?(.*?);$", raw).groups() - # parse prefixes - raw = re.split("[\s]+", raw_prefix) - for r in ["", ";"]: - while r in raw: raw.remove(r) - self.split = list(raw) + # parse prefixes + raw = re.split("[\s]+", raw_prefix) + for r in ["", ";"]: + while r in raw: raw.remove(r) + self.split = list(raw) - for r in ["method", "public", "protected", "static", "final", "deprecated", "abstract", "default", "operator"]: - while r in raw: raw.remove(r) + for r in ["method", "public", "protected", "static", "final", "deprecated", "abstract", "default", "operator", "synchronized"]: + while r in raw: raw.remove(r) - self.typ = raw[0] - self.name = raw[1] + self.typ = raw[0] + self.name = raw[1] - # parse args - self.args = [] - for arg in re.split(",\s*", raw_args): - arg = re.split("\s", arg) - # ignore annotations for now - arg = [ a for a in arg if not a.startswith("@") ] - if len(arg[0]) > 0: - self.args.append(arg[0]) + # parse args + self.args = [] + for arg in re.split(",\s*", raw_args): + arg = re.split("\s", arg) + # ignore annotations for now + arg = [ a for a in arg if not a.startswith("@") ] + if len(arg[0]) > 0: + self.args.append(arg[0]) - # parse throws - self.throws = [] - for throw in re.split(",\s*", raw_throws): - self.throws.append(throw) + # parse throws + self.throws = [] + for throw in re.split(",\s*", raw_throws): + self.throws.append(throw) + else: + raise ValueError("Unknown signature format: " + sig_format) - self.ident = ident(self.raw) + self.ident = "-".join((self.typ, self.name, "-".join(self.args))) + + def sig_matches(self, typ, name, args): + return typ == self.typ and name == self.name and args == self.args def __hash__(self): return hash(self.raw) @@ -147,7 +146,7 @@ class Method(): class Class(): - def __init__(self, pkg, line, raw, blame): + def __init__(self, pkg, line, raw, blame, sig_format = 1): self.pkg = pkg self.line = line self.raw = raw.strip(" {;") @@ -156,31 +155,44 @@ class Class(): self.fields = [] self.methods = [] - # drop generics for now; may need multiple passes - raw = re.sub("<[^<]+?>", "", raw) - raw = re.sub("<[^<]+?>", "", raw) + if sig_format == 2: + V2LineParser(raw).parse_into_class(self) + elif sig_format == 1: + # drop generics for now; may need multiple passes + raw = re.sub("<[^<]+?>", "", raw) + raw = re.sub("<[^<]+?>", "", raw) - raw = raw.split() - self.split = list(raw) - if "class" in raw: - self.fullname = raw[raw.index("class")+1] - elif "interface" in raw: - self.fullname = raw[raw.index("interface")+1] - elif "@interface" in raw: - self.fullname = raw[raw.index("@interface")+1] - else: - raise ValueError("Funky class type %s" % (self.raw)) + raw = raw.split() + self.split = list(raw) + if "class" in raw: + self.fullname = raw[raw.index("class")+1] + elif "interface" in raw: + self.fullname = raw[raw.index("interface")+1] + elif "@interface" in raw: + self.fullname = raw[raw.index("@interface")+1] + else: + raise ValueError("Funky class type %s" % (self.raw)) - if "extends" in raw: - self.extends = raw[raw.index("extends")+1] - self.extends_path = self.extends.split(".") + if "extends" in raw: + self.extends = raw[raw.index("extends")+1] + else: + self.extends = None + + if "implements" in raw: + self.implements = raw[raw.index("implements")+1] + else: + self.implements = None else: - self.extends = None - self.extends_path = [] + raise ValueError("Unknown signature format: " + sig_format) self.fullname = self.pkg.name + "." + self.fullname self.fullname_path = self.fullname.split(".") + if self.extends is not None: + self.extends_path = self.extends.split(".") + else: + self.extends_path = [] + self.name = self.fullname[self.fullname.rindex(".")+1:] def merge_from(self, other): @@ -208,6 +220,285 @@ class Package(): def __repr__(self): return self.raw +class V2Tokenizer(): + DELIMITER = re.compile(r'\s+|[()@<>;,={}/"]|\[\]') + STRING_SPECIAL = re.compile(r'["\\]') + + def __init__(self, raw): + self.raw = raw + self.current = 0 + + def __iter__(self): + return self + + def next(self): + if self.current >= len(self.raw): + raise StopIteration + + start = self.current + match = V2Tokenizer.DELIMITER.search(self.raw, start) + if match: + match_start = match.start() + if match_start == self.current: + end = match.end() + else: + end = match_start + else: + end = len(self.raw) + + token = self.raw[start:end] + self.current = end + + if token.strip() == "": + return self.next() + + if token == "@" and self.raw[start:start+10] == "@interface": + return token + self.next() + + if token == '/' and self.raw[start:start+2] == "//": + self.current = len(self.raw) + raise StopIteration + + if token == '"': + return token + self.tokenize_string() + + return token + + def tokenize_string(self): + start = self.current + end = len(self.raw) + while start < end: + match = V2Tokenizer.STRING_SPECIAL.search(self.raw, start) + if match: + if match.group() == '"': + end = match.end() + break + elif match.group() == '\\': + # ignore whatever is after the slash + start += 2 + else: + raise ValueError("Unexpected match: `%s`" % (match.group())) + else: + raise ValueError("Unexpected EOF tokenizing string: `%s`" % (self.raw[self.current - 1:],)) + + token = self.raw[self.current:end] + self.current = end + return token + +class V2LineParser(): + MODIFIERS = set("public protected internal private abstract default static final transient volatile synchronized".split()) + JAVA_LANG_TYPES = set("AbstractMethodError AbstractStringBuilder Appendable ArithmeticException ArrayIndexOutOfBoundsException ArrayStoreException AssertionError AutoCloseable Boolean BootstrapMethodError Byte Character CharSequence Class ClassCastException ClassCircularityError ClassFormatError ClassLoader ClassNotFoundException Cloneable CloneNotSupportedException Comparable Compiler Deprecated Double Enum EnumConstantNotPresentException Error Exception ExceptionInInitializerError Float FunctionalInterface IllegalAccessError IllegalAccessException IllegalArgumentException IllegalMonitorStateException IllegalStateException IllegalThreadStateException IncompatibleClassChangeError IndexOutOfBoundsException InheritableThreadLocal InstantiationError InstantiationException Integer InternalError InterruptedException Iterable LinkageError Long Math NegativeArraySizeException NoClassDefFoundError NoSuchFieldError NoSuchFieldException NoSuchMethodError NoSuchMethodException NullPointerException Number NumberFormatException Object OutOfMemoryError Override Package package-info.java Process ProcessBuilder ProcessEnvironment ProcessImpl Readable ReflectiveOperationException Runnable Runtime RuntimeException RuntimePermission SafeVarargs SecurityException SecurityManager Short StackOverflowError StackTraceElement StrictMath String StringBuffer StringBuilder StringIndexOutOfBoundsException SuppressWarnings System Thread ThreadDeath ThreadGroup ThreadLocal Throwable TypeNotPresentException UNIXProcess UnknownError UnsatisfiedLinkError UnsupportedClassVersionError UnsupportedOperationException VerifyError VirtualMachineError Void".split()) + + def __init__(self, raw): + self.tokenized = list(V2Tokenizer(raw)) + self.current = 0 + + def parse_into_method(self, method): + method.split = [] + kind = self.parse_one_of("ctor", "method") + method.split.append(kind) + annotations = self.parse_annotations() + method.split.extend(self.parse_modifiers()) + self.parse_matching_paren("<", ">") + if "@Deprecated" in annotations: + method.split.append("deprecated") + if kind == "ctor": + method.typ = "ctor" + else: + method.typ = self.parse_type() + method.split.append(method.typ) + method.name = self.parse_name() + method.split.append(method.name) + self.parse_token("(") + method.args = self.parse_args() + self.parse_token(")") + method.throws = self.parse_throws() + if "@interface" in method.clazz.split: + self.parse_annotation_default() + self.parse_token(";") + self.parse_eof() + + def parse_into_class(self, clazz): + clazz.split = [] + annotations = self.parse_annotations() + if "@Deprecated" in annotations: + clazz.split.append("deprecated") + clazz.split.extend(self.parse_modifiers()) + kind = self.parse_one_of("class", "interface", "@interface", "enum") + if kind == "enum": + # enums are implicitly final + clazz.split.append("final") + clazz.split.append(kind) + clazz.fullname = self.parse_name() + self.parse_matching_paren("<", ">") + extends = self.parse_extends() + clazz.extends = extends[0] if extends else None + implements = self.parse_implements() + clazz.implements = implements[0] if implements else None + # The checks assume that interfaces are always found in implements, which isn't true for + # subinterfaces. + if not implements and "interface" in clazz.split: + clazz.implements = clazz.extends + self.parse_token("{") + self.parse_eof() + + def parse_into_field(self, field): + kind = self.parse_token("field") + field.split = [kind] + annotations = self.parse_annotations() + if "@Deprecated" in annotations: + field.split.append("deprecated") + field.split.extend(self.parse_modifiers()) + field.typ = self.parse_type() + field.split.append(field.typ) + field.name = self.parse_name() + field.split.append(field.name) + if self.parse_if("="): + field.value = self.parse_value_stripped() + else: + field.value = None + + self.parse_token(";") + self.parse_eof() + + def lookahead(self): + return self.tokenized[self.current] + + def parse_one_of(self, *options): + found = self.lookahead() + if found not in options: + raise ValueError("Parsing failed, expected one of `%s` but found `%s` in %s" % (options, found, repr(self.tokenized))) + return self.parse_token() + + def parse_token(self, tok = None): + found = self.lookahead() + if tok is not None and found != tok: + raise ValueError("Parsing failed, expected `%s` but found `%s` in %s" % (tok, found, repr(self.tokenized))) + self.current += 1 + return found + + def eof(self): + return self.current == len(self.tokenized) + + def parse_eof(self): + if not self.eof(): + raise ValueError("Parsing failed, expected EOF, but %s has not been parsed in %s" % (self.tokenized[self.current:], self.tokenized)) + + def parse_if(self, tok): + if not self.eof() and self.lookahead() == tok: + self.parse_token() + return True + return False + + def parse_annotations(self): + ret = [] + while self.lookahead() == "@": + ret.append(self.parse_annotation()) + return ret + + def parse_annotation(self): + ret = self.parse_token("@") + self.parse_token() + self.parse_matching_paren("(", ")") + return ret + + def parse_matching_paren(self, open, close): + start = self.current + if not self.parse_if(open): + return + length = len(self.tokenized) + count = 1 + while count > 0: + if self.current == length: + raise ValueError("Unexpected EOF looking for closing paren: `%s`" % (self.tokenized[start:],)) + t = self.parse_token() + if t == open: + count += 1 + elif t == close: + count -= 1 + return self.tokenized[start:self.current] + + def parse_modifiers(self): + ret = [] + while self.lookahead() in V2LineParser.MODIFIERS: + ret.append(self.parse_token()) + return ret + + def parse_type(self): + type = self.parse_token() + if type in V2LineParser.JAVA_LANG_TYPES: + type = "java.lang." + type + self.parse_matching_paren("<", ">") + while self.parse_if("[]"): + type += "[]" + return type + + def parse_arg_type(self): + type = self.parse_type() + if self.parse_if("..."): + type += "..." + return type + + def parse_name(self): + return self.parse_token() + + def parse_args(self): + args = [] + if self.lookahead() == ")": + return args + + while True: + args.append(self.parse_arg()) + if self.lookahead() == ")": + return args + self.parse_token(",") + + def parse_arg(self): + self.parse_annotations() + return self.parse_arg_type() + + def parse_throws(self): + ret = [] + if self.parse_if("throws"): + ret.append(self.parse_type()) + while self.parse_if(","): + ret.append(self.parse_type()) + return ret + + def parse_extends(self): + if self.parse_if("extends"): + return self.parse_space_delimited_type_list() + return [] + + def parse_implements(self): + if self.parse_if("implements"): + return self.parse_space_delimited_type_list() + return [] + + def parse_space_delimited_type_list(self, terminals = ["implements", "{"]): + types = [] + while True: + types.append(self.parse_type()) + if self.lookahead() in terminals: + return types + + def parse_annotation_default(self): + if self.parse_if("default"): + self.parse_value() + + def parse_value(self): + if self.lookahead() == "{": + return " ".join(self.parse_matching_paren("{", "}")) + elif self.lookahead() == "(": + return " ".join(self.parse_matching_paren("(", ")")) + else: + return self.parse_token() + + def parse_value_stripped(self): + value = self.parse_value() + if value[0] == '"': + return value[1:-1] + return value + def _parse_stream(f, clazz_cb=None, base_f=None, out_classes_with_base=None, in_classes_with_base=[]): @@ -252,6 +543,7 @@ def _parse_stream_to_generator(f): pkg = None clazz = None blame = None + sig_format = 1 re_blame = re.compile("^([a-z0-9]{7,}) \(<([^>]+)>.+?\) (.+?)$") for raw in f: @@ -264,16 +556,18 @@ def _parse_stream_to_generator(f): else: blame = None - if raw.startswith("package"): + if line == 1 and raw == "// Signature format: 2.0": + sig_format = 2 + elif raw.startswith("package"): pkg = Package(line, raw, blame) elif raw.startswith(" ") and raw.endswith("{"): - clazz = Class(pkg, line, raw, blame) + clazz = Class(pkg, line, raw, blame, sig_format=sig_format) elif raw.startswith(" ctor"): - clazz.ctors.append(Method(clazz, line, raw, blame)) + clazz.ctors.append(Method(clazz, line, raw, blame, sig_format=sig_format)) elif raw.startswith(" method"): - clazz.methods.append(Method(clazz, line, raw, blame)) + clazz.methods.append(Method(clazz, line, raw, blame, sig_format=sig_format)) elif raw.startswith(" field"): - clazz.fields.append(Field(clazz, line, raw, blame)) + clazz.fields.append(Field(clazz, line, raw, blame, sig_format=sig_format)) elif raw.startswith(" }") and clazz: yield clazz @@ -367,7 +661,7 @@ 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 = "%s-%s-%s" % (clazz.fullname, detail.ident if detail else None, msg) sig = sig.replace(" deprecated ", " ") failures[sig] = Failure(sig, clazz, detail, error, rule, msg) @@ -408,7 +702,7 @@ def verify_constants(clazz): def verify_enums(clazz): """Enums are bad, mmkay?""" - if "extends java.lang.Enum" in clazz.raw: + if clazz.extends == "java.lang.Enum" or "enum" in clazz.split: error(clazz, None, "F5", "Enums are not allowed") @@ -467,7 +761,7 @@ def verify_listeners(clazz): interface OnFooListener { void onFoo() }""" if clazz.name.endswith("Listener"): - if " abstract class " in clazz.raw: + if "abstract" in clazz.split and "class" in clazz.split: error(clazz, None, "L1", "Listeners should be an interface, or otherwise renamed Callback") for m in clazz.methods: @@ -546,16 +840,16 @@ def verify_equals(clazz): eq = False hc = False for m in clazz.methods: - if " static " in m.raw: continue - if "boolean equals(java.lang.Object)" in m.raw: eq = True - if "int hashCode()" in m.raw: hc = True + if "static" in m.split: continue + if m.sig_matches("boolean", "equals", ["java.lang.Object"]): eq = True + if m.sig_matches("int", "hashCode", []): hc = True if eq != hc: error(clazz, None, "M8", "Must override both equals and hashCode; missing one") def verify_parcelable(clazz): """Verify that Parcelable objects aren't hiding required bits.""" - if "implements android.os.Parcelable" in clazz.raw: + if clazz.implements == "android.os.Parcelable": creator = [ i for i in clazz.fields if i.name == "CREATOR" ] write = [ i for i in clazz.methods if i.name == "writeToParcel" ] describe = [ i for i in clazz.methods if i.name == "describeContents" ] @@ -563,8 +857,7 @@ def verify_parcelable(clazz): if len(creator) == 0 or len(write) == 0 or len(describe) == 0: error(clazz, None, "FW3", "Parcelable requires CREATOR, writeToParcel, and describeContents; missing one") - if ((" final class " not in clazz.raw) and - (" final deprecated class " not in clazz.raw)): + if "final" not in clazz.split: error(clazz, None, "FW8", "Parcelable classes must be final") for c in clazz.ctors: @@ -684,7 +977,7 @@ def verify_helper_classes(clazz): """Verify that helper classes are named consistently with what they extend. All developer extendable methods should be named onFoo().""" test_methods = False - if "extends android.app.Service" in clazz.raw: + if clazz.extends == "android.app.Service": test_methods = True if not clazz.name.endswith("Service"): error(clazz, None, "CL4", "Inconsistent class name; should be FooService") @@ -696,7 +989,7 @@ def verify_helper_classes(clazz): if f.value != clazz.fullname: error(clazz, f, "C4", "Inconsistent interface constant; expected '%s'" % (clazz.fullname)) - if "extends android.content.ContentProvider" in clazz.raw: + if clazz.extends == "android.content.ContentProvider": test_methods = True if not clazz.name.endswith("Provider"): error(clazz, None, "CL4", "Inconsistent class name; should be FooProvider") @@ -708,12 +1001,12 @@ def verify_helper_classes(clazz): if f.value != clazz.fullname: error(clazz, f, "C4", "Inconsistent interface constant; expected '%s'" % (clazz.fullname)) - if "extends android.content.BroadcastReceiver" in clazz.raw: + if clazz.extends == "android.content.BroadcastReceiver": test_methods = True if not clazz.name.endswith("Receiver"): error(clazz, None, "CL4", "Inconsistent class name; should be FooReceiver") - if "extends android.app.Activity" in clazz.raw: + if clazz.extends == "android.app.Activity": test_methods = True if not clazz.name.endswith("Activity"): error(clazz, None, "CL4", "Inconsistent class name; should be FooActivity") @@ -731,7 +1024,7 @@ def verify_helper_classes(clazz): def verify_builder(clazz): """Verify builder classes. Methods should return the builder to enable chaining.""" - if " extends " in clazz.raw: return + if clazz.extends: return if not clazz.name.endswith("Builder"): return if clazz.name != "Builder": @@ -759,7 +1052,7 @@ 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: + if clazz.extends == "android.os.Binder" or clazz.implements == "android.os.IInterface": error(clazz, None, None, "Raw AIDL interfaces must not be exposed") @@ -1168,7 +1461,7 @@ def verify_abstract_inner(clazz): """Verifies that abstract inner classes are static.""" if re.match(".+?\.[A-Z][^\.]+\.[A-Z]", clazz.fullname): - if " abstract " in clazz.raw and " static " not in clazz.raw: + if "abstract" in clazz.split and "static" not in clazz.split: warn(clazz, None, None, "Abstract inner classes should be static to improve testability") @@ -1263,8 +1556,8 @@ def verify_units(clazz): def verify_closable(clazz): """Verifies that classes are AutoClosable.""" - if "implements java.lang.AutoCloseable" in clazz.raw: return - if "implements java.io.Closeable" in clazz.raw: return + if clazz.implements == "java.lang.AutoCloseable": return + if clazz.implements == "java.io.Closeable": return for m in clazz.methods: if len(m.args) > 0: continue @@ -1350,6 +1643,9 @@ def verify_method_name_not_kotlin_operator(clazz): def verify_collections_over_arrays(clazz): """Warn that [] should be Collections.""" + if "@interface" in clazz.split: + return + 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: @@ -1683,11 +1979,11 @@ def show_deprecations_at_birth(cur, prev): del cur[prev_clazz.fullname] for clazz in cur.values(): - if " deprecated " in clazz.raw and not clazz.fullname in prev: + if "deprecated" in clazz.split and not clazz.fullname in prev: error(clazz, None, None, "Found API deprecation at birth") for i in clazz.ctors + clazz.methods + clazz.fields: - if " deprecated " in i.raw: + if "deprecated" in i.split: error(clazz, i, None, "Found API deprecation at birth") print "%s Deprecated at birth %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), diff --git a/tools/apilint/apilint_test.py b/tools/apilint/apilint_test.py index ece69a99f5792..4e3c0349c0bc7 100644 --- a/tools/apilint/apilint_test.py +++ b/tools/apilint/apilint_test.py @@ -143,5 +143,133 @@ class BaseFileTests(unittest.TestCase): out_classes_with_base=classes_with_base) self.assertEquals(map(lambda x: x.fullname, classes_with_base), ["android.app.WallpaperColors"]) +class V2TokenizerTests(unittest.TestCase): + def _test(self, raw, expected): + self.assertEquals(apilint.V2Tokenizer(raw).tokenize(), expected) + + def test_simple(self): + self._test(" method public some.Type someName(some.Argument arg, int arg);", + ['method', 'public', 'some.Type', 'someName', '(', 'some.Argument', + 'arg', ',', 'int', 'arg', ')', ';']) + self._test("class Some.Class extends SomeOther {", + ['class', 'Some.Class', 'extends', 'SomeOther', '{']) + + def test_annotation(self): + self._test("method @Nullable public void name();", + ['method', '@', 'Nullable', 'public', 'void', 'name', '(', ')', ';']) + + def test_annotation_args(self): + self._test("@Some(val=1, other=2) class Class {", + ['@', 'Some', '(', 'val', '=', '1', ',', 'other', '=', '2', ')', + 'class', 'Class', '{']) + def test_comment(self): + self._test("some //comment", ['some']) + + def test_strings(self): + self._test(r'"" "foo" "\"" "\\"', ['""', '"foo"', r'"\""', r'"\\"']) + + def test_at_interface(self): + self._test("public @interface Annotation {", + ['public', '@interface', 'Annotation', '{']) + + def test_array_type(self): + self._test("int[][]", ['int', '[]', '[]']) + + def test_generics(self): + self._test("<>foobar", + ['<', '>', 'foobar', '<', 'A', 'extends', 'Object', '>']) + +class V2ParserTests(unittest.TestCase): + def _cls(self, raw): + pkg = apilint.Package(999, "package pkg {", None) + return apilint.Class(pkg, 1, raw, '', sig_format=2) + + def _method(self, raw, cls=None): + if not cls: + cls = self._cls("class Class {") + return apilint.Method(cls, 1, raw, '', sig_format=2) + + def _field(self, raw): + cls = self._cls("class Class {") + return apilint.Field(cls, 1, raw, '', sig_format=2) + + def test_class(self): + cls = self._cls("@Deprecated @IntRange(from=1, to=2) public static abstract class Some.Name extends Super implements Interface {") + self.assertTrue('deprecated' in cls.split) + self.assertTrue('static' in cls.split) + self.assertTrue('abstract' in cls.split) + self.assertTrue('class' in cls.split) + self.assertEquals('Super', cls.extends) + self.assertEquals('Interface', cls.implements) + self.assertEquals('pkg.Some.Name', cls.fullname) + + def test_interface(self): + cls = self._cls("@Deprecated @IntRange(from=1, to=2) public interface Some.Name extends Interface {") + self.assertTrue('deprecated' in cls.split) + self.assertTrue('interface' in cls.split) + self.assertEquals('Interface', cls.extends) + self.assertEquals('Interface', cls.implements) + self.assertEquals('pkg.Some.Name', cls.fullname) + + def test_at_interface(self): + cls = self._cls("@java.lang.annotation.Target({java.lang.annotation.ElementType.TYPE, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.PARAMETER, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.LOCAL_VARIABLE}) @java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS) public @interface SuppressLint {") + self.assertTrue('@interface' in cls.split) + self.assertEquals('pkg.SuppressLint', cls.fullname) + + def test_parse_method(self): + m = self._method("method @Deprecated public static Class[][] name(" + + "Class[][], Class[][]...) throws Exception, T;") + self.assertTrue('static' in m.split) + self.assertTrue('public' in m.split) + self.assertTrue('method' in m.split) + self.assertTrue('deprecated' in m.split) + self.assertEquals('java.lang.Class[][]', m.typ) + self.assertEquals('name', m.name) + self.assertEquals(['java.lang.Class[][]', 'java.lang.Class[][]...'], m.args) + self.assertEquals(['java.lang.Exception', 'T'], m.throws) + + def test_ctor(self): + m = self._method("ctor @Deprecated ClassName();") + self.assertTrue('ctor' in m.split) + self.assertTrue('deprecated' in m.split) + self.assertEquals('ctor', m.typ) + self.assertEquals('ClassName', m.name) + + def test_parse_annotation_method(self): + cls = self._cls("@interface Annotation {") + self._method('method abstract String category() default "";', cls=cls) + self._method('method abstract boolean deepExport() default false;', cls=cls) + self._method('method abstract ViewDebug.FlagToString[] flagMapping() default {};', cls=cls) + + def test_parse_string_field(self): + f = self._field('field @Deprecated public final String SOME_NAME = "value";') + self.assertTrue('field' in f.split) + self.assertTrue('deprecated' in f.split) + self.assertTrue('final' in f.split) + self.assertEquals('java.lang.String', f.typ) + self.assertEquals('SOME_NAME', f.name) + self.assertEquals('value', f.value) + + def test_parse_field(self): + f = self._field('field public Object SOME_NAME;') + self.assertTrue('field' in f.split) + self.assertEquals('java.lang.Object', f.typ) + self.assertEquals('SOME_NAME', f.name) + self.assertEquals(None, f.value) + + def test_parse_int_field(self): + f = self._field('field public int NAME = 123;') + self.assertTrue('field' in f.split) + self.assertEquals('int', f.typ) + self.assertEquals('NAME', f.name) + self.assertEquals('123', f.value) + + def test_parse_quotient_field(self): + f = self._field('field public int NAME = (0.0/0.0);') + self.assertTrue('field' in f.split) + self.assertEquals('int', f.typ) + self.assertEquals('NAME', f.name) + self.assertEquals('( 0.0 / 0.0 )', f.value) + if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() From e5eeae7c23e2b35565274ad9cac38a3a550f33c9 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Fri, 4 Jan 2019 20:10:06 +0100 Subject: [PATCH 10/13] ApiLint: Performance improvements for 2.0 signature format parsing Test: tools/apilint/apilint_sha.sh HEAD Test: python tools/apilint/apilint_test.py Change-Id: I76b979dd81702afce6468ac377230e589a25e08f (cherry picked from commit a30d062775da3812589a6ce3e49be9d697ef0d6d) --- tools/apilint/apilint.py | 96 ++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 847b43bdd5ae2..5680c8ca3a6ac 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -220,55 +220,65 @@ class Package(): def __repr__(self): return self.raw -class V2Tokenizer(): +class V2Tokenizer(object): + __slots__ = ["raw"] + DELIMITER = re.compile(r'\s+|[()@<>;,={}/"]|\[\]') STRING_SPECIAL = re.compile(r'["\\]') def __init__(self, raw): self.raw = raw - self.current = 0 - def __iter__(self): - return self + def tokenize(self): + tokens = [] + current = 0 + raw = self.raw + length = len(raw) - def next(self): - if self.current >= len(self.raw): - raise StopIteration + while current < length: + while current < length: + start = current + match = V2Tokenizer.DELIMITER.search(raw, start) + if match is not None: + match_start = match.start() + if match_start == current: + end = match.end() + else: + end = match_start + else: + end = length - start = self.current - match = V2Tokenizer.DELIMITER.search(self.raw, start) - if match: - match_start = match.start() - if match_start == self.current: - end = match.end() - else: - end = match_start - else: - end = len(self.raw) + token = raw[start:end] + current = end - token = self.raw[start:end] - self.current = end + if token == "" or token[0] == " ": + continue + else: + break - if token.strip() == "": - return self.next() + if token == "@": + if raw[start:start+11] == "@interface ": + current = start + 11 + tokens.append("@interface") + continue + elif token == '/': + if raw[start:start+2] == "//": + current = length + continue + elif token == '"': + current, string_token = self.tokenize_string(raw, length, current) + tokens.append(token + string_token) + continue - if token == "@" and self.raw[start:start+10] == "@interface": - return token + self.next() + tokens.append(token) - if token == '/' and self.raw[start:start+2] == "//": - self.current = len(self.raw) - raise StopIteration + return tokens - if token == '"': - return token + self.tokenize_string() - - return token - - def tokenize_string(self): - start = self.current - end = len(self.raw) + def tokenize_string(self, raw, length, current): + start = current + end = length while start < end: - match = V2Tokenizer.STRING_SPECIAL.search(self.raw, start) + match = V2Tokenizer.STRING_SPECIAL.search(raw, start) if match: if match.group() == '"': end = match.end() @@ -279,19 +289,21 @@ class V2Tokenizer(): else: raise ValueError("Unexpected match: `%s`" % (match.group())) else: - raise ValueError("Unexpected EOF tokenizing string: `%s`" % (self.raw[self.current - 1:],)) + raise ValueError("Unexpected EOF tokenizing string: `%s`" % (raw[current - 1:],)) - token = self.raw[self.current:end] - self.current = end - return token + token = raw[current:end] + return end, token + +class V2LineParser(object): + __slots__ = ["tokenized", "current", "len"] -class V2LineParser(): MODIFIERS = set("public protected internal private abstract default static final transient volatile synchronized".split()) JAVA_LANG_TYPES = set("AbstractMethodError AbstractStringBuilder Appendable ArithmeticException ArrayIndexOutOfBoundsException ArrayStoreException AssertionError AutoCloseable Boolean BootstrapMethodError Byte Character CharSequence Class ClassCastException ClassCircularityError ClassFormatError ClassLoader ClassNotFoundException Cloneable CloneNotSupportedException Comparable Compiler Deprecated Double Enum EnumConstantNotPresentException Error Exception ExceptionInInitializerError Float FunctionalInterface IllegalAccessError IllegalAccessException IllegalArgumentException IllegalMonitorStateException IllegalStateException IllegalThreadStateException IncompatibleClassChangeError IndexOutOfBoundsException InheritableThreadLocal InstantiationError InstantiationException Integer InternalError InterruptedException Iterable LinkageError Long Math NegativeArraySizeException NoClassDefFoundError NoSuchFieldError NoSuchFieldException NoSuchMethodError NoSuchMethodException NullPointerException Number NumberFormatException Object OutOfMemoryError Override Package package-info.java Process ProcessBuilder ProcessEnvironment ProcessImpl Readable ReflectiveOperationException Runnable Runtime RuntimeException RuntimePermission SafeVarargs SecurityException SecurityManager Short StackOverflowError StackTraceElement StrictMath String StringBuffer StringBuilder StringIndexOutOfBoundsException SuppressWarnings System Thread ThreadDeath ThreadGroup ThreadLocal Throwable TypeNotPresentException UNIXProcess UnknownError UnsatisfiedLinkError UnsupportedClassVersionError UnsupportedOperationException VerifyError VirtualMachineError Void".split()) def __init__(self, raw): - self.tokenized = list(V2Tokenizer(raw)) + self.tokenized = V2Tokenizer(raw).tokenize() self.current = 0 + self.len = len(self.tokenized) def parse_into_method(self, method): method.split = [] @@ -378,7 +390,7 @@ class V2LineParser(): return found def eof(self): - return self.current == len(self.tokenized) + return self.current == self.len def parse_eof(self): if not self.eof(): From 1f1b6a84644916bd2b48f26ccb8cb31b336d4e2f Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Sat, 5 Jan 2019 20:09:38 +0100 Subject: [PATCH 11/13] ApiLint: Performance improvements for verification Test: tools/apilint/apilint_sha.sh HEAD Test: python tools/apilint/apilint_test.py Change-Id: I90f18181cd0d3c43f176d7c9d1b198f6f5172390 (cherry picked from commit 7f8886a9933abbb6f1fdd97d920185b80d439339) --- tools/apilint/apilint.py | 81 ++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 5680c8ca3a6ac..441c1209a7426 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -26,7 +26,7 @@ $ git blame api/current.txt -t -e > /tmp/currentblame.txt $ apilint.py /tmp/currentblame.txt previous.txt --no-color """ -import re, sys, collections, traceback, argparse +import re, sys, collections, traceback, argparse, itertools BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8) @@ -1073,48 +1073,66 @@ def verify_internal(clazz): if clazz.pkg.name.startswith("com.android"): error(clazz, None, None, "Internal classes must not be exposed") +def layering_build_ranking(ranking_list): + r = {} + for rank, ps in enumerate(ranking_list): + if not isinstance(ps, list): + ps = [ps] + for p in ps: + rs = r + for n in p.split('.'): + if n not in rs: + rs[n] = {} + rs = rs[n] + rs['-rank'] = rank + return r + +LAYERING_PACKAGE_RANKING = layering_build_ranking([ + ["android.service","android.accessibilityservice","android.inputmethodservice","android.printservice","android.appwidget","android.webkit","android.preference","android.gesture","android.print"], + "android.app", + "android.widget", + "android.view", + "android.animation", + "android.provider", + ["android.content","android.graphics.drawable"], + "android.database", + "android.text", + "android.graphics", + "android.os", + "android.util" +]) def verify_layering(clazz): """Catch package layering violations. For example, something in android.os depending on android.app.""" - ranking = [ - ["android.service","android.accessibilityservice","android.inputmethodservice","android.printservice","android.appwidget","android.webkit","android.preference","android.gesture","android.print"], - "android.app", - "android.widget", - "android.view", - "android.animation", - "android.provider", - ["android.content","android.graphics.drawable"], - "android.database", - "android.text", - "android.graphics", - "android.os", - "android.util" - ] def rank(p): - for i in range(len(ranking)): - if isinstance(ranking[i], list): - for j in ranking[i]: - if p.startswith(j): return i + r = None + l = LAYERING_PACKAGE_RANKING + for n in p.split('.'): + if n in l: + l = l[n] + if '-rank' in l: + r = l['-rank'] else: - if p.startswith(ranking[i]): return i + break + return r cr = rank(clazz.pkg.name) if cr is None: return for f in clazz.fields: ir = rank(f.typ) - if ir and ir < cr: + if ir is not None and ir < cr: warn(clazz, f, "FW6", "Field type violates package layering") - for m in clazz.methods: + for m in itertools.chain(clazz.methods, clazz.ctors): ir = rank(m.typ) - if ir and ir < cr: + if ir is not None and ir < cr: warn(clazz, m, "FW6", "Method return type violates package layering") for arg in m.args: ir = rank(arg) - if ir and ir < cr: + if ir is not None and ir < cr: warn(clazz, m, "FW6", "Method argument type violates package layering") @@ -1205,21 +1223,18 @@ def verify_exception(clazz): 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") +GOOGLE_IGNORECASE = re.compile("google", re.IGNORECASE) def verify_google(clazz): """Verifies that APIs never reference Google.""" - if re.search("google", clazz.raw, re.IGNORECASE): + if GOOGLE_IGNORECASE.search(clazz.raw) is not None: 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") + for test in clazz.ctors, clazz.fields, clazz.methods: + for t in test: + if GOOGLE_IGNORECASE.search(t.raw) is not None: + error(clazz, t, None, "Must never reference Google") def verify_bitset(clazz): From 5cdfb69429eb60fc274424d4dcb6b166ed550a42 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Sat, 5 Jan 2019 22:04:55 +0100 Subject: [PATCH 12/13] ApiLint: Add Kotlin-style type support Test: tools/apilint/apilint_sha.sh HEAD && python tools/apilint/apilint_test.py Change-Id: Iac1fdabcbeffe57c8288d73b2359e8ce0b2bc3eb (cherry picked from commit 7884d6b9090c586ac0d72abe0e6efab191a143a7) --- tools/apilint/apilint.py | 29 +++++++++++++++++++++++++---- tools/apilint/apilint_test.py | 15 +++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 441c1209a7426..acf1f1e9902e8 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -223,7 +223,7 @@ class Package(): class V2Tokenizer(object): __slots__ = ["raw"] - DELIMITER = re.compile(r'\s+|[()@<>;,={}/"]|\[\]') + DELIMITER = re.compile(r'\s+|[()@<>;,={}/"!?]|\[\]|\.\.\.') STRING_SPECIAL = re.compile(r'["\\]') def __init__(self, raw): @@ -435,19 +435,32 @@ class V2LineParser(object): ret.append(self.parse_token()) return ret + def parse_kotlin_nullability(self): + t = self.lookahead() + if t == "?" or t == "!": + return self.parse_token() + return None + def parse_type(self): type = self.parse_token() if type in V2LineParser.JAVA_LANG_TYPES: type = "java.lang." + type self.parse_matching_paren("<", ">") - while self.parse_if("[]"): - type += "[]" + while True: + t = self.lookahead() + if t == "[]": + type += self.parse_token() + elif self.parse_kotlin_nullability() is not None: + pass # discard nullability for now + else: + break return type def parse_arg_type(self): type = self.parse_type() if self.parse_if("..."): type += "..." + self.parse_kotlin_nullability() # discard nullability for now return type def parse_name(self): @@ -466,7 +479,15 @@ class V2LineParser(object): def parse_arg(self): self.parse_annotations() - return self.parse_arg_type() + type = self.parse_arg_type() + l = self.lookahead() + if l != "," and l != ")": + self.parse_token() # kotlin argument name + if self.parse_if('='): # kotlin default value + (self.parse_matching_paren('(', ')') or + self.parse_matching_paren('{', '}') or + self.parse_token() and self.parse_matching_paren('(', ')')) + return type def parse_throws(self): ret = [] diff --git a/tools/apilint/apilint_test.py b/tools/apilint/apilint_test.py index 4e3c0349c0bc7..081e98defa171 100644 --- a/tools/apilint/apilint_test.py +++ b/tools/apilint/apilint_test.py @@ -154,6 +154,14 @@ class V2TokenizerTests(unittest.TestCase): self._test("class Some.Class extends SomeOther {", ['class', 'Some.Class', 'extends', 'SomeOther', '{']) + def test_varargs(self): + self._test("name(String...)", + ['name', '(', 'String', '...', ')']) + + def test_kotlin(self): + self._test("String? name(String!...)", + ['String', '?', 'name', '(', 'String', '!', '...', ')']) + def test_annotation(self): self._test("method @Nullable public void name();", ['method', '@', 'Nullable', 'public', 'void', 'name', '(', ')', ';']) @@ -271,5 +279,12 @@ class V2ParserTests(unittest.TestCase): self.assertEquals('NAME', f.name) self.assertEquals('( 0.0 / 0.0 )', f.value) + def test_kotlin_types(self): + self._field('field public List?[]![]? NAME;') + self._method("method Class?[]![][]? name(Type!, Type argname," + + "Class[][]?[]!...!) throws Exception, T;") + self._method("method T name(T a = 1, T b = A(1), Lambda f = { false }, N? n = null, " + + """double c = (1/0), float d = 1.0f, String s = "heyo", char c = 'a');""") + if __name__ == "__main__": unittest.main() From d1e3892119a6a415fb39da5db21e06483db0ae03 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Mon, 14 Jan 2019 15:44:15 +0100 Subject: [PATCH 13/13] ApiLint: Add operator keyword and property parsing Also fix up some issues with expression parsing, type use annotations, etc. Test: python tools/apilint/apilint_test.py Change-Id: I38145a51470ce6c3e5813a546d681489fd87fc19 (cherry picked from commit 403c8e35d8e7cc0f81a0a2c42d038c47e1b2703f) --- tools/apilint/apilint.py | 34 ++++++++++++++++++--------- tools/apilint/apilint_test.py | 43 ++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index acf1f1e9902e8..d1fe43ea0c7c9 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -297,7 +297,7 @@ class V2Tokenizer(object): class V2LineParser(object): __slots__ = ["tokenized", "current", "len"] - MODIFIERS = set("public protected internal private abstract default static final transient volatile synchronized".split()) + MODIFIERS = set("public protected internal private abstract default static final transient volatile synchronized native operator sealed strictfp infix inline suspend vararg".split()) JAVA_LANG_TYPES = set("AbstractMethodError AbstractStringBuilder Appendable ArithmeticException ArrayIndexOutOfBoundsException ArrayStoreException AssertionError AutoCloseable Boolean BootstrapMethodError Byte Character CharSequence Class ClassCastException ClassCircularityError ClassFormatError ClassLoader ClassNotFoundException Cloneable CloneNotSupportedException Comparable Compiler Deprecated Double Enum EnumConstantNotPresentException Error Exception ExceptionInInitializerError Float FunctionalInterface IllegalAccessError IllegalAccessException IllegalArgumentException IllegalMonitorStateException IllegalStateException IllegalThreadStateException IncompatibleClassChangeError IndexOutOfBoundsException InheritableThreadLocal InstantiationError InstantiationException Integer InternalError InterruptedException Iterable LinkageError Long Math NegativeArraySizeException NoClassDefFoundError NoSuchFieldError NoSuchFieldException NoSuchMethodError NoSuchMethodException NullPointerException Number NumberFormatException Object OutOfMemoryError Override Package package-info.java Process ProcessBuilder ProcessEnvironment ProcessImpl Readable ReflectiveOperationException Runnable Runtime RuntimeException RuntimePermission SafeVarargs SecurityException SecurityManager Short StackOverflowError StackTraceElement StrictMath String StringBuffer StringBuilder StringIndexOutOfBoundsException SuppressWarnings System Thread ThreadDeath ThreadGroup ThreadLocal Throwable TypeNotPresentException UNIXProcess UnknownError UnsatisfiedLinkError UnsupportedClassVersionError UnsupportedOperationException VerifyError VirtualMachineError Void".split()) def __init__(self, raw): @@ -355,7 +355,7 @@ class V2LineParser(object): self.parse_eof() def parse_into_field(self, field): - kind = self.parse_token("field") + kind = self.parse_one_of("field", "property") field.split = [kind] annotations = self.parse_annotations() if "@Deprecated" in annotations: @@ -442,13 +442,19 @@ class V2LineParser(object): return None def parse_type(self): + self.parse_annotations() type = self.parse_token() + if type[-1] == '.': + self.parse_annotations() + type += self.parse_token() if type in V2LineParser.JAVA_LANG_TYPES: type = "java.lang." + type self.parse_matching_paren("<", ">") while True: t = self.lookahead() - if t == "[]": + if t == "@": + self.parse_annotation() + elif t == "[]": type += self.parse_token() elif self.parse_kotlin_nullability() is not None: pass # discard nullability for now @@ -478,17 +484,23 @@ class V2LineParser(object): self.parse_token(",") def parse_arg(self): + self.parse_if("vararg") # kotlin vararg self.parse_annotations() type = self.parse_arg_type() l = self.lookahead() if l != "," and l != ")": - self.parse_token() # kotlin argument name + if self.lookahead() != '=': + self.parse_token() # kotlin argument name if self.parse_if('='): # kotlin default value - (self.parse_matching_paren('(', ')') or - self.parse_matching_paren('{', '}') or - self.parse_token() and self.parse_matching_paren('(', ')')) + self.parse_expression() return type + def parse_expression(self): + while not self.lookahead() in [')', ',', ';']: + (self.parse_matching_paren('(', ')') or + self.parse_matching_paren('{', '}') or + self.parse_token()) + def parse_throws(self): ret = [] if self.parse_if("throws"): @@ -516,7 +528,7 @@ class V2LineParser(object): def parse_annotation_default(self): if self.parse_if("default"): - self.parse_value() + self.parse_expression() def parse_value(self): if self.lookahead() == "{": @@ -599,7 +611,7 @@ def _parse_stream_to_generator(f): clazz.ctors.append(Method(clazz, line, raw, blame, sig_format=sig_format)) elif raw.startswith(" method"): clazz.methods.append(Method(clazz, line, raw, blame, sig_format=sig_format)) - elif raw.startswith(" field"): + elif raw.startswith(" field") or raw.startswith(" property"): clazz.fields.append(Field(clazz, line, raw, blame, sig_format=sig_format)) elif raw.startswith(" }") and clazz: yield clazz @@ -942,7 +954,7 @@ def verify_fields(clazz): else: error(clazz, f, "F2", "Bare fields must be marked final, or add accessors if mutable") - if not "static" in f.split: + if "static" not in f.split and "property" not in f.split: if not re.match("[a-z]([a-zA-Z]+)?", f.name): error(clazz, f, "S1", "Non-static fields must be named using myField style") @@ -1650,7 +1662,7 @@ def verify_method_name_not_kotlin_operator(clazz): binary.add(op) for m in clazz.methods: - if 'static' in m.split: + if 'static' in m.split or 'operator' in m.split: continue # https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators diff --git a/tools/apilint/apilint_test.py b/tools/apilint/apilint_test.py index 081e98defa171..fde61a902341c 100644 --- a/tools/apilint/apilint_test.py +++ b/tools/apilint/apilint_test.py @@ -225,11 +225,12 @@ class V2ParserTests(unittest.TestCase): self.assertEquals('pkg.SuppressLint', cls.fullname) def test_parse_method(self): - m = self._method("method @Deprecated public static Class[][] name(" + m = self._method("method @Deprecated public static native Class[][] name(" + "Class[][], Class[][]...) throws Exception, T;") self.assertTrue('static' in m.split) self.assertTrue('public' in m.split) self.assertTrue('method' in m.split) + self.assertTrue('native' in m.split) self.assertTrue('deprecated' in m.split) self.assertEquals('java.lang.Class[][]', m.typ) self.assertEquals('name', m.name) @@ -248,6 +249,7 @@ class V2ParserTests(unittest.TestCase): self._method('method abstract String category() default "";', cls=cls) self._method('method abstract boolean deepExport() default false;', cls=cls) self._method('method abstract ViewDebug.FlagToString[] flagMapping() default {};', cls=cls) + self._method('method abstract ViewDebug.FlagToString[] flagMapping() default (double)java.lang.Float.NEGATIVE_INFINITY;', cls=cls) def test_parse_string_field(self): f = self._field('field @Deprecated public final String SOME_NAME = "value";') @@ -286,5 +288,44 @@ class V2ParserTests(unittest.TestCase): self._method("method T name(T a = 1, T b = A(1), Lambda f = { false }, N? n = null, " + """double c = (1/0), float d = 1.0f, String s = "heyo", char c = 'a');""") + def test_kotlin_operator(self): + self._method('method public operator void unaryPlus(androidx.navigation.NavDestination);') + self._method('method public static operator androidx.navigation.NavDestination get(androidx.navigation.NavGraph, @IdRes int id);') + self._method('method public static operator T get(androidx.navigation.NavigatorProvider, kotlin.reflect.KClass clazz);') + + def test_kotlin_property(self): + self._field('property public VM value;') + self._field('property public final String? action;') + + def test_kotlin_varargs(self): + self._method('method public void error(int p = "42", Integer int2 = "null", int p1 = "42", vararg String args);') + + def test_kotlin_default_values(self): + self._method('method public void foo(String! = null, String! = "Hello World", int = 42);') + self._method('method void method(String, String firstArg = "hello", int secondArg = "42", String thirdArg = "world");') + self._method('method void method(String, String firstArg = "hello", int secondArg = "42");') + self._method('method void method(String, String firstArg = "hello");') + self._method('method void edit(android.Type, boolean commit = false, Function1 action);') + self._method('method LruCache lruCache(int maxSize, Function2 sizeOf = { _, _ -> 1 }, Function1 create = { (V)null }, Function4 onEntryRemoved = { _, _, _, _ -> });') + self._method('method android.Bitmap? drawToBitmap(android.View, android.Config config = android.graphics.Bitmap.Config.ARGB_8888);') + self._method('method void emptyLambda(Function0 sizeOf = {});') + self._method('method void method1(int p = 42, Integer? int2 = null, int p1 = 42, String str = "hello world", java.lang.String... args);') + self._method('method void method2(int p, int int2 = (2 * int) * some.other.pkg.Constants.Misc.SIZE);') + self._method('method void method3(String str, int p, int int2 = double(int) + str.length);') + self._method('method void print(test.pkg.Foo foo = test.pkg.Foo());') + + def test_type_use_annotation(self): + self._method('method public static int codePointAt(char @NonNull [], int);') + self._method('method @NonNull public java.util.Set> entrySet();') + + m = self._method('method @NonNull public java.lang.annotation.@NonNull Annotation @NonNull [] getAnnotations();') + self.assertEquals('java.lang.annotation.Annotation[]', m.typ) + + m = self._method('method @NonNull public abstract java.lang.annotation.@NonNull Annotation @NonNull [] @NonNull [] getParameterAnnotations();') + self.assertEquals('java.lang.annotation.Annotation[][]', m.typ) + + m = self._method('method @NonNull public @NonNull String @NonNull [] split(@NonNull String, int);') + self.assertEquals('java.lang.String[]', m.typ) + if __name__ == "__main__": unittest.main()