Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Android lint-baseline.xml files from flutter/packages #88011

Open
14 of 15 tasks
stuartmorgan opened this issue Aug 11, 2021 · 8 comments
Open
14 of 15 tasks

Remove Android lint-baseline.xml files from flutter/packages #88011

stuartmorgan opened this issue Aug 11, 2021 · 8 comments
Labels
P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-android Android applications specifically team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-android Owned by Android platform team triaged-android Triaged by Android platform team

Comments

@stuartmorgan
Copy link
Contributor

stuartmorgan commented Aug 11, 2021 •
edited

In flutter/plugins#4206 we are turning on Android linting, but it does not attempt to fix existing issues. Instead, all existing issues have been codified into lint-baseline.xml so that any new issues will be caught by CI.

We should go plugin by plugin and fix (or explicitly disable in a targetted way) all of the issues, and remove both lint-baseline.xml, and the corresponding baseline file("lint-baseline.xml") from build.gradle.

The plugins to be cleaned up are:

[Edit: Updated the list with more plugins since https://github.com/flutter/packages/pull/3648 adds more baseline files]
@stuartmorgan stuartmorgan added team Infra upgrades, team productivity, code health, technical debt. See also team: labels. platform-android Android applications specifically plugin P2 Important issues not at the top of the work list labels Aug 11, 2021
@stuartmorgan stuartmorgan mentioned this issue Aug 11, 2021
Merged
11 tasks
@stuartmorgan stuartmorgan added the good first issue Relatively approachable for first-time contributors label Nov 22, 2021
@cyanglaz cyanglaz mentioned this issue Dec 13, 2021
Closed
@vedantkulkarni

This comment was marked as outdated.

Sign in to view
@stuartmorgan

This comment was marked as outdated.

Sign in to view
@vedantkulkarni

This comment was marked as outdated.

Sign in to view
@sdkdeepa

This comment was marked as outdated.

Sign in to view
@stuartmorgan

This comment was marked as outdated.

Sign in to view
@stuartmorgan

This comment was marked as outdated.

Sign in to view
@stuartmorgan stuartmorgan changed the title Remove Android lint-baseline.xml files from flutter/plugins Remove Android lint-baseline.xml files from flutter/packages Mar 6, 2023
@stuartmorgan stuartmorgan added package flutter/packages repository. See also p: labels. and removed plugin labels Mar 9, 2023
@stuartmorgan stuartmorgan mentioned this issue Apr 5, 2023
Merged
11 tasks
@stuartmorgan
Copy link
Contributor Author

stuartmorgan commented Apr 5, 2023 •
edited

For anyone interested in working on this, the process is:

  • Pick a plugin from the list above.
  • Look at the issues in that plugin's android/lint-baseline.xml, and fix any number of the issues there, removing them from the baseline file as they are fixed.
    • For packages that use pigeon, if they aren't using 9.2.3 or later, just updating to the latest version will almost certainly fix some warnings. (For a mass update like this, you can generate a new lint-baseline.xml by deleting the file locally then re-running the lint check.)
    • If you are using Android Studio, see these docs for instruction on enabling the lints in the IDE. By default, most will not show up. (Unfortunately, they must be individually enabled; there is no option to sync them with build.gradle's lint settings.)
  • If you fix all remaining issues, delete the lint-baseline.xml file and remove the baseline file("lint-baseline.xml") entry from android/build.gradle.
    • Fixing all issues in one PR is not required; incremental PRs to reduce the size of lint-baseline.xml are very welcome.
  • Ensure that the lint-android repository tooling command still passes for the plugin.
  • Send a PR with those fixes.

Note to anyone finding this via the good first contribution label: this is marked as such because it's a good first contribution specifically in the Android plugin domain. If you don't have any experience with Java, or just aren't interested in plugins, this is not going to be a good first contribution for you.

(It's not necessary to pre-announce that you are interested in working on this; the chances of two people working on exactly the same fixes at the same time are very low.)

stuartmorgan added a commit to flutter/packages that referenced this issue Apr 7, 2023
@stuartmorgan
[various] Enable warnings as errors, and all warnings, for Android li…
87459c4 
…nt (#3648)

For all Android plugins:
- Enable all warnings for `lint`
- Treat all warnings as errors for `lint`

This significantly increases the scope of issues that we'll catch in CI. To
allow enabling this without having to make tons of fixes first, so that
we get the incremental benefit immediately, this adds new baselines for
all plugins. We can incrementally clean those baselines up over time.
(In practice we haven't prioritized that, but it would be good to start
paying down that technical debt incrementally at some point.)

See flutter/flutter#88011
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue Apr 11, 2023
@stuartmorgan
[url_launcher] Fix Android Java warnings
7a70420 
Fixes Android lint warnings found by the `lint-android` repo tool
command, and removes the baseline file.

Also fixes a few warnings that showed up in Android Studio.

Part of flutter/flutter#88011
@stuartmorgan stuartmorgan mentioned this issue Apr 11, 2023
Merged
11 tasks
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue Apr 11, 2023
@stuartmorgan
[url_launcher] Fix Android Java warnings
79166c5 
Fixes Android lint warnings found by the `lint-android` repo tool
command, and removes the baseline file.

Also fixes a few warnings that showed up in Android Studio.

Part of flutter/flutter#88011
This was referenced Apr 11, 2023
Merged
Merged
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue Apr 13, 2023
@stuartmorgan
[shared_preferences] Remove Android lint baseline
6281bb3 
All of the issues in the baseline file were stale, so it can just be
removed.

Part of flutter/flutter#88011
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue May 17, 2023
@stuartmorgan
[flutter_plugin_android_lifecycle] Fix lints
462249b 
Removes lint-baseline.xml and fixes the resulting warnings.

Part of flutter/flutter#88011
@stuartmorgan stuartmorgan mentioned this issue May 17, 2023
Merged
11 tasks
@stuartmorgan stuartmorgan removed the good first issue Relatively approachable for first-time contributors label May 17, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this issue May 17, 2023
@stuartmorgan
[camera] Fix Java lints in camerax version (#3966)
b5ba619 
Removes list-baseline.xml and fixes all violations. Includes a few minor opporunistic fixes of warnings surfaced in Android Studio but not by the linter.

Part of flutter/flutter#88011
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue May 17, 2023
@stuartmorgan
[in_app_purchase] Fix Java lints
f313b85 
Removes lint-baseline.xml, fixing all resulting warnings.

Also makes some fixes that weren't flagged by the command-line lint, but
were flagged by Android Studio's linter, to reduce the noise in the UI.
This includes all of the conversions of anonymous subclasses to lambdas,
which were entirely tool-generated.

Part of flutter/flutter#88011
stuartmorgan added a commit to stuartmorgan/packages that referenced this issue May 17, 2023
@stuartmorgan
[in_app_purchase] Fix Java lints
141dde9 
Removes lint-baseline.xml, fixing all resulting warnings.

Also makes some fixes that weren't flagged by the command-line lint, but
were flagged by Android Studio's linter, to reduce the noise in the UI.
This includes all of the conversions of anonymous subclasses to lambdas,
which were entirely tool-generated.

Part of flutter/flutter#88011
@stuartmorgan stuartmorgan mentioned this issue May 17, 2023
Merged
11 tasks
auto-submit bot pushed a commit to flutter/packages that referenced this issue May 18, 2023
@stuartmorgan
[flutter_plugin_android_lifecycle] Fix lints (#4030)
b31a128 
Removes lint-baseline.xml and fixes the resulting warnings.

Part of flutter/flutter#88011
@stuartmorgan
Copy link
Contributor Author

Everything is done now except for espresso. That will be non-trivial; there are hundreds of missing nullability violations, and it's all public API, so will require someone to dig into the API structure and make decisions about what the nullability should be (unlike all of the other plugins, where the decision almost always just involved looking at local code).

auto-submit bot pushed a commit to flutter/packages that referenced this issue May 23, 2023
@stuartmorgan
[in_app_purchase] Fix Java lints (#4032)
1a54d76 
Removes lint-baseline.xml, fixing all resulting warnings.

Also makes some fixes that weren't flagged by the command-line lint, but were flagged by Android Studio's linter, to reduce the noise in the UI. This includes all of the conversions of anonymous subclasses to lambdas, which were entirely tool-generated.

Part of flutter/flutter#88011
zhouyuanbo pushed a commit to zhouyuanbo/video_player_android_2.4.8 that referenced this issue Jun 1, 2023
@stuartmorgan
[various] Enable warnings as errors, and all warnings, for Android li…
49dd5a4 
…nt (#3648)

For all Android plugins:
- Enable all warnings for `lint`
- Treat all warnings as errors for `lint`

This significantly increases the scope of issues that we'll catch in CI. To
allow enabling this without having to make tons of fixes first, so that
we get the incremental benefit immediately, this adds new baselines for
all plugins. We can incrementally clean those baselines up over time.
(In practice we haven't prioritized that, but it would be good to start
paying down that technical debt incrementally at some point.)

See flutter/flutter#88011
zhouyuanbo pushed a commit to zhouyuanbo/video_player_android_2.4.8 that referenced this issue Jun 1, 2023
@stuartmorgan
[video_player] Fix Android lints (#3886)
fc094c4 
Removes lint-baseline.xml and fixes all exposed lints. Includes an update of Pigeon to the latest version, to pick up warning fixes in generated code.

Part of flutter/flutter#88011
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-android Owned by Android platform team triaged-android Triaged by Android platform team labels Jul 8, 2023
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[various] Enable warnings as errors, and all warnings, for Android li…
fb4487e 
…nt (flutter#3648)

For all Android plugins:
- Enable all warnings for `lint`
- Treat all warnings as errors for `lint`

This significantly increases the scope of issues that we'll catch in CI. To
allow enabling this without having to make tons of fixes first, so that
we get the incremental benefit immediately, this adds new baselines for
all plugins. We can incrementally clean those baselines up over time.
(In practice we haven't prioritized that, but it would be good to start
paying down that technical debt incrementally at some point.)

See flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[path_provider] Fix Android lint warnings (flutter#3706)
fa783a5 
Removes the lint-baseline.xml file, and fixes all issues. Includes updating to the latest version of Pigeon to pick up the lint fixes for the generated file.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@bparrishMines @nploi
[camera_android] Fix Android lint warnings (flutter#3716)
c94270b 
Removes the lint-baseline.xml file, and fixes all issues.

The `KotlinPropertyAccess` doesn't seem to work well with generics. For example,

```java
class CameraFeature<T> {
  T getValue();
}

class ExposureFeature extends CameraFeature<Exposure> {
  @OverRide
  Exposure getValue() {
    ...
  }
}
```

The `ExposureFeature.getValue` causes the lint warning. I went ahead and suppressed this for all `CameraFeatures`.

I also added `@Nonnull` or `@Nullable` to some private fields that weren't required. This helped deduce whether a value was always nonnull or not.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[google_sign_in] Fix Android Java warnings (flutter#3762)
15fb3aa 
Removes lint-baseline.xml, and fixes all resulting warnings. Also fixes some warnings that only show up in the AS linter as opportunistic fixes.

All anonymous object->lambda conversions were auto-generated by the AS auto-fix for those warnings.

Fixed an edge case bug found during warning fixes; the Dart `getTokens` call has an argument that was (I assume accidentally) declared as `bool? shouldRecoverAuth = true` instead of `bool shouldRecoverAuth = true`, which means that while it defaults to `true` if not passed, it can be explicitly passed `null` and will keep that value. This was being passed directly into the method channel, and on the Java side was extracted as a `boolean` which would throw an exception on null.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@mgmix @nploi
[google_maps_flutter_android] Fix Android lint warnings (flutter#3751)
7de6c1a 
Fixed the Android lint warning shown in `lint-baseline.xml` and removed the baseline.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@mgmix @nploi
[local_auth_android] Fix Android lint warnings (flutter#3764)
72c8635 
Removed lint-baseline.xml and fixed all warnings.
The removal is mainly for unused resources that were identified by the `./gradlew lint` check.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[quick_actions] Fix Android lint issues (flutter#3885)
b21f0c6 
Removes lint-baseline.xml and fixes all warnings.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[video_player] Fix Android lints (flutter#3886)
f97d893 
Removes lint-baseline.xml and fixes all exposed lints. Includes an update of Pigeon to the latest version, to pick up warning fixes in generated code.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[image_picker] Fix Android lints (flutter#3887)
7420b55 
Removes the lint-baseline.xml and fixes the resulting issues. Also does some Andoid-Studio-suggested auto-conversions of anonymous classes to lambdas.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[camera] Fix Android lint issues (flutter#3993)
b55e1a4 
Fixes the new lints that came with a recent AGP update, and removes the baseline file.

Fixes flutter/flutter#126710
Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[camera] Fix Java lints in camerax version (flutter#3966)
b7a857f 
Removes list-baseline.xml and fixes all violations. Includes a few minor opporunistic fixes of warnings surfaced in Android Studio but not by the linter.

Part of flutter/flutter#88011
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
@stuartmorgan @nploi
[flutter_plugin_android_lifecycle] Fix lints (flutter#4030)
48df69a 
Removes lint-baseline.xml and fixes the resulting warnings.

Part of flutter/flutter#88011
tony-hristov pushed a commit to tony-hristov/webview_flutter that referenced this issue Sep 12, 2023
@stuartmorgan
[various] Enable warnings as errors, and all warnings, for Android li…
730ed16 
…nt (#3648)

For all Android plugins:
- Enable all warnings for `lint`
- Treat all warnings as errors for `lint`

This significantly increases the scope of issues that we'll catch in CI. To
allow enabling this without having to make tons of fixes first, so that
we get the incremental benefit immediately, this adds new baselines for
all plugins. We can incrementally clean those baselines up over time.
(In practice we haven't prioritized that, but it would be good to start
paying down that technical debt incrementally at some point.)

See flutter/flutter#88011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-android Android applications specifically team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-android Owned by Android platform team triaged-android Triaged by Android platform team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@stuartmorgan @Hixie @sdkdeepa @vedantkulkarni @gmackall @TahaTesser