Refine CompatChanges check and enable as error.
Because shifting newly written code over to using CompatChanges is
important, this change refines the recently added check and upgrades
it to become a fatal build error.
Bug: 169879376
Test: atest error_prone_android_framework_test
Change-Id: Ic3126518ebaac9995b8f649e44b839de30faa17f
diff --git a/Android.bp b/Android.bp
index 5af7756..f69f9d8 100644
--- a/Android.bp
+++ b/Android.bp
@@ -598,6 +598,11 @@
"//frameworks/base/apex/jobscheduler/framework",
"//frameworks/base/packages/Tethering/tests/unit",
],
+ errorprone: {
+ javacflags: [
+ "-Xep:AndroidFrameworkCompatChange:ERROR",
+ ],
+ },
}
// This "framework" module is NOT installed to the device. It's
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java
index 8829b0d..9c84f50 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java
@@ -17,12 +17,13 @@
package com.google.errorprone.bugpatterns.android;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.bugpatterns.android.TargetSdkChecker.binaryTreeExact;
import static com.google.errorprone.matchers.FieldMatchers.anyFieldInClass;
import static com.google.errorprone.matchers.FieldMatchers.staticField;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.anything;
-import static com.google.errorprone.matchers.Matchers.binaryTree;
+import static com.google.errorprone.matchers.Matchers.kindIs;
import static com.google.errorprone.matchers.Matchers.not;
import com.google.auto.service.AutoService;
@@ -34,6 +35,7 @@
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.BinaryTree;
import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.Tree.Kind;
/**
* Each SDK level often has dozens of different behavior changes, which can be
@@ -85,14 +87,28 @@
staticField("android.os.Build.VERSION_CODES", "O"),
staticField("android.os.Build.VERSION_CODES", "O_MR1"),
staticField("android.os.Build.VERSION_CODES", "P"),
- staticField("android.os.Build.VERSION_CODES", "Q"));
+ staticField("android.os.Build.VERSION_CODES", "Q"),
+ staticField("android.os.Build.VERSION_CODES", "R"));
+
+ private static final Matcher<ExpressionTree> R_VERSION_CODE =
+ staticField("android.os.Build.VERSION_CODES", "R");
+
+ private static final Matcher<ExpressionTree> CUR_DEVELOPMENT_VERSION_CODE =
+ staticField("android.os.Build.VERSION_CODES", "CUR_DEVELOPMENT");
private static final Matcher<ExpressionTree> MODERN_VERSION_CODE =
- allOf(VERSION_CODE, not(LEGACY_VERSION_CODE));
+ allOf(VERSION_CODE, not(LEGACY_VERSION_CODE), not(CUR_DEVELOPMENT_VERSION_CODE));
+
+ private static final Matcher<ExpressionTree> BOOLEAN_OPERATOR = anyOf(
+ kindIs(Kind.LESS_THAN), kindIs(Kind.LESS_THAN_EQUAL),
+ kindIs(Kind.GREATER_THAN), kindIs(Kind.GREATER_THAN_EQUAL),
+ kindIs(Kind.EQUAL_TO), kindIs(Kind.NOT_EQUAL_TO));
private static final Matcher<BinaryTree> INVALID = anyOf(
- binaryTree(MODERN_VERSION_CODE, anything()),
- binaryTree(anything(), MODERN_VERSION_CODE));
+ allOf(BOOLEAN_OPERATOR, binaryTreeExact(MODERN_VERSION_CODE, anything())),
+ allOf(BOOLEAN_OPERATOR, binaryTreeExact(anything(), MODERN_VERSION_CODE)),
+ allOf(kindIs(Kind.GREATER_THAN), binaryTreeExact(anything(), R_VERSION_CODE)),
+ allOf(kindIs(Kind.LESS_THAN), binaryTreeExact(R_VERSION_CODE, anything())));
@Override
public Description matchBinary(BinaryTree tree, VisitorState state) {
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java
index 232cf3f..e1ebf42 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java
@@ -89,7 +89,7 @@
return Description.NO_MATCH;
}
- private static Matcher<BinaryTree> binaryTreeExact(Matcher<ExpressionTree> left,
+ static Matcher<BinaryTree> binaryTreeExact(Matcher<ExpressionTree> left,
Matcher<ExpressionTree> right) {
return new Matcher<BinaryTree>() {
@Override
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java
index 105633e..4625d43 100644
--- a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java
@@ -42,17 +42,17 @@
"public class Example {",
" void test(int targetSdkVersion) {",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion < Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion < Build.VERSION_CODES.S) { }",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion <= Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion <= Build.VERSION_CODES.S) { }",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion > Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion > Build.VERSION_CODES.S) { }",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion >= Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion >= Build.VERSION_CODES.S) { }",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion == Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion == Build.VERSION_CODES.S) { }",
" // BUG: Diagnostic contains:",
- " if (targetSdkVersion != Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion != Build.VERSION_CODES.S) { }",
" }",
"}")
.doTest();
@@ -63,18 +63,29 @@
compilationHelper
.addSourceFile("/android/os/Build.java")
.addSourceLines("Example.java",
- "import static android.os.Build.VERSION_CODES.R;",
+ "import android.os.Build;",
+ "import static android.os.Build.VERSION_CODES.S;",
"public class Example {",
" void test(int targetSdkVersion) {",
" // BUG: Diagnostic contains:",
- " boolean indirect = R > targetSdkVersion;",
+ " boolean indirect = S >= targetSdkVersion;",
+ " // BUG: Diagnostic contains:",
+ " if (targetSdkVersion > Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion >= Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion < Build.VERSION_CODES.R) { }",
+ " if (targetSdkVersion <= Build.VERSION_CODES.R) { }",
+ " // BUG: Diagnostic contains:",
+ " if (Build.VERSION_CODES.R < targetSdkVersion) { }",
+ " if (Build.VERSION_CODES.R <= targetSdkVersion) { }",
+ " if (Build.VERSION_CODES.R > targetSdkVersion) { }",
+ " if (Build.VERSION_CODES.R >= targetSdkVersion) { }",
" }",
"}")
.doTest();
}
@Test
- public void testLegacyIgnored() {
+ public void testIgnored() {
compilationHelper
.addSourceFile("/android/os/Build.java")
.addSourceLines("Example.java",
@@ -82,6 +93,8 @@
"public class Example {",
" void test(int targetSdkVersion) {",
" if (targetSdkVersion < Build.VERSION_CODES.DONUT) { }",
+ " String result = \"test\" + Build.VERSION_CODES.S;",
+ " if (targetSdkVersion != Build.VERSION_CODES.CUR_DEVELOPMENT) { }",
" }",
"}")
.doTest();
diff --git a/errorprone/tests/res/android/os/Build.java b/errorprone/tests/res/android/os/Build.java
index 0b51bdb..2d354e2 100644
--- a/errorprone/tests/res/android/os/Build.java
+++ b/errorprone/tests/res/android/os/Build.java
@@ -18,7 +18,9 @@
public class Build {
public static class VERSION_CODES {
+ public static final int CUR_DEVELOPMENT = 10000;
public static final int DONUT = 4;
public static final int R = 30;
+ public static final int S = CUR_DEVELOPMENT;
}
}
diff --git a/services/Android.bp b/services/Android.bp
index ef52c2a..971f4dd 100644
--- a/services/Android.bp
+++ b/services/Android.bp
@@ -3,6 +3,11 @@
plugins: [
"error_prone_android_framework",
],
+ errorprone: {
+ javacflags: [
+ "-Xep:AndroidFrameworkCompatChange:ERROR",
+ ],
+ },
}
filegroup {