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)

2 files changed, 139 insertions, 26 deletions

diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py
index 1eec218c7be3..6476abd8268e 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 59fc14d97bee..ece69a99f579 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<android.app.WallpaperColors> 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<android.app.WallpaperColors> 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