Skip to content

GitLab

T
tangled
Commit 24b16741 authored by Peter Wen's avatar Peter Wen Committed by Commit Bot
Browse files
Options

Android: Update lint.md and remove default files 

These defaults are no longer necessary now that downstream is using its
own suppressions and baseline files. This makes sure that all targets
use lint the way that these updated docs say.

Bug: 1139957
Change-Id: I1ce9d79e990d472c719cdde688f2ca34a8100ac9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533575
Commit-Queue: Peter Wen <wnwen@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: default avatarMohamed Heikal <mheikal@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827002}
parent f7c82132
Show whitespace changes
Inline Side-by-side
Showing with 90 additions and 66 deletions
build/android/docs/lint.md
View file @ 24b16741
# Lint # Lint
Android's [**lint**](http://developer.android.com/tools/help/lint.html) is a static Android's [**lint**](https://developer.android.com/tools/help/lint.html) is a
analysis tool that Chromium uses to catch possible issues in Java code. static analysis tool that Chromium uses to catch possible issues in Java code.
This is a list of [**checks**](http://tools.android.com/tips/lint-checks) that
you might encounter.
[TOC] [TOC]
## How Chromium uses lint ## How Chromium uses lint
Chromium runs lint on a per-target basis for all targets using any of the Chromium only runs lint on apk or bundle targets that explicitly set
following templates if they are marked as Chromium code (i.e., `enable_lint = true`. Some example targets that have this set are:
`chromium_code = true`):
- `android_apk`
- `android_library`
- `instrumentation_test_apk`
- `unittest_apk`
Chromium also runs lint on a per-target basis for all targets using any of the
following templates if they are marked as Chromium code and they support
Android (i.e., `supports_android = true`):
- `java_library` - `//chrome/android:monochrome_public_bundle`
- `//android_webview/support_library/boundary_interfaces:boundary_interface_example_apk`
This is implemented in the - `//remoting/android:remoting_apk`
[`android_lint`](https://source.chromium.org/chromium/chromium/src/+/HEAD:build/config/android/internal_rules.gni)
gn template.
## My code has a lint error ## My code has a lint error
...@@ -36,19 +27,20 @@ In descending order of preference: ...@@ -36,19 +27,20 @@ In descending order of preference:
While this isn't always the right response, fixing the lint error or warning While this isn't always the right response, fixing the lint error or warning
should be the default. should be the default.
### Suppress it in code ### Suppress it locally
Android provides an annotation, Java provides an annotation,
[`@SuppressLint`](http://developer.android.com/reference/android/annotation/SuppressLint.html), [`@SuppressWarnings`](https://developer.android.com/reference/java/lang/SuppressWarnings),
that tells lint to ignore the annotated element. It can be used on classes, that tells lint to ignore the annotated element. It can be used on classes,
constructors, methods, parameters, fields, or local variables, though usage constructors, methods, parameters, fields, or local variables, though usage in
in Chromium is typically limited to the first three. Chromium is typically limited to the first three. You do not need to import it
since it is in the `java.lang` package.
Like many suppression annotations, `@SuppressLint` takes a value that tells **lint** Like many suppression annotations, `@SuppressWarnings` takes a value that tells
what to ignore. It can be a single `String`: **lint** what to ignore. It can be a single `String`:
```java ```java
@SuppressLint("NewApi") @SuppressWarnings("NewApi")
public void foo() { public void foo() {
a.methodThatRequiresHighSdkLevel(); a.methodThatRequiresHighSdkLevel();
} }
...@@ -57,7 +49,7 @@ public void foo() { ...@@ -57,7 +49,7 @@ public void foo() {
It can also be a list of `String`s: It can also be a list of `String`s:
```java ```java
@SuppressLint({ @SuppressWarnings({
"NewApi", "NewApi",
"UseSparseArrays" "UseSparseArrays"
}) })
...@@ -68,24 +60,76 @@ public Map<Integer, FakeObject> bar() { ...@@ -68,24 +60,76 @@ public Map<Integer, FakeObject> bar() {
} }
``` ```
This is the preferred way of suppressing warnings in a limited scope. For resource xml files you can use `tools:ignore`:
### Suppress it in the suppressions XML file
**lint** can be given an XML configuration containing warnings or errors that ```xml
should be ignored. Chromium's lint suppression XML file can be found in <?xml version="1.0" encoding="utf-8"?>
[`build/android/lint/suppressions.xml`](https://chromium.googlesource.com/chromium/src/+/master/build/android/lint/suppressions.xml). <resources xmlns:tools="http://schemas.android.com/tools">
It can be updated to suppress current warnings by running: <!-- TODO(crbug/###): remove tools:ignore once these colors are used -->
<color name="hi" tools:ignore="NewApi,UnusedResources">@color/unused</color>
</resources>
```
```bash The examples above are the recommended ways of suppressing lint warnings.
$ python build/android/lint/suppress.py <result.xml file>
### Suppress it in a `lint-suppressions.xml` file
**lint** can be given a per-target XML configuration file containing warnings or
errors that should be ignored. Each target defines its own configuration file
via the `lint_suppressions_file` gn variable. It is usually defined near its
`enable_lint` gn variable.
These suppressions files should only be used for temporarily ignoring warnings
that are too hard (or not possible) to suppress locally, and permanently
ignoring warnings only for this target. To permanently ignore a warning for all
targets, add the warning to the `_DISABLED_ALWAYS` list in
[build/android/gyp/lint.py](https://source.chromium.org/chromium/chromium/src/+/master:build/android/gyp/lint.py).
Disabling globally makes lint a bit faster.
Here is an example of how to structure a suppressions XML file:
```xml
<?xml version="1.0" encoding="utf-8" ?>
<lint>
<!-- Chrome is a system app. -->
<issue id="ProtectedPermissions" severity="ignore"/>
<issue id="UnusedResources">
<!-- 1 raw resources are accessed by URL in various places. -->
<ignore regexp="gen/remoting/android/.*/res/raw/credits.*"/>
<!-- TODO(crbug.com/###): Remove the following line. -->
<ignore regexp="The resource `R.string.soon_to_be_used` appears to be unused"/>
</issue>
</lint>
``` ```
e.g., to suppress lint errors found in `media_java`: ## What are `lint-baseline.xml` files for?
```bash Baseline files are to help us introduce new lint warnings and errors without
$ python build/android/lint/suppress.py out/Debug/gen/media/base/android/media_java__lint/result.xml blocking on fixing all our existing code that violate these new errors. Since
``` they are generated files, they should **not** be used to suppress lint warnings.
One of the approaches above should be used instead. Eventually all the errors in
baseline files should be either fixed or ignored permanently.
The following are some common scenarios where you may need to update baseline
files.
### I updated `cmdline-tools` and now there are tons of new errors!
This happens every time lint is updated, since lint is provided by
`cmdline-tools`.
Baseline files are defined via the `lint_baseline_file` gn variable. It is
usually defined near a target's `enable_lint` gn variable. To regenerate the
baseline file, delete it and re-run the lint target. The command will fail, but
the baseline file will have been generated.
This may need to be repeated for all targets that have set `enable_lint = true`,
including downstream targets. Downstream baseline files should be updated and
first to avoid build breakages. Each target has its own `lint_baseline_file`
defined and so all these files can be removed and regenerated as needed.
**This mechanism should only be used for disabling warnings across the entire code base; class-specific lint warnings should be disabled inline.** ### I updated `library X` and now there are tons of new errors!
This is usually because `library X`'s aar contains custom lint checks and/or
custom annotation definition. Follow the same procedure as updates to
`cmdline-tools`.
chrome/android/BUILD.gn
View file @ 24b16741
...@@ -2928,7 +2928,9 @@ template("monochrome_or_trichrome_public_bundle_tmpl") { ...@@ -2928,7 +2928,9 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
"include_32_bit_webview", "include_32_bit_webview",
"include_64_bit_webview", "include_64_bit_webview",
"is_64_bit_browser", "is_64_bit_browser",
"lint_baseline_file",
"lint_min_sdk_version", "lint_min_sdk_version",
"lint_suppressions_file",
"static_library_provider", "static_library_provider",
"static_library_synchronized_proguard", "static_library_synchronized_proguard",
"expected_libs_and_assets", "expected_libs_and_assets",
...@@ -2970,6 +2972,8 @@ monochrome_or_trichrome_public_bundle_tmpl("monochrome_public_bundle") { ...@@ -2970,6 +2972,8 @@ monochrome_or_trichrome_public_bundle_tmpl("monochrome_public_bundle") {
# lowest shipping minSdkVersion to catch all potential NewApi errors. # lowest shipping minSdkVersion to catch all potential NewApi errors.
lint_min_sdk_version = default_min_sdk_version lint_min_sdk_version = default_min_sdk_version
enable_lint = true enable_lint = true
lint_baseline_file = "expectations/lint-baseline.xml"
lint_suppressions_file = "expectations/lint-suppressions.xml"
bundle_suffix = "" bundle_suffix = ""
if (android_64bit_target_cpu) { if (android_64bit_target_cpu) {
......
chrome/android/chrome_public_apk_tmpl.gni
View file @ 24b16741
...@@ -373,18 +373,6 @@ template("chrome_public_common_apk_or_module_tmpl") { ...@@ -373,18 +373,6 @@ template("chrome_public_common_apk_or_module_tmpl") {
deps += [ "//chrome/android:chrome_all_java" ] deps += [ "//chrome/android:chrome_all_java" ]
} }
# Use a consistent baseline so that it is easy to regenerate by deleting the
# file and re-building the "android_lint" target.
if (defined(enable_lint) && enable_lint) {
if (!defined(lint_baseline_file)) {
lint_baseline_file = "//chrome/android/expectations/lint-baseline.xml"
}
if (!defined(lint_suppressions_file)) {
lint_suppressions_file =
"//chrome/android/expectations/lint-suppressions.xml"
}
}
# Prefer to add this data_dep on the final target instead of java targets # Prefer to add this data_dep on the final target instead of java targets
# like chrome_all_java so that all other targets can build in parallel with # like chrome_all_java so that all other targets can build in parallel with
# lint. # lint.
......
chrome/android/modules/chrome_bundle_tmpl.gni
View file @ 24b16741
...@@ -226,18 +226,6 @@ template("chrome_bundle") { ...@@ -226,18 +226,6 @@ template("chrome_bundle") {
is_multi_abi = _is_multi_abi is_multi_abi = _is_multi_abi
validate_services = _enable_chrome_module validate_services = _enable_chrome_module
# Use a consistent baseline so that it is easy to regenerate by deleting the
# file and re-building the "android_lint" target.
if (defined(enable_lint) && enable_lint) {
if (!defined(lint_baseline_file)) {
lint_baseline_file = "//chrome/android/expectations/lint-baseline.xml"
}
if (!defined(lint_suppressions_file)) {
lint_suppressions_file =
"//chrome/android/expectations/lint-suppressions.xml"
}
}
# List of DFMs that are installed by default by wrapper scripts, to make # List of DFMs that are installed by default by wrapper scripts, to make
# testing easier. This removes the need to manually specify, e.g., # testing easier. This removes the need to manually specify, e.g.,
# "-m dev_ui" on every install or run. # "-m dev_ui" on every install or run.
......
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