Commit d2bd56ab authored by Peter Wen's avatar Peter Wen Committed by Commit Bot

Android: Fix lint and add baseline

Turn off lint for chrome_public_test_apk as it was generating a lot of
warnings and errors that prevented relanding this lint CL ASAP. The
baseline was generated from monochrome_bundle, which covered almost all
the other lint targets with the exception of chrome_public_test_apk.

Since it was only possible to easily generate a baseline from one
target, it is faster to disable chrome_public_test_apk for the time
being.

Tbr: Submit before new errors are introduced
Bug: 1107450
Fixed: 1107450
Change-Id: Id0aaeaa6eaf7d1b561617a2ff5840401599730ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308852Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790196}
parent aa9733ba
...@@ -51,6 +51,7 @@ def _GenerateProjectFile(android_manifest, ...@@ -51,6 +51,7 @@ def _GenerateProjectFile(android_manifest,
android_sdk_root, android_sdk_root,
cache_dir, cache_dir,
sources=None, sources=None,
classpath=None,
srcjar_sources=None, srcjar_sources=None,
resource_sources=None, resource_sources=None,
android_sdk_version=None): android_sdk_version=None):
...@@ -79,6 +80,10 @@ def _GenerateProjectFile(android_manifest, ...@@ -79,6 +80,10 @@ def _GenerateProjectFile(android_manifest,
for source in sources: for source in sources:
src = ElementTree.SubElement(main_module, 'src') src = ElementTree.SubElement(main_module, 'src')
src.set('file', _SrcRelative(source)) src.set('file', _SrcRelative(source))
if classpath:
for file_path in classpath:
classpath_element = ElementTree.SubElement(main_module, 'classpath')
classpath_element.set('file', _SrcRelative(file_path))
if resource_sources: if resource_sources:
for resource_file in resource_sources: for resource_file in resource_sources:
resource = ElementTree.SubElement(main_module, 'resource') resource = ElementTree.SubElement(main_module, 'resource')
...@@ -116,6 +121,7 @@ def _RunLint(lint_binary_path, ...@@ -116,6 +121,7 @@ def _RunLint(lint_binary_path,
config_path, config_path,
manifest_path, manifest_path,
sources, sources,
classpath,
cache_dir, cache_dir,
android_sdk_version, android_sdk_version,
srcjars, srcjars,
...@@ -125,6 +131,7 @@ def _RunLint(lint_binary_path, ...@@ -125,6 +131,7 @@ def _RunLint(lint_binary_path,
resource_zips, resource_zips,
android_sdk_root, android_sdk_root,
lint_gen_dir, lint_gen_dir,
baseline,
testonly_target=False, testonly_target=False,
can_fail_build=False, can_fail_build=False,
silent=False): silent=False):
...@@ -139,6 +146,8 @@ def _RunLint(lint_binary_path, ...@@ -139,6 +146,8 @@ def _RunLint(lint_binary_path,
'--exitcode', # Sets error code if there are errors. '--exitcode', # Sets error code if there are errors.
'--quiet', # Silences lint's "." progress updates. '--quiet', # Silences lint's "." progress updates.
] ]
if baseline:
cmd.extend(['--baseline', _SrcRelative(baseline)])
if config_path: if config_path:
cmd.extend(['--config', _SrcRelative(config_path)]) cmd.extend(['--config', _SrcRelative(config_path)])
if testonly_target: if testonly_target:
...@@ -189,7 +198,8 @@ def _RunLint(lint_binary_path, ...@@ -189,7 +198,8 @@ def _RunLint(lint_binary_path,
logging.info('Generating project file') logging.info('Generating project file')
project_file_root = _GenerateProjectFile(lint_android_manifest_path, project_file_root = _GenerateProjectFile(lint_android_manifest_path,
android_sdk_root, cache_dir, sources, android_sdk_root, cache_dir, sources,
srcjar_sources, resource_sources, classpath, srcjar_sources,
resource_sources,
android_sdk_version) android_sdk_version)
project_xml_path = os.path.join(lint_gen_dir, 'project.xml') project_xml_path = os.path.join(lint_gen_dir, 'project.xml')
...@@ -289,12 +299,18 @@ def _ParseArgs(argv): ...@@ -289,12 +299,18 @@ def _ParseArgs(argv):
action='append', action='append',
help='GYP-list of resource zips, zip files of generated ' help='GYP-list of resource zips, zip files of generated '
'resource files.') 'resource files.')
parser.add_argument('--classpath',
help='List of jars to add to the classpath.')
parser.add_argument('--baseline',
help='Baseline file to ignore existing errors and fail '
'on new errors.')
args = parser.parse_args(build_utils.ExpandFileArgs(argv)) args = parser.parse_args(build_utils.ExpandFileArgs(argv))
args.java_sources = build_utils.ParseGnList(args.java_sources) args.java_sources = build_utils.ParseGnList(args.java_sources)
args.srcjars = build_utils.ParseGnList(args.srcjars) args.srcjars = build_utils.ParseGnList(args.srcjars)
args.resource_sources = build_utils.ParseGnList(args.resource_sources) args.resource_sources = build_utils.ParseGnList(args.resource_sources)
args.resource_zips = build_utils.ParseGnList(args.resource_zips) args.resource_zips = build_utils.ParseGnList(args.resource_zips)
args.classpath = build_utils.ParseGnList(args.classpath)
return args return args
...@@ -319,6 +335,7 @@ def main(): ...@@ -319,6 +335,7 @@ def main():
args.config_path, args.config_path,
args.manifest_path, args.manifest_path,
sources, sources,
args.classpath,
args.cache_dir, args.cache_dir,
args.android_sdk_version, args.android_sdk_version,
args.srcjars, args.srcjars,
...@@ -328,6 +345,7 @@ def main(): ...@@ -328,6 +345,7 @@ def main():
args.resource_zips, args.resource_zips,
args.android_sdk_root, args.android_sdk_root,
args.lint_gen_dir, args.lint_gen_dir,
args.baseline,
testonly_target=args.testonly, testonly_target=args.testonly,
can_fail_build=args.can_fail_build, can_fail_build=args.can_fail_build,
silent=args.silent) silent=args.silent)
......
...@@ -1787,12 +1787,18 @@ def main(argv): ...@@ -1787,12 +1787,18 @@ def main(argv):
deps_info['jetified_full_jar_classpath'] = sorted( deps_info['jetified_full_jar_classpath'] = sorted(
jetified_full_jar_classpath) jetified_full_jar_classpath)
elif options.type == 'android_app_bundle': elif options.type == 'android_app_bundle':
# bundles require javac_full_classpath to create .aab.jar.info. # bundles require javac_full_classpath to create .aab.jar.info and require
# javac_full_interface_classpath for lint.
javac_full_classpath = set() javac_full_classpath = set()
javac_full_interface_classpath = set()
for d in deps.Direct('android_app_bundle_module'): for d in deps.Direct('android_app_bundle_module'):
javac_full_classpath.update(p for p in d['javac_full_classpath']) javac_full_classpath.update(d['javac_full_classpath'])
javac_full_interface_classpath.update(d['javac_full_interface_classpath'])
javac_full_classpath.add(d['unprocessed_jar_path']) javac_full_classpath.add(d['unprocessed_jar_path'])
javac_full_interface_classpath.add(d['interface_jar_path'])
deps_info['javac_full_classpath'] = sorted(javac_full_classpath) deps_info['javac_full_classpath'] = sorted(javac_full_classpath)
deps_info['javac_full_interface_classpath'] = sorted(
javac_full_interface_classpath)
if options.type in ('android_apk', 'dist_jar', 'android_app_bundle_module', if options.type in ('android_apk', 'dist_jar', 'android_app_bundle_module',
'android_app_bundle'): 'android_app_bundle'):
......
This diff is collapsed.
...@@ -201,6 +201,9 @@ Still reading? ...@@ -201,6 +201,9 @@ Still reading?
<!-- We no longer supply class files to lint. --> <!-- We no longer supply class files to lint. -->
<ignore regexp="No `.class` files were found in project"/> <ignore regexp="No `.class` files were found in project"/>
</issue> </issue>
<!-- We use the same baseline for all our targets. Since some targets run less
code than other targets, there will always be unused suppressions. -->
<issue id="LintBaseline" severity="ignore"/>
<issue id="LogConditional" severity="ignore"/> <issue id="LogConditional" severity="ignore"/>
<issue id="LongLogTag" severity="ignore"/> <issue id="LongLogTag" severity="ignore"/>
<issue id="MergeRootFrame" severity="Error"> <issue id="MergeRootFrame" severity="Error">
...@@ -223,6 +226,8 @@ Still reading? ...@@ -223,6 +226,8 @@ Still reading?
<issue id="MissingQuantity" severity="ignore"/> <issue id="MissingQuantity" severity="ignore"/>
<issue id="MissingRegistered" severity="ignore"/> <issue id="MissingRegistered" severity="ignore"/>
<issue id="MissingSuperCall" severity="Error"> <issue id="MissingSuperCall" severity="Error">
<!-- TODO(wnwen): File bug to fix -->
<ignore regexp="remoting/android/java/src/org/chromium/chromoting/Chromoting.java"/>
<ignore regexp="chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectionToolbar.java"/> <ignore regexp="chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectionToolbar.java"/>
</issue> </issue>
<issue id="MissingTranslation"> <issue id="MissingTranslation">
...@@ -247,6 +252,9 @@ Still reading? ...@@ -247,6 +252,9 @@ Still reading?
<ignore regexp="components/content_capture/android/java/src/org/chromium/components/content_capture"/> <ignore regexp="components/content_capture/android/java/src/org/chromium/components/content_capture"/>
<!-- 1: TODO(crbug.com/1085487): Fix --> <!-- 1: TODO(crbug.com/1085487): Fix -->
<ignore regexp="chrome/android/javatests/src/org/chromium/chrome/browser/directactions/DirectActionTestRule.java"/> <ignore regexp="chrome/android/javatests/src/org/chromium/chrome/browser/directactions/DirectActionTestRule.java"/>
<!-- 2: TODO(wnwen): File bugs to fix -->
<ignore regexp="android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/util/BoundaryInterfaceReflectionUtil.java"/>
<ignore regexp="chrome/android/java/src/org/chromium/chrome/browser/media/PictureInPictureActivity.java"/>
<!-- Endnote: Please specify number of suppressions when adding more --> <!-- Endnote: Please specify number of suppressions when adding more -->
</issue> </issue>
<!-- This warning just adds a lot of false positives. --> <!-- This warning just adds a lot of false positives. -->
...@@ -454,6 +462,8 @@ Still reading? ...@@ -454,6 +462,8 @@ Still reading?
<issue id="VisibleForTests" severity="Error"> <issue id="VisibleForTests" severity="Error">
<ignore regexp="/javatests/"/> <ignore regexp="/javatests/"/>
<ignore regexp="/test/"/> <ignore regexp="/test/"/>
<!-- TODO(wnwen): File bug to fix -->
<ignore regexp="remoting/android/internal/apk/src/org/chromium/chromoting/RemotingApplication.java"/>
<!-- TODO(crbug.com/757124): Remove all these specific Feedback files after underlying issue is resolved --> <!-- TODO(crbug.com/757124): Remove all these specific Feedback files after underlying issue is resolved -->
<!-- Underlying issue is that Android FeedbackOptions.Builder using @VisibleForTesting without 'otherwise='. --> <!-- Underlying issue is that Android FeedbackOptions.Builder using @VisibleForTesting without 'otherwise='. -->
<ignore regexp="clank/java/src/com/google/android/apps/chrome/feedback/FeedbackUtil.java"/> <ignore regexp="clank/java/src/com/google/android/apps/chrome/feedback/FeedbackUtil.java"/>
...@@ -474,4 +484,8 @@ Still reading? ...@@ -474,4 +484,8 @@ Still reading?
<!-- Discussed in crbug.com/1069204, ignoring this class of errors since these are Q+ constants. --> <!-- Discussed in crbug.com/1069204, ignoring this class of errors since these are Q+ constants. -->
<ignore regexp="Must be one of: LineBreaker.BREAK_STRATEGY_SIMPLE, LineBreaker.BREAK_STRATEGY_HIGH_QUALITY, LineBreaker.BREAK_STRATEGY_BALANCED"/> <ignore regexp="Must be one of: LineBreaker.BREAK_STRATEGY_SIMPLE, LineBreaker.BREAK_STRATEGY_HIGH_QUALITY, LineBreaker.BREAK_STRATEGY_BALANCED"/>
</issue> </issue>
<issue id="WrongThread">
<!-- TODO(wnwen): File bug to fix -->
<ignore regexp="chrome/test/android/test_trusted_web_activity/src/org/chromium/chrome/browser/browserservices/TestTrustedWebActivityService.java"/>
</issue>
</lint> </lint>
...@@ -1056,9 +1056,17 @@ if (enable_java_templates) { ...@@ -1056,9 +1056,17 @@ if (enable_java_templates) {
inputs += [ invoker.build_config ] inputs += [ invoker.build_config ]
_rebased_build_config = _rebased_build_config =
rebase_path(invoker.build_config, root_build_dir) rebase_path(invoker.build_config, root_build_dir)
# TODO(wnwen): Remove this baseline once it is empty.
baseline = "//build/android/lint/baseline.xml"
_rebased_baseline = rebase_path(baseline, root_build_dir)
args += [ args += [
"--baseline=$_rebased_baseline",
"--java-sources=@FileArg($_rebased_build_config:deps_info:lint_java_sources)", "--java-sources=@FileArg($_rebased_build_config:deps_info:lint_java_sources)",
"--srcjars=@FileArg($_rebased_build_config:deps_info:lint_srcjars)", "--srcjars=@FileArg($_rebased_build_config:deps_info:lint_srcjars)",
"--srcjars=@FileArg($_rebased_build_config:deps_info:lint_srcjars)",
"--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)",
"--manifest-path=@FileArg($_rebased_build_config:deps_info:lint_android_manifest)", "--manifest-path=@FileArg($_rebased_build_config:deps_info:lint_android_manifest)",
"--resource-sources=@FileArg($_rebased_build_config:deps_info:lint_resource_sources)", "--resource-sources=@FileArg($_rebased_build_config:deps_info:lint_resource_sources)",
"--resource-zips=@FileArg($_rebased_build_config:deps_info:lint_resource_zips)", "--resource-zips=@FileArg($_rebased_build_config:deps_info:lint_resource_zips)",
......
...@@ -2307,7 +2307,8 @@ template("monochrome_test_apk_tmpl") { ...@@ -2307,7 +2307,8 @@ template("monochrome_test_apk_tmpl") {
} }
chrome_test_apk_tmpl("chrome_public_test_apk") { chrome_test_apk_tmpl("chrome_public_test_apk") {
enable_lint = true # TODO(wnwen): Re-enable when new lint failures are fixed.
#enable_lint = true
apk_name = "ChromePublicTest" apk_name = "ChromePublicTest"
android_manifest = chrome_public_test_apk_manifest android_manifest = chrome_public_test_apk_manifest
android_manifest_dep = ":chrome_public_test_apk_manifest" android_manifest_dep = ":chrome_public_test_apk_manifest"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment