| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 1 | # Lint |
| 2 | |
| Andrew Grieve | ec828460 | 2023-10-16 15:53:25 | [diff] [blame] | 3 | [Android Lint] is [one of the static analysis tools] that Chromium uses to catch |
| 4 | possible issues in Java code. |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 5 | |
| Andrew Grieve | ec828460 | 2023-10-16 15:53:25 | [diff] [blame] | 6 | This is a [list of checks] that you might encounter. |
| 7 | |
| 8 | [Android Lint]: https://googlesamples.github.io/android-custom-lint-rules/book.md.html |
| 9 | [one of the static analysis tools]: static_analysis.md |
| 10 | [list of checks]: https://googlesamples.github.io/android-custom-lint-rules/checks/index.md.html |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 11 | |
| 12 | [TOC] |
| 13 | |
| 14 | ## How Chromium uses lint |
| 15 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 16 | Chromium only runs lint on apk or bundle targets that explicitly set |
| Nate Fischer | 80e5d0b | 2024-04-10 14:38:05 | [diff] [blame] | 17 | `enable_lint = true`. You can run lint by compiling the apk or bundle target |
| 18 | with ninja; once the code finishes compiling, ninja will automatically run lint |
| 19 | on the code. |
| 20 | |
| 21 | Some example targets that have lint enabled are: |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 22 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 23 | - `//chrome/android:monochrome_public_bundle` |
| 24 | - `//android_webview/support_library/boundary_interfaces:boundary_interface_example_apk` |
| Nate Fischer | 80e5d0b | 2024-04-10 14:38:05 | [diff] [blame] | 25 | - Other targets with `enable_lint` enabled: https://source.chromium.org/search?q=lang:gn%20enable_lint%5C%20%3D%5C%20true&ss=chromium |
| 26 | |
| 27 | If you think lint is not running and already verified your GN |
| 28 | target has `enable_lint = true`, then you can double check that |
| 29 | `android_static_analysis` is set to `"on"` (this is the default value): |
| 30 | |
| 31 | ```shell |
| 32 | $ gn args out/Default --list=android_static_analysis |
| 33 | android_static_analysis |
| 34 | Current value (from the default) = "on" |
| 35 | From //build/config/android/config.gni:85 |
| 36 | ``` |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 37 | |
| 38 | ## My code has a lint error |
| 39 | |
| 40 | If lint reports an issue in your code, there are several possible remedies. |
| 41 | In descending order of preference: |
| 42 | |
| 43 | ### Fix it |
| 44 | |
| 45 | While this isn't always the right response, fixing the lint error or warning |
| 46 | should be the default. |
| 47 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 48 | ### Suppress it locally |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 49 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 50 | Java provides an annotation, |
| 51 | [`@SuppressWarnings`](https://developer.android.com/reference/java/lang/SuppressWarnings), |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 52 | that tells lint to ignore the annotated element. It can be used on classes, |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 53 | constructors, methods, parameters, fields, or local variables, though usage in |
| 54 | Chromium is typically limited to the first three. You do not need to import it |
| 55 | since it is in the `java.lang` package. |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 56 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 57 | Like many suppression annotations, `@SuppressWarnings` takes a value that tells |
| 58 | **lint** what to ignore. It can be a single `String`: |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 59 | |
| 60 | ```java |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 61 | @SuppressWarnings("NewApi") |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 62 | public void foo() { |
| 63 | a.methodThatRequiresHighSdkLevel(); |
| 64 | } |
| 65 | ``` |
| 66 | |
| 67 | It can also be a list of `String`s: |
| 68 | |
| 69 | ```java |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 70 | @SuppressWarnings({ |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 71 | "NewApi", |
| 72 | "UseSparseArrays" |
| 73 | }) |
| 74 | public Map<Integer, FakeObject> bar() { |
| 75 | Map<Integer, FakeObject> shouldBeASparseArray = new HashMap<Integer, FakeObject>(); |
| 76 | another.methodThatRequiresHighSdkLevel(shouldBeASparseArray); |
| 77 | return shouldBeASparseArray; |
| 78 | } |
| 79 | ``` |
| 80 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 81 | For resource xml files you can use `tools:ignore`: |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 82 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 83 | ```xml |
| 84 | <?xml version="1.0" encoding="utf-8"?> |
| 85 | <resources xmlns:tools="http://schemas.android.com/tools"> |
| 86 | <!-- TODO(crbug/###): remove tools:ignore once these colors are used --> |
| 87 | <color name="hi" tools:ignore="NewApi,UnusedResources">@color/unused</color> |
| 88 | </resources> |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 89 | ``` |
| 90 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 91 | The examples above are the recommended ways of suppressing lint warnings. |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 92 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 93 | ### Suppress it in a `lint-suppressions.xml` file |
| 94 | |
| 95 | **lint** can be given a per-target XML configuration file containing warnings or |
| 96 | errors that should be ignored. Each target defines its own configuration file |
| 97 | via the `lint_suppressions_file` gn variable. It is usually defined near its |
| 98 | `enable_lint` gn variable. |
| 99 | |
| 100 | These suppressions files should only be used for temporarily ignoring warnings |
| 101 | that are too hard (or not possible) to suppress locally, and permanently |
| 102 | ignoring warnings only for this target. To permanently ignore a warning for all |
| 103 | targets, add the warning to the `_DISABLED_ALWAYS` list in |
| John Palmer | 58f5d9f | 2021-05-19 18:58:29 | [diff] [blame] | 104 | [build/android/gyp/lint.py](https://source.chromium.org/chromium/chromium/src/+/main:build/android/gyp/lint.py). |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 105 | Disabling globally makes lint a bit faster. |
| 106 | |
| Peter Wen | 397a12c2 | 2020-11-21 01:52:08 | [diff] [blame] | 107 | The exception to the above rule is for warnings that affect multiple languages. |
| 108 | Feel free to suppress those in lint-suppressions.xml files since it is not |
| 109 | practical to suppress them in each language file and it is a lot of extra bloat |
| 110 | to list out every language for every violation in lint-baseline.xml files. |
| 111 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 112 | Here is an example of how to structure a suppressions XML file: |
| 113 | |
| 114 | ```xml |
| 115 | <?xml version="1.0" encoding="utf-8" ?> |
| 116 | <lint> |
| 117 | <!-- Chrome is a system app. --> |
| 118 | <issue id="ProtectedPermissions" severity="ignore"/> |
| 119 | <issue id="UnusedResources"> |
| 120 | <!-- 1 raw resources are accessed by URL in various places. --> |
| 121 | <ignore regexp="gen/remoting/android/.*/res/raw/credits.*"/> |
| 122 | <!-- TODO(crbug.com/###): Remove the following line. --> |
| 123 | <ignore regexp="The resource `R.string.soon_to_be_used` appears to be unused"/> |
| 124 | </issue> |
| 125 | </lint> |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 126 | ``` |
| 127 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 128 | ## What are `lint-baseline.xml` files for? |
| jbudorick | 8d4d974 | 2016-04-26 04:47:10 | [diff] [blame] | 129 | |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 130 | Baseline files are to help us introduce new lint warnings and errors without |
| 131 | blocking on fixing all our existing code that violate these new errors. Since |
| 132 | they are generated files, they should **not** be used to suppress lint warnings. |
| 133 | One of the approaches above should be used instead. Eventually all the errors in |
| 134 | baseline files should be either fixed or ignored permanently. |
| 135 | |
| Peter Wen | bf73bf5 | 2022-11-07 18:06:56 | [diff] [blame] | 136 | Most devs do not need to update baseline files and should not need the script |
| 137 | below. Occasionally when making large build configuration changes it may be |
| 138 | necessary to update baseline files (e.g. increasing the min_sdk_version). |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 139 | |
| 140 | Baseline files are defined via the `lint_baseline_file` gn variable. It is |
| Peter Wen | bf73bf5 | 2022-11-07 18:06:56 | [diff] [blame] | 141 | usually defined near a target's `enable_lint` gn variable. To regenerate all |
| 142 | baseline files, run: |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 143 | |
| Peter Wen | bf73bf5 | 2022-11-07 18:06:56 | [diff] [blame] | 144 | ``` |
| 145 | $ third_party/android_build_tools/lint/rebuild_baselines.py |
| 146 | ``` |
| Peter Wen | 24b1674 | 2020-11-12 22:39:27 | [diff] [blame] | 147 | |
| Peter Wen | bf73bf5 | 2022-11-07 18:06:56 | [diff] [blame] | 148 | This script will also update baseline files in downstream //clank if needed. |
| 149 | Since downstream and upstream use separate lint binaries, it is usually safe |
| Andrew Grieve | ec828460 | 2023-10-16 15:53:25 | [diff] [blame] | 150 | to simply land the update CLs in any order. |
