Skip to content

Enable Java lints for the plugins/packages repos #119836

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.

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

Closed
bparrishMines opened this issue Feb 2, 2023 · 10 comments
Closed

Enable Java lints for the plugins/packages repos #119836

bparrishMines opened this issue Feb 2, 2023 · 10 comments
Assignees
Labels
c: contributor-productivity Team-specific productivity, code health, technical debt. P1 High-priority issues at the top of the work list package flutter/packages repository. See also p: labels. platform-android Android applications specifically

Comments

@bparrishMines
Copy link
Contributor

bparrishMines commented Feb 2, 2023

A linter runs for the Dart code and Objective-C/Swift code in the flutter/plugins and flutter/packages repos.

This issue tracks enabling it for Java and Kotlin.

We should be able to just use the command provided by gradle:
See https://developer.android.com/studio/write/lint

gradlew lint
./gradlew lint

I would imagine we would initially need a config file to exclude plugins and fix the lint warnings incrementally.

cc @camsim99 @reidbaker @stuartmorgan FYI

@bparrishMines bparrishMines added platform-android Android applications specifically plugin package flutter/packages repository. See also p: labels. P1 High-priority issues at the top of the work list labels Feb 2, 2023
@bparrishMines
Copy link
Contributor Author

@stuartmorgan Is this something we could do before or after #113764?

@bparrishMines bparrishMines removed the P1 High-priority issues at the top of the work list label Feb 2, 2023
@stuartmorgan-g
Copy link
Contributor

I we do it before, we just need to make sure it's added in parallel to both repos so we don't re-diverge their test set.

I would imagine we would initially need a config file to exclude plugins and fix the lint warnings incrementally.

We already have lint_baseline.xml set up as part of uploading build warning output, so hopefully we could just update that.

@stuartmorgan-g stuartmorgan-g added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Feb 2, 2023
@stuartmorgan-g
Copy link
Contributor

Would this address #91868 for Android, or are lint warnings and build warnings disjoint?

@reidbaker
Copy link
Contributor

I am pro making this happen.

@reidbaker reidbaker added the P1 High-priority issues at the top of the work list label Feb 2, 2023
@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Feb 23, 2023

We already have lint_baseline.xml set up as part of uploading build warning output

... which we have because I set up a command over a year ago that runs this lint command (lint-android). I forgot that's what it was doing 🤦🏻

Closing as a duplicate of #87071. I'll look into why some warnings aren't being flagged by this, like flutter/packages#3273. Maybe it's just the exclusions, and what we need is a push to remove exclusions that were set up when we turned this on?

@stuartmorgan-g
Copy link
Contributor

I'll look into why some warnings aren't being flagged by this, like flutter/packages#3273. Maybe it's just the exclusions, and what we need is a push to remove exclusions that were set up when we turned this on?

It looks like having warnings output doesn't cause it to return a non-zero exit code. Re-opening while I investigate further.

@stuartmorgan-g stuartmorgan-g self-assigned this Feb 23, 2023
@stuartmorgan-g
Copy link
Contributor

Also, while not relevant to it not failing CI, it appears that running the lint command twice only outputs warnings the first time, which is definitely not a behavior we want.

@stuartmorgan-g
Copy link
Contributor

It looks like having warnings output doesn't cause it to return a non-zero exit code. Re-opening while I investigate further.

Investigating further: I'm not crazy thinking I had to set up baselines to get things to pass when I first did this, I just wasn't looking at the output closely enough. The output I was seeing was:

> Task :camera_android:lintDebug
Lint found no errors or warnings

2 errors/warnings were listed in the baseline file (/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/lint-baseline.xml) but not found in the project; perhaps they have been fixed?

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/7.0.2/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 1s
52 actionable tasks: 52 executed
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:158: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_HIGH);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:162: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_2160P);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:166: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_1080P);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:170: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_720P);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:174: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_480P);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:178: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_QVGA);
                                 ^
/Users/stuartmorgan/src/packages/packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:182: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_LOW);
                                 ^
7 warnings

I missed the Lint found no errors or warnings bit. So those warnings are being dumped to the console when I run lint as part of the overall gradle run, but aren't part of the lint step itself. That may also explain why it doesn't output anything when run a second time; it's not the step actually being targetted.

@stuartmorgan-g
Copy link
Contributor

Okay, it looks like what's happening here is that the Gradle lint is different from javac lints, and the deprecation warnings are coming from the latter. So this is a duplicate, and the above would be #91868

@github-actions
Copy link

github-actions bot commented Mar 9, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: contributor-productivity Team-specific productivity, code health, technical debt. P1 High-priority issues at the top of the work list package flutter/packages repository. See also p: labels. platform-android Android applications specifically
Projects
None yet
Development

No branches or pull requests

3 participants