Closes #31: Add lint for fields-only classes.

This adds a lint to make sure that classes that only contain final fields have
a protected or public constructor in the API so that external clients can mock
these classes. It also checks that these classes are not final for test
subclassing.

apilint/src/main/resources/apilint.py

@@ -279,7 +279,7 @@ def __repr__(self):
 
     def json(self):
         return {
-            'rule': repr(self.rule),
+            'rule': self.rule,
             'msg': self.msg,
             'detail': repr(self.detail),
             'line': repr(self.line),
@@ -507,44 +507,28 @@ def verify_protected(clazz):
         if "protected" in f.split:
             error(clazz, f, "M7", "Protected fields not allowed; must be public")
 
+def verify_final_fields_only_class(clazz):
+    if clazz.methods or not clazz.fields:
+        # Not a final field-only class
+        return
+
+    for f in clazz.fields:
+        if "final" not in f.split:
+            # Not a final field-only class
+            return
+
+    if not clazz.ctors:
+        error(clazz, None, "GV1", "Field-only classes need at least one constructor for mocking.")
+
+    if "final" in clazz.split:
+        error(clazz, None, "GV2", "Field-only classes should not be final for mocking.")
 
 def verify_fields(clazz):
     """Verify that all exposed fields are final.
     Exposed fields must follow myName style.
     Catch internal mFoo objects being exposed."""
 
-    IGNORE_BARE_FIELDS = [
-        "android.app.ActivityManager.RecentTaskInfo",
-        "android.app.Notification",
-        "android.content.pm.ActivityInfo",
-        "android.content.pm.ApplicationInfo",
-        "android.content.pm.ComponentInfo",
-        "android.content.pm.ResolveInfo",
-        "android.content.pm.FeatureGroupInfo",
-        "android.content.pm.InstrumentationInfo",
-        "android.content.pm.PackageInfo",
-        "android.content.pm.PackageItemInfo",
-        "android.content.res.Configuration",
-        "android.graphics.BitmapFactory.Options",
-        "android.os.Message",
-        "android.system.StructPollfd",
-    ]
-
     for f in clazz.fields:
-        if not "final" in f.split:
-            if clazz.fullname in IGNORE_BARE_FIELDS:
-                pass
-            elif clazz.fullname.endswith("LayoutParams"):
-                pass
-            elif clazz.fullname.startswith("android.util.Mutable"):
-                pass
-            else:
-                error(clazz, f, "F2", "Bare fields must be marked final, or add accessors if mutable")
-
-        if not "static" in f.split:
-            if not re.match("[a-z]([a-zA-Z]+)?", f.name):
-                error(clazz, f, "S1", "Non-static fields must be named using myField style")
-
         if re.match("[ms][A-Z]", f.name):
             error(clazz, f, "F1", "Internal objects must not be exposed")
 
@@ -1433,6 +1417,7 @@ def examine_clazz(clazz):
     verify_tense(clazz)
     verify_icu(clazz)
     verify_clone(clazz)
+    verify_final_fields_only_class(clazz)
 
 
 def examine_stream(stream):
@@ -1632,16 +1617,17 @@ def dump_result_json(args, compat_fail, api_changes, failures):
             print("")
         sys.exit(131)
 
+    if len(cur_fail) != 0:
+        print("%s API style issues %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))))
+        for f in sorted(cur_fail):
+            print(cur_fail[f])
+            print("")
+        sys.exit(77)
+
     if args['show_noticed'] and len(cur_noticed) != 0:
         print("%s API changes noticed %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))))
         for f in sorted(cur_noticed.keys()):
             print(f)
         print("")
         sys.exit(10)
 
-    if len(cur_fail) != 0:
-        print("%s API style issues %s\n" % ((format(fg=WHITE, bg=BLUE, bold=True), format(reset=True))))
-        for f in sorted(cur_fail):
-            print(cur_fail[f])
-            print("")
-        sys.exit(77)

apilint/src/test/resources/apilint_test.py

@@ -13,6 +13,7 @@
 ERROR_CODES = {
     'NO_CHANGE': 0,
     'API_CHANGE': 10,
+    'API_ERROR': 77,
     'INCOMPATIBLE': 131,
 }
 
@@ -43,6 +44,11 @@
 
     error_code = sp.call(test)
 
+    with open(json_file) as f:
+        json_result = json.load(f)
+
+    print(json.dumps(json_result, indent=2))
+
     expected_error_code = ERROR_CODES[t["expected"]]
     if error_code != expected_error_code:
          print("The following test is expected to fail with {} "
@@ -51,12 +57,8 @@
          print(" ".join(test))
          sys.exit(1)
 
-    with open(json_file) as f:
-        json_result = json.load(f)
-
-    print(json.dumps(json_result, indent=2))
-
-    assert len(json_result['failures']) == 0
+    if t['expected'] != 'API_ERROR':
+        assert len(json_result['failures']) == 0
 
     if t['expected'] == 'INCOMPATIBLE':
         assert len(json_result['compat_failures']) == 1
@@ -66,6 +68,11 @@
     if t['expected'] == 'API_CHANGE':
         assert len(json_result['api_changes']) > 0
 
+    if t['expected'] == 'API_ERROR':
+        assert len(json_result['failures']) > 0
+        assert json_result['failure'] == True
+        assert json_result['failures'][0]['rule'] == t['rule']
+
     if t['expected'] == 'NO_CHANGE':
         assert len(json_result['api_changes']) == 0
         assert json_result['failure'] == False

apilint/src/test/resources/apilint_test/test-fields-only-class-final.after.txt

@@ -0,0 +1,8 @@
+package test {
+  public final class FieldOnlyTest {
+    ctor protected FieldOnlyTest();
+    field public final int testField0;
+    field public final int testField1;
+    field public final int testField2;
+  }
+}

apilint/src/test/resources/apilint_test/test-fields-only-class-final.before.txt

@@ -0,0 +1,2 @@
+package test {
+}

apilint/src/test/resources/apilint_test/test-fields-only-class.after.txt

@@ -0,0 +1,7 @@
+package test {
+  public class TestFieldsOnly {
+    field public final int testField0;
+    field public final int testField1;
+    field public final int testField2;
+  }
+}

apilint/src/test/resources/apilint_test/test-fields-only-class.before.txt

@@ -0,0 +1,2 @@
+package test {
+}

apilint/src/test/resources/apilint_test/tests.py

@@ -68,6 +68,14 @@
   },{
     "test": "test-annotation-add-class",
     "expected": "API_CHANGE"
+  },{
+    "test": "test-fields-only-class",
+    "expected": "API_ERROR",
+    "rule": "GV1"
+  },{
+    "test": "test-fields-only-class-final",
+    "expected": "API_ERROR",
+    "rule": "GV2"
   },{
     "test": "test-whitespace-change",
     "expected": "NO_CHANGE"