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.

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

Improve guava-android Animal Sniffer compatibility testing for Java 8+ APIs #7197

Open
cpovirk opened this issue May 2, 2024 · 4 comments
Open
Labels
P3 package=general type=other Miscellaneous activities not covered by other type= labels

Comments

@cpovirk
Copy link
Member

cpovirk commented May 2, 2024

As of 33.2.0 (and even slightly earlier for package-private and guava-testlib APIs), we have APIs in our Android flavor that require library desugaring. For them, we disable Animal Sniffer. We could do better by having multiple Animal Sniffer runs:

  • the current one, which flags anything that requires a newer version than Lollipop
  • another one that flags anything that requires a newer version than Nougat (the version that we might work with without library desugaring)
  • another one that flags anything that requires more than Lollipop + library desugaring
  • another one that flags anything that requires more than Java 8

Then we'd want to use 2 slightly different sets of annotations to disable Animal Sniffer:

  • one set that disables all those checks (in case we want to conditionally use an API that might not be available)
  • one that disables only the first check

This is not a great description of what I'm going for, but I wanted to get down some rough notes after already touching on this in the linked TODO and in the release notes.

@cpovirk cpovirk added type=other Miscellaneous activities not covered by other type= labels package=general P3 labels May 2, 2024
@cpovirk
Copy link
Member Author

cpovirk commented Jun 13, 2024

Another part of the picture is guava-jre: It is convenient to be able to put the same @IgnoreJRERequirement usages in guava-jre as in guava-android (to reduce merge conflicts), but doing so disables checking of usages of Java 9+ APIs. If we were to design a suite of annotations, then perhaps we could have one that is appropriate to use in both branches but that doesn't disable Java 9+ API checking.

[edit: Currently, we're inconsistent about whether to use @IgnoreJRERequirement (and @SuppressWarnings("java7ApiChecker), which is harmless to include but which also doesn't help much with merge conflicts if it always appears with @IgnoreJRERequirement) in guava-jre.]

@cpovirk
Copy link
Member Author

cpovirk commented Jul 22, 2024

This almost became non-theoretical: As I learned in the course of investigating open-toast/gummy-bears#64, Byte.compareUnsigned is a method that is always desugared under Android but not available under Java 8.

@cpovirk
Copy link
Member Author

cpovirk commented Oct 31, 2024

Putting the annotations in both branches might also reduce the number of differences in the class files generated by the two flavors (especially if we also remove the Java7ApiChecker suppressions, another thing that we've discussed). At least inside Google, that would reduce (but not to zero) the number of times that we get errors when apps (wrongly but commonly) mix together the flavors.

@cpovirk
Copy link
Member Author

cpovirk commented Nov 1, 2024

The introduction of different annotations would also be an opportune time to fix the https://errorprone.info/bugpattern/IdentifierName violation of the current annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 package=general type=other Miscellaneous activities not covered by other type= labels
Projects
None yet
Development

No branches or pull requests

1 participant