Fixed
Status Update
Comments
vi...@google.com <vi...@google.com> #2
Thank you for submitting the feedback. But we are unable to reproduce the issue on 3.3 RC1 from our end. If you can provide a sample project or a screenshot of the issue it will be really helpful to triage the issue.
If we do not get an update in the next 30 days we will close this issue.
If we do not get an update in the next 30 days we will close this issue.
sa...@gmail.com <sa...@gmail.com> #3
Hi,
Here's a sample project that can reproduce the issue by running Lint : ./gradlew lint
The lint result file is present at the root of the project.
It reproduces the wrong InlinedApi warning & NewApi error.
I've also updated the project to use the new 3.3-rc2 version.
In fact, it's only reproductible when a project have multiple modules.
In my sample there is a class `AndroidVersions` that is in a `core` module and which is used in my `app` module (in MainActivity class).
If the 2 classes are in the same modules, Lint is running properly. But moving the `AndroidVersions` to the core module break it.
Here's a sample project that can reproduce the issue by running Lint : ./gradlew lint
The lint result file is present at the root of the project.
It reproduces the wrong InlinedApi warning & NewApi error.
I've also updated the project to use the new 3.3-rc2 version.
In fact, it's only reproductible when a project have multiple modules.
In my sample there is a class `AndroidVersions` that is in a `core` module and which is used in my `app` module (in MainActivity class).
If the 2 classes are in the same modules, Lint is running properly. But moving the `AndroidVersions` to the core module break it.
es...@google.com <es...@google.com>
tn...@google.com <tn...@google.com> #4
Hi, the issue is still reproducible with the 3.3-rc03 version.
zh...@gmail.com <zh...@gmail.com> #5
Thank you for your feedback. Team may reach out for more feedback in reproducing or triaging this issue.
tn...@google.com <tn...@google.com> #6
Hi, the bug is still here on the final 3.3.0 version.
dr...@gmail.com <dr...@gmail.com> #7
We are seeing the same thing in our app which is heavily modularized. We're on AGP 3.5.3 and Gradle 4.4 and are constantly forced to baseline false positives with these three rules.
tn...@google.com <tn...@google.com> #8
Here's the reason this happens:
Normally, when lint finds a method call or field reference that requires a higher API level than the current minSdkVersion, it looks to see if the call is inside a
if (SDK_INT > x) { ... }
body. (*: it's a little more complicated than that -- for example, it will also let you be in an else clause with a reverse condition, and you can also put a version check at the beginning of the method and exit early etc -- but the principle is the same).
Then I made the check more sophisticated, such that if your violating call is surrounded by say
if (foo()) { ... }
lint will check the "foo" method to see if it looks like it performs a version check.
In its simplest form, this simply peeks at the method body of the given method, "foo" above, and if that method simply does
"return SDK_INT > x"
then we're good.
I later extended this to be more clever, such that if you're surrounded by
if (foo_bar(x, y, z, 28)) { ... }
and the method body does
return SDK_INT > parameter4
then it will check the 4th argument to the method call and see if that one is >= X.
....anyway...
This whole mechanism relies on lint's ability to go and look at the method body. That works fine when the utility method is available as source.
But the second you extract the method body into a separate module, now it's just a call into a random method. Lint doesn't go and try to analyze the bytecode of methods (and soon that won't even make sense, since we plan to switch libraries to only contain the API surface, and remove method bodies (and private methods etc etc) for better compile time avoidance as well as other changes, similar to how android.jar currently is just exposing the Android SDK API surface, with none of the implementation details.
So that's why lint can't look at methods into binary libraries and figure out that an *arbitrarily* named function with a set of arbitrary methods implies that a given API has been verified.
---
There are two potential solutions here.
The first one is for us to add a new annotation to the AndroidX annotations library, perhaps
@ChecksApi(min=28)
fun foo_bar() { ... }
and perhaps a variation with parameters,
@CheckApi(apiLevelParameter=2)
fun foo_bar(something: Boolean, myLevel: Int)
Lint could use those annotations on foreign methods to tell whether a method represents an API check. This work isn't done yet so obviously isn't a solution for you.
The second solution is a workaround. Lint already uses some simple heuristics to guess if a method represents a version check method. So if you name your own version utility methods to conform to one of these patterns, lint should work. I realize that renaming methods may be out of the question if this is a commonly used method in your codebase with a lot of existing references, but I'm throwing it out there as a potential workaround.
Here's the prefixes and suffixes in method names that lint looks for:
private static final String[] VERSION_METHOD_NAME_PREFIXES = {
"isAtLeast", "isRunning", "is", "runningOn", "running"
};
private static final String[] VERSION_METHOD_NAME_SUFFIXES = {
"OrLater", "OrAbove", "OrHigher", "OrNewer", "Sdk"
};
and here are some examples from Lint's unit tests (VersionChecksTest, if you're curious)
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastKitKat"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatSdk"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatSDK"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isRunningKitkatOrLater"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKeyLimePieOrLater"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatOrHigher"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatOrNewer"));
assertEquals(
17, VersionChecks.getMinSdkVersionFromMethodName("isRunningJellyBeanMR1OrLater"));
assertEquals(20, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastKitKatWatch"));
As you can see, the version numbers here are tied to the build version codes. (It also handles using just the dessert letter after isAtLeast; this is how it supports the AndroidX BuildCompat class which has a bunch of isAtLeast methods; those methods suffer from the same problem I've mentioned above (it's a binary/third party library version check, so lint doesn't see its method implementation, and instead it infers the requirement from the name.)
Normally, when lint finds a method call or field reference that requires a higher API level than the current minSdkVersion, it looks to see if the call is inside a
if (SDK_INT > x) { ... }
body. (*: it's a little more complicated than that -- for example, it will also let you be in an else clause with a reverse condition, and you can also put a version check at the beginning of the method and exit early etc -- but the principle is the same).
Then I made the check more sophisticated, such that if your violating call is surrounded by say
if (foo()) { ... }
lint will check the "foo" method to see if it looks like it performs a version check.
In its simplest form, this simply peeks at the method body of the given method, "foo" above, and if that method simply does
"return SDK_INT > x"
then we're good.
I later extended this to be more clever, such that if you're surrounded by
if (foo_bar(x, y, z, 28)) { ... }
and the method body does
return SDK_INT > parameter4
then it will check the 4th argument to the method call and see if that one is >= X.
....anyway...
This whole mechanism relies on lint's ability to go and look at the method body. That works fine when the utility method is available as source.
But the second you extract the method body into a separate module, now it's just a call into a random method. Lint doesn't go and try to analyze the bytecode of methods (and soon that won't even make sense, since we plan to switch libraries to only contain the API surface, and remove method bodies (and private methods etc etc) for better compile time avoidance as well as other changes, similar to how android.jar currently is just exposing the Android SDK API surface, with none of the implementation details.
So that's why lint can't look at methods into binary libraries and figure out that an *arbitrarily* named function with a set of arbitrary methods implies that a given API has been verified.
---
There are two potential solutions here.
The first one is for us to add a new annotation to the AndroidX annotations library, perhaps
@ChecksApi(min=28)
fun foo_bar() { ... }
and perhaps a variation with parameters,
@CheckApi(apiLevelParameter=2)
fun foo_bar(something: Boolean, myLevel: Int)
Lint could use those annotations on foreign methods to tell whether a method represents an API check. This work isn't done yet so obviously isn't a solution for you.
The second solution is a workaround. Lint already uses some simple heuristics to guess if a method represents a version check method. So if you name your own version utility methods to conform to one of these patterns, lint should work. I realize that renaming methods may be out of the question if this is a commonly used method in your codebase with a lot of existing references, but I'm throwing it out there as a potential workaround.
Here's the prefixes and suffixes in method names that lint looks for:
private static final String[] VERSION_METHOD_NAME_PREFIXES = {
"isAtLeast", "isRunning", "is", "runningOn", "running"
};
private static final String[] VERSION_METHOD_NAME_SUFFIXES = {
"OrLater", "OrAbove", "OrHigher", "OrNewer", "Sdk"
};
and here are some examples from Lint's unit tests (VersionChecksTest, if you're curious)
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastKitKat"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatSdk"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatSDK"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isRunningKitkatOrLater"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKeyLimePieOrLater"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatOrHigher"));
assertEquals(19, VersionChecks.getMinSdkVersionFromMethodName("isKitKatOrNewer"));
assertEquals(
17, VersionChecks.getMinSdkVersionFromMethodName("isRunningJellyBeanMR1OrLater"));
assertEquals(20, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastKitKatWatch"));
As you can see, the version numbers here are tied to the build version codes. (It also handles using just the dessert letter after isAtLeast; this is how it supports the AndroidX BuildCompat class which has a bunch of isAtLeast methods; those methods suffer from the same problem I've mentioned above (it's a binary/third party library version check, so lint doesn't see its method implementation, and instead it infers the requirement from the name.)
de...@google.com <de...@google.com> #9
Actually, I realized I can & should broaden the version name check a bit -- to allow "has" as a prefix, and more importantly, to allow "API_?<number>" as a version (right now it only looks for build codes).
Here's the additional unit test cases for the CL I just uploaded:
assertEquals(29, VersionChecks.getMinSdkVersionFromMethodName("hasQ"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("hasApi28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastApi28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastAPI_28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isApi28OrLater"));
(This obviously won't help you until this rolls out into an upcoming 4.0 beta; the above workaround or baseline is what you'll need to use until then.)
Here's the additional unit test cases for the CL I just uploaded:
assertEquals(29, VersionChecks.getMinSdkVersionFromMethodName("hasQ"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("hasApi28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastApi28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isAtLeastAPI_28"));
assertEquals(28, VersionChecks.getMinSdkVersionFromMethodName("isApi28OrLater"));
(This obviously won't help you until this rolls out into an upcoming 4.0 beta; the above workaround or baseline is what you'll need to use until then.)
Description
I wanted to float an idea for a new lint check: An @OpenForTesting annotation in Kotlin, similar to the @VisibleForTesting annotation that exists currently. The idea being here to indicate that a class or method is marked as open so that it can be subclassed for testing (either manually or with Mockito), and should not be overridden or subclassed in production code. This would be beneficial for those writing android instrumentation tests, where mocking final methods and classes is not possible before Android P (
This change would consist of 2 parts:
1) A new annotation class to indicate this, such as @OpenForTesting
2) A new lint check to enforce that no methods are overridden or classes subclassed that contain this annotation. IDE highlighting would also be ideal.
Please let me know what you think about this idea!