New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 599052 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

@IntDef is not enforced

Project Member Reported by dgn@chromium.org, Mar 30 2016

Issue description

@IntDef({Foo.BAR})
@Retention(RetentionPolicy.SOURCE)
public @interface Foo {
   public static int BAR = 0;
}

public static void doThing(@Foo int foo) {...}

doThing(42);  // <-- Builds just fine!

I tried both GYP and GN Incremental, the above works fine, while I expect to get an error or at list a warning for not using a compatible value when calling doThing.

 
This is almost certainly due to Android's lint too claiming to support .jar files, but its source code just has:
if (jar) {
  // TODO
}

Should be fixed if we first extract them.
Turns out "almost certainly" was slightly exaggerated.

Looks like actually that .jar is working just fine (not sure to what extend those //TODO blocks affect things...

E.g.:
org/chromium/chrome/browser/ChromeLifetimeController.class Do not call `setSeed()` on a `SecureRandom` with a fixed seed: it is not secure. Use `getSeed()`.: SecureRandom [warning]
org/chromium/chrome/browser/ChromeLifetimeController.class Potentially insecure random numbers on Android 4.3 and older. Read https://android-developers.blogspot.com/2013/08/some-securerandom-thoughts.html for more info.: TrulyRandom [warning]

I was also able to trigger:
../../../../../../../../../tmp/tmpn5zqWa/0/ChromeLifetimeController.java:43 Constants `BAZ2` and `BAZ` specify the same exact value (1); this is usually a cut & paste or merge error: UniqueConstants [warning]
    @IntDef({BAR, BAZ, BAZ2})
                       ~~~~

Which is based on parsing .java. The IntDef checks are all .java parsing-based.

I tested @MainThread vs @WorkerThread, @IntDef with bad value, as well as activity.getSharedPreferences("a", 0).edit().commit();

None of these trigger warnings. They all use visitMethodInvocation(), so perhaps there's an issue with lint's command-line parser not calling these callbacks?

For reference, code is here: https://android.googlesource.com/platform/tools/base/+/master/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks

Comment 3 by dgn@chromium.org, Mar 31 2016

Interesting find. That's not the only occurrence of warnings not being triggered. I remember trying without success to get deprecation notices for gmscore for example. Something in our toolchain swallowing warnings could explain that as well.
Cc: yfried...@chromium.org n...@chromium.org
newt - Yaron mentioned you've dealt with lint in the past. Any ideas about this?
Think I've figured it out.

Adding "--libraries ../../third_party/android_tools/sdk/extras/android/support/annotations/android-support-annotations.jar"

makes them show up!

Comment 6 by n...@chromium.org, Mar 31 2016

Nice find :)
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/869ed1705425ef599bc733eb059ae36df2d34bf1

commit 869ed1705425ef599bc733eb059ae36df2d34bf1
Author: agrieve <agrieve@chromium.org>
Date: Thu Mar 31 18:37:56 2016

Don't run android lint on third_party/android_swipe_refresh

BUG= 599052 

Review URL: https://codereview.chromium.org/1844383002

Cr-Commit-Position: refs/heads/master@{#384342}

[modify] https://crrev.com/869ed1705425ef599bc733eb059ae36df2d34bf1/third_party/android_swipe_refresh/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb3dd1ad7c04c091faf285aa2f78da0b7cec7a76

commit eb3dd1ad7c04c091faf285aa2f78da0b7cec7a76
Author: agrieve <agrieve@chromium.org>
Date: Wed Apr 06 03:12:57 2016

Enable support-annotations and android framework lint checks

annotations such as: @IntDef, @MainThread, etc
warnings such as: HandlerLeak, MissingSuperCall, ValidFragment

Problem was that we were not passing lint the classpath.

This change is GN-only since GYP lint warnings fail the build.
Will follow-up with a GYP change once warnings are fixed.

BUG= 599052 

Review URL: https://codereview.chromium.org/1847823003

Cr-Commit-Position: refs/heads/master@{#385373}

[modify] https://crrev.com/eb3dd1ad7c04c091faf285aa2f78da0b7cec7a76/build/android/gyp/lint.py
[modify] https://crrev.com/eb3dd1ad7c04c091faf285aa2f78da0b7cec7a76/build/android/lint/suppressions.xml
[modify] https://crrev.com/eb3dd1ad7c04c091faf285aa2f78da0b7cec7a76/build/config/android/internal_rules.gni

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef5e20f9fe84651ca03fb53ed15910121cedfc0a

commit ef5e20f9fe84651ca03fb53ed15910121cedfc0a
Author: mlopatkin <mlopatkin@yandex-team.ru>
Date: Fri Apr 08 20:26:47 2016

Properly set up classpath for Android Lint (and fix its stderr filter)

Lint's stderr filter actually was dropping all output of the tool
instead of just single 'Picking up _JAVA_OPTIONS' line. Fixing this
revealed a lot of internal lint errors.

Most of them were CRC errors from interface JARs, so I switched back to
use normal JARs.

Others were internal errors of InvalidPackageName check which was trying
to check Android SDK classes, because these classes were added to
classpath as a jar. It turns out that Lint treats SDK jar differently but
only if it is properly specified. The only way to tell Lint which SDK
jar to use is to create project.properties file (like in old Ant-based
build). So lint.py now accepts SDK version as a command-line argument
and creates dummy project.properties in temporary folder.

BUG= 599052 

Review URL: https://codereview.chromium.org/1870073002

Cr-Commit-Position: refs/heads/master@{#386188}

[modify] https://crrev.com/ef5e20f9fe84651ca03fb53ed15910121cedfc0a/build/android/android_lint_cache.gyp
[modify] https://crrev.com/ef5e20f9fe84651ca03fb53ed15910121cedfc0a/build/android/gyp/lint.py
[modify] https://crrev.com/ef5e20f9fe84651ca03fb53ed15910121cedfc0a/build/config/android/internal_rules.gni

Status: Fixed (was: Assigned)

Sign in to add a comment