Revert "Android: Add lint regression test (reland)"

This reverts commit 82523b3.

Reason for revert: LintTest breaks compile here https://ci.chromium.org/p/chromium/builders/ci/android-archive-rel/14988

Original change's description:
> Android: Add lint regression test (reland)
> 
> New API related checks are some of our most valuable lint checks, as
> triggering an error causes a crash directly.
> 
> This CL adds compile test targets to ensure that code with API level
> errors trigger lint warnings. This prevents lint being accidentally
> turned off.
> 
> Also delete now obsolete //build/android/lint directory and remove some
> already-fixed suppressions from lint-suppressions.xml.
> 
> This is a reland of 3f2f2ac with
> missing LintTest.java added back.
> 
> Tbr: mheikal@chromium.org
> Bug: 1108309
> Change-Id: Ic5ac63ccd8aac2ebd3193d26294b8e27b233c903
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416575
> Reviewed-by: Peter Wen <wnwen@chromium.org>
> Commit-Queue: Peter Wen <wnwen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#807963}

TBR=wnwen@chromium.org,mheikal@chromium.org

Change-Id: I72013a4a8ddc6f15da461674acce98a6afd348f4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1108309
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416743
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807980}

build/android/BUILD.gn

@@ -16,27 +16,6 @@ if (enable_java_templates) {
     create_cache = true
   }
 
-  android_library("lint_test_java") {
-    sources = [ "java/test/LintTest.java" ]
-  }
-
-  android_apk("lint_test_apk") {
-    # This cannot be marked testonly since lint has special ignores for testonly
-    # targets. We need to test linting a normal apk target.
-    apk_name = "LintTest"
-    deps = [ ":lint_test_java" ]
-    android_manifest = "//build/android/AndroidManifest.xml"
-  }
-
-  android_lint("lint_test") {
-    lint_expected_warnings = "InlinedApi,NewApi"
-    _test_apk_target = ":lint_test_apk"
-    deps = [ "${_test_apk_target}__java" ]
-    build_config_dep = "$_test_apk_target$build_config_target_suffix"
-    build_config = get_label_info(_test_apk_target, "target_gen_dir") + "/" +
-                   get_label_info(_test_apk_target, "name") + ".build_config"
-  }
-
   if (enable_jdk_library_desugaring) {
     dex_jdk_libs("all_jdk_libs") {
       output = "$target_out_dir/$target_name.l8.dex"

build/android/gyp/lint.py

@@ -8,7 +8,6 @@
 from __future__ import print_function
 
 import argparse
-import functools
 import logging
 import os
 import re
@@ -150,17 +149,6 @@ def _WriteXmlFile(root, path):
             root, encoding='utf-8')).toprettyxml(indent='  '))
 
 
-def _CheckLintWarning(expected_warnings, lint_output):
-  for expected_warning in expected_warnings.split(','):
-    expected_str = '[{}]'.format(expected_warning)
-    if expected_str not in lint_output:
-      raise Exception('Expected {!r} warning in lint output:\n{}.'.format(
-          expected_str, lint_output))
-
-  # Do not print warning
-  return ''
-
-
 def _RunLint(lint_binary_path,
              config_path,
              manifest_path,
@@ -176,7 +164,6 @@ def _RunLint(lint_binary_path,
              android_sdk_root,
              lint_gen_dir,
              baseline,
-             expected_warnings,
              testonly_target=False,
              warnings_as_errors=False):
   logging.info('Lint starting')
@@ -262,8 +249,6 @@ def _RunLint(lint_binary_path,
   # This filter is necessary for JDK11.
   stderr_filter = build_utils.FilterReflectiveAccessJavaWarnings
   stdout_filter = lambda x: build_utils.FilterLines(x, 'No issues found')
-  if expected_warnings:
-    stdout_filter = functools.partial(_CheckLintWarning, expected_warnings)
 
   start = time.time()
   logging.debug('Lint command %s', ' '.join(cmd))
@@ -354,9 +339,6 @@ def _ParseArgs(argv):
   parser.add_argument('--baseline',
                       help='Baseline file to ignore existing errors and fail '
                       'on new errors.')
-  parser.add_argument('--expected-warnings',
-                      help='Comma separated list of warnings to test for in '
-                      'the output, failing if not found.')
 
   args = parser.parse_args(build_utils.ExpandFileArgs(argv))
   args.java_sources = build_utils.ParseGnList(args.java_sources)
@@ -401,7 +383,6 @@ def main():
            args.android_sdk_root,
            args.lint_gen_dir,
            args.baseline,
-           args.expected_warnings,
            testonly_target=args.testonly,
            warnings_as_errors=args.warnings_as_errors)
   logging.info('Creating stamp file')

build/android/lint/suppress.py

@@ -0,0 +1,131 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Add all generated lint_result.xml files to suppressions.xml"""
+
+# pylint: disable=no-member
+
+from __future__ import print_function
+
+import argparse
+import os
+import re
+import sys
+from xml.dom import minidom
+
+_BUILD_ANDROID_DIR = os.path.join(os.path.dirname(__file__), '..')
+sys.path.append(_BUILD_ANDROID_DIR)
+
+_TMP_DIR_RE = re.compile(r'^/tmp/.*/(SRC_ROOT[0-9]+|PRODUCT_DIR)/')
+_THIS_FILE = os.path.abspath(__file__)
+_DEFAULT_CONFIG_PATH = os.path.join(os.path.dirname(_THIS_FILE),
+                                    'suppressions.xml')
+_INSERT_COMMENT = ('TODO: This line was added by suppress.py,'
+                   ' please add an explanation.')
+
+
+class _Issue(object):
+
+  def __init__(self, dom_element):
+    self.regexps = set()
+    self.dom_element = dom_element
+
+
+def _CollectIssuesFromDom(dom):
+  issues_dict = {}
+  for issue_element in dom.getElementsByTagName('issue'):
+    issue_id = issue_element.attributes['id'].value
+    issue = _Issue(issue_element)
+    issues_dict[issue_id] = issue
+    for child in issue_element.childNodes:
+      if child.nodeType != minidom.Node.ELEMENT_NODE:
+        continue
+      if child.tagName == 'ignore' and child.getAttribute('regexp'):
+        issue.regexps.add(child.getAttribute('regexp'))
+
+  return issues_dict
+
+
+def _TrimWhitespaceNodes(n):
+  """Remove all whitespace-only TEXT_NODEs."""
+  rm_children = []
+  for c in n.childNodes:
+    if c.nodeType == minidom.Node.TEXT_NODE and c.data.strip() == '':
+      rm_children.append(c)
+    else:
+      _TrimWhitespaceNodes(c)
+
+  for c in rm_children:
+    n.removeChild(c)
+
+
+def _ParseAndInsertNewSuppressions(result_path, config_path):
+  print('Parsing %s' % config_path)
+  config_dom = minidom.parse(config_path)
+  issues_dict = _CollectIssuesFromDom(config_dom)
+  print('Parsing and merging %s' % result_path)
+  dom = minidom.parse(result_path)
+  for issue_element in dom.getElementsByTagName('issue'):
+    issue_id = issue_element.attributes['id'].value
+    severity = issue_element.attributes['severity'].value
+    path = issue_element.getElementsByTagName(
+        'location')[0].attributes['file'].value
+    # Strip temporary file path.
+    path = re.sub(_TMP_DIR_RE, '', path)
+    # Escape Java inner class name separator and suppress with regex instead
+    # of path. Doesn't use re.escape() as it is a bit too aggressive and
+    # escapes '_', causing trouble with PRODUCT_DIR.
+    regexp = path.replace('$', r'\$')
+    if issue_id not in issues_dict:
+      element = config_dom.createElement('issue')
+      element.attributes['id'] = issue_id
+      element.attributes['severity'] = severity
+      config_dom.documentElement.appendChild(element)
+      issue = _Issue(element)
+      issues_dict[issue_id] = issue
+    else:
+      issue = issues_dict[issue_id]
+      if issue.dom_element.getAttribute('severity') == 'ignore':
+        continue
+
+    if regexp not in issue.regexps:
+      issue.regexps.add(regexp)
+      ignore_element = config_dom.createElement('ignore')
+      ignore_element.attributes['regexp'] = regexp
+      issue.dom_element.appendChild(config_dom.createComment(_INSERT_COMMENT))
+      issue.dom_element.appendChild(ignore_element)
+
+    for issue_id, issue in issues_dict.iteritems():
+      if issue.dom_element.getAttribute('severity') == 'ignore':
+        print('Warning: [%s] is suppressed globally.' % issue_id)
+
+  # toprettyxml inserts whitespace, so delete whitespace first.
+  _TrimWhitespaceNodes(config_dom.documentElement)
+
+  with open(config_path, 'w') as f:
+    f.write(config_dom.toprettyxml(indent='  ', encoding='utf-8'))
+  print('Updated %s' % config_path)
+
+
+def _Suppress(config_path, result_path):
+  _ParseAndInsertNewSuppressions(result_path, config_path)
+
+
+def main():
+  parser = argparse.ArgumentParser(description=__doc__)
+  parser.add_argument('--config',
+                      help='Path to suppression.xml config file',
+                      default=_DEFAULT_CONFIG_PATH)
+  parser.add_argument('result_path',
+                      help='Lint results xml file',
+                      metavar='RESULT_FILE')
+  args = parser.parse_args()
+
+  _Suppress(args.config, args.result_path)
+
+
+if __name__ == '__main__':
+  main()

build/config/android/internal_rules.gni

@@ -988,22 +988,28 @@ if (enable_java_templates) {
         pool = "//build/toolchain:link_pool($default_toolchain)"
       }
 
-      # Lint requires generated sources and generated resources from the build.
-      # Turbine __header targets depend on all generated sources, and the
-      # __assetres targets depend on all generated resources.
+      # Lint only requires generated sources and generated resources from the
+      # build. Since turbine __header targets already depend on and generate all
+      # the generated sources, and the __assetres targets include all generated
+      # resources, lint only needs to depend on the header and assetres targets
+      # rather than the top level java targets.
       if (defined(invoker.deps)) {
         foreach(_dep, invoker.deps) {
           _target_label = get_label_info(_dep, "label_no_toolchain")
           if (filter_exclude([ _target_label ], _java_library_patterns) == [] &&
               filter_exclude([ _target_label ], _java_resource_patterns) !=
               []) {
             deps += [
+              # Strictly speaking the __header target is sufficient, since they
+              # already depend on resources (due to srcjar deps), but prefer to
+              # be more explicit here since if/when __header targets stop
+              # depending on resources (e.g. if R.java generation moves to java
+              # targets), lint will not be affected.
               "${_target_label}__assetres",
               "${_target_label}__header",
             ]
           } else {
-            # Keep non-java deps as they may generate files used only by lint.
-            # e.g. generated suppressions.xml files.
+            # Keep non-java deps as they may generate files used by lint.
             deps += [ _dep ]
           }
         }
@@ -1106,13 +1112,6 @@ if (enable_java_templates) {
           # The full classpath is required for annotation checks like @IntDef.
           "--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)",
         ]
-
-        if (defined(invoker.lint_expected_warnings)) {
-          args += [
-            "--expected-warnings",
-            invoker.lint_expected_warnings,
-          ]
-        }
       }
 
       outputs = [ _stamp_path ]

chrome/android/expectations/lint-suppressions.xml

@@ -176,8 +176,12 @@ Still reading?
     <ignore regexp="Static interface  method requires API level 24"/>
     <!-- 1: TaskInfo is refactored at API 29. -->
     <ignore regexp="Field requires API level .*`android.app.TaskInfo"/>
+    <!-- 1: TODO(crbug.com/1082222): Fix -->
+    <ignore regexp="chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/header/HeaderView.java"/>
     <!-- 1: TODO(crbug.com/1085410): Fix -->
     <ignore regexp="components/content_capture/android/java/src/org/chromium/components/content_capture"/>
+    <!-- 1: TODO(wnwen): File bugs to fix -->
+    <ignore regexp="android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/util/BoundaryInterfaceReflectionUtil.java"/>
     <!-- Endnote: Please specify number of suppressions when adding more -->
   </issue>
   <!-- This warning just adds a lot of false positives. -->