[flutter_plugin_tools] Add a command to lint Android code

[stuartmorgan]

Adds a new lint-android command to run gradlew lint on Android plugins.

Also standardizes the names of the Cirrus tasks that run all the build and platform-specific (i.e., not Dart unit test) tests for each platform, as they were getting unnecessarily long and complex in some cases.

Fixes flutter/flutter#87071

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
• All existing and new tests are passing.

[stuartmorgan]

This is a WIP since the new command will fail on many plugins. @blasten How do you want to proceed here:

1. --exclude all plugins that fail, and enable them as they are made to work?
2. Configure all plugins to not fail on error for now via the XML configuration, and remove that as they are fixed up?
3. Not add it to CI at all yet, get things working locally, then turn it on later?
4. ???

My preference is (1) since it uses the established place for tracking what's currently not turned on, but I can see a good argument for (2) as well given that we'd have output on the bots to work from right away. (We'll need an issues tracking getting it fully running for whatever option we choose, of course.)

Other open questions:

• Are there baseline options you want to set for all plugins before turning this on, or do you want to just make adjusting that part of fully opting each plugin into the run?
• Are you fine with punting on the XML parser for now, and just showing the partial output that we get by default, and expecting people to run it locally for full details? (There's also a middle-ground option where we parse out the name of the file but rather than parsing it and displaying it prettily, just dump the XML to stdout, so there's complete output in CI.)

[blasten]

If moved to Github Actions, it could upload the generated HTML as an attachment using the GitHub API

[stuartmorgan]

Looks like Cirrus could do it too: https://cirrus-ci.org/guide/writing-tasks/#artifacts-instruction. In fact: https://cirrus-ci.org/examples/#android-lint looks pretty interesting.

I can see how easy it will be to manage storage; we'd want them to auto-expire, ideally.

[blasten]

I think it only requires flutter pub get. Could this command invoke it before running ./gradlew?

[stuartmorgan]

This does not appear to be the case. Maybe flutter should have a specific command to generate files like this without actually doing builds?

[blasten]

do you remember what file was missing?

[stuartmorgan]

Yep, gradlew.

If you git clean a Flutter app (which removes android/gradlew), then flutter pub get or flutter packages get there's still no android/gradlew.

[blasten]

LGTM other than the minor comments

[blasten]

Configure all plugins to not fail on error for now via the XML configuration, and remove that as they are fixed up?

abortOnError false in build.gradle https://developer.android.com/studio/write/lint#gradle wdyt?

Are there baseline options you want to set for all plugins before turning this on, or do you want to just make adjusting that part of fully opting each plugin into the run?

Ideally, plugins don't use baseline files since these are meant to hide existing issues.

Are you fine with punting on the XML parser for now, and just showing the partial output that we get by default, and expecting people to run it locally for full details?

For the time being, could it dump the xml content to stdout?

[stuartmorgan]

Configure all plugins to not fail on error for now via the XML configuration, and remove that as they are fixed up?

abortOnError false in build.gradle https://developer.android.com/studio/write/lint#gradle wdyt?

Are there baseline options you want to set for all plugins before turning this on, or do you want to just make adjusting that part of fully opting each plugin into the run?

Ideally, plugins don't use baseline files since these are meant to hide existing issues.

Sorry, by baseline options I meant "are there any non-default lint settings we want to set". E.g., I don't know if there are things that aren't checked by default but we would want to enable.

But looking at baseline files more now that I know of them, that's strictly better than abortOnError false because we'll immediately start getting CI telling us about new failures for all plugins, rather than us having to go back and fix each existing plugin completely before getting any benefit at all for that plugin (since if regressions don't turn CI red we will, realistically, never notice them). So I'll see about setting those up as the interim solution, with an issue filed for removing them.

Are you fine with punting on the XML parser for now, and just showing the partial output that we get by default, and expecting people to run it locally for full details?

For the time being, could it dump the xml content to stdout?

I'll see how the parsed-by-Cirrus options pans out, and fall back to this if it doesn't give us what we need. Hopefully I'll have a new version of the PR up later today.

[stuartmorgan]

@blasten Okay, I think this is ready for a full review now. Changes:

• Extracted a little helper file for Gradle commands (as I previously did with Xcode) and refactored existing things to use it (as well as adding unit tests for it). It correctly handles the Windows vs non-Windows Gradle wrapper file name.
• Added "GradleDependency" to the ignore list in all plugins; it would be a regular source of out-of-band failures, which we absolutely don't want more of. (You might want to consider setting up a manual process of occasionally doing a trial PR that strips that out of everything and runs CI, to quickly get a list of everything that has dependencies that we might want to update.)
• Changed the lint command to only run lint for the plugin module itself, not transitively for everything, to avoid getting failures for things that aren't actually part of the plugin (like integration_test has lint issues for older targets flutter#88005)
  • I looked at adding app as well, but it seemed like the things it was flagging were likely not that interesting given that they are example apps. You may want to take a look as a follow-up and see if you do want to enable them.
• Added baseline files for plugins where necessary to make things green, and filed Remove Android lint-baseline.xml files from flutter/packages flutter#88011 for cleaning them up
• Added the Cirrus command to upload, parse, and display the lint results as part of the run summary UI. Seems to work pretty well (you can't really see much now because of the exclusion files, but https://github.com/flutter/plugins/runs/3293114401 is an example from before I did that).

[blasten]

But looking at baseline files more now that I know of them, that's strictly better than abortOnError false

Sounds good

what is the awesome integration that shows linter warnings as GitHub comments in the PR?

[blasten]

LGTM. This is great!