Add custom Error Prone check for SDK comparisons.
Over the years we've had several obscure bugs related to how SDK level
comparisons are performed, specifically during the window of time
where we've started distributing the "frankenbuild" to developers.
Consider the case where a framework developer shipping release "R"
wants to only grant a specific behavior to modern apps; they could
write this in two different ways:
1. if (targetSdkVersion > Build.VERSION_CODES.Q) {
2. if (targetSdkVersion >= Build.VERSION_CODES.R) {
The safer of these two options is (2), which will ensure that
developers only get the behavior when *both* the app and the
platform concur on the specific SDK level having shipped.
Consider the breakage that would happen with option (1) if we
started shipping APKs that are based on the final R SDK, but are
then installed on earlier preview releases which still consider R
to be CUR_DEVELOPMENT; they'd risk crashing due to behaviors that
were never part of the official R SDK.
Bug: 64412239
Test: ./build/soong/soong_ui.bash --make-mode services RUN_ERROR_PRONE=true
Exempt-From-Owner-Approval: trivial blueprint changes
Change-Id: Ia20181f8602451ac9a719ea488d148e160708592
diff --git a/errorprone/Android.bp b/errorprone/Android.bp
new file mode 100644
index 0000000..016b855
--- /dev/null
+++ b/errorprone/Android.bp
@@ -0,0 +1,25 @@
+
+java_plugin {
+ name: "error_prone_android_framework",
+
+ static_libs: [
+ "error_prone_android_framework_lib",
+ ],
+}
+
+java_library_host {
+ name: "error_prone_android_framework_lib",
+
+ srcs: ["java/**/*.java"],
+
+ static_libs: [
+ "//external/error_prone:error_prone_core",
+ "//external/dagger2:dagger2-auto-service",
+ ],
+
+ plugins: [
+ "//external/dagger2:dagger2-auto-service",
+ ],
+
+ javacflags: ["-verbose"],
+}
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java
new file mode 100644
index 0000000..1ce816c
--- /dev/null
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/TargetSdkChecker.java
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns.android;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+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.kindIs;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.FieldMatchers;
+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;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+ name = "AndroidFrameworkTargetSdk",
+ summary = "Verifies that all target SDK comparisons are sane",
+ severity = WARNING)
+public final class TargetSdkChecker extends BugChecker implements BinaryTreeMatcher {
+ private static final Matcher<ExpressionTree> VERSION_CODE = FieldMatchers
+ .anyFieldInClass("android.os.Build.VERSION_CODES");
+
+ private static final Matcher<BinaryTree> INVALID_OLD_BEHAVIOR = anyOf(
+ allOf(kindIs(Kind.LESS_THAN_EQUAL), binaryTreeExact(anything(), VERSION_CODE)),
+ allOf(kindIs(Kind.GREATER_THAN_EQUAL), binaryTreeExact(VERSION_CODE, anything())));
+
+ private static final Matcher<BinaryTree> INVALID_NEW_BEHAVIOR = anyOf(
+ allOf(kindIs(Kind.GREATER_THAN), binaryTreeExact(anything(), VERSION_CODE)),
+ allOf(kindIs(Kind.LESS_THAN), binaryTreeExact(VERSION_CODE, anything())));
+
+ @Override
+ public Description matchBinary(BinaryTree tree, VisitorState state) {
+ if (INVALID_OLD_BEHAVIOR.matches(tree, state)) {
+ return buildDescription(tree)
+ .setMessage("Legacy behaviors must be written in style "
+ + "'targetSdk < Build.VERSION_CODES.Z'")
+ .build();
+ }
+ if (INVALID_NEW_BEHAVIOR.matches(tree, state)) {
+ return buildDescription(tree)
+ .setMessage("Modern behaviors must be written in style "
+ + "'targetSdk >= Build.VERSION_CODES.Z'")
+ .build();
+ }
+ return Description.NO_MATCH;
+ }
+
+ private static Matcher<BinaryTree> binaryTreeExact(Matcher<ExpressionTree> left,
+ Matcher<ExpressionTree> right) {
+ return new Matcher<BinaryTree>() {
+ @Override
+ public boolean matches(BinaryTree tree, VisitorState state) {
+ return left.matches(tree.getLeftOperand(), state)
+ && right.matches(tree.getRightOperand(), state);
+ }
+ };
+ }
+}
diff --git a/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java b/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java
new file mode 100644
index 0000000..46f0fb2
--- /dev/null
+++ b/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2018 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.matchers;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.ImportTree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import javax.annotation.Nullable;
+
+// TODO(glorioso): this likely wants to be a fluent interface like MethodMatchers.
+// Ex: [staticField()|instanceField()]
+// .[onClass(String)|onAnyClass|onClassMatching]
+// .[named(String)|withAnyName|withNameMatching]
+/** Static utility methods for creating {@link Matcher}s for detecting references to fields. */
+public final class FieldMatchers {
+ private FieldMatchers() {}
+
+ public static Matcher<ExpressionTree> anyFieldInClass(String className) {
+ return new FieldReferenceMatcher() {
+ @Override
+ boolean classIsAppropriate(ClassSymbol classSymbol) {
+ return classSymbol.getQualifiedName().contentEquals(className);
+ }
+
+ @Override
+ boolean fieldSymbolIsAppropriate(Symbol symbol) {
+ return true;
+ }
+ };
+ }
+
+ public static Matcher<ExpressionTree> staticField(String className, String fieldName) {
+ return new FieldReferenceMatcher() {
+ @Override
+ boolean classIsAppropriate(ClassSymbol classSymbol) {
+ return classSymbol.getQualifiedName().contentEquals(className);
+ }
+
+ @Override
+ boolean fieldSymbolIsAppropriate(Symbol symbol) {
+ return symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName);
+ }
+ };
+ }
+
+ public static Matcher<ExpressionTree> instanceField(String className, String fieldName) {
+ return new FieldReferenceMatcher() {
+ @Override
+ boolean classIsAppropriate(ClassSymbol classSymbol) {
+ return classSymbol.getQualifiedName().contentEquals(className);
+ }
+
+ @Override
+ boolean fieldSymbolIsAppropriate(Symbol symbol) {
+ return !symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName);
+ }
+ };
+ }
+
+ private abstract static class FieldReferenceMatcher implements Matcher<ExpressionTree> {
+ @Override
+ public boolean matches(ExpressionTree expressionTree, VisitorState state) {
+ return isSymbolFieldInAppropriateClass(ASTHelpers.getSymbol(expressionTree))
+ // Don't match if this is part of a static import tree, since they will get the finding
+ // on any usage of the field in their source.
+ && ASTHelpers.findEnclosingNode(state.getPath(), ImportTree.class) == null;
+ }
+
+ private boolean isSymbolFieldInAppropriateClass(@Nullable Symbol symbol) {
+ if (symbol == null) {
+ return false;
+ }
+ return symbol.getKind().isField()
+ && fieldSymbolIsAppropriate(symbol)
+ && classIsAppropriate(symbol.owner.enclClass());
+ }
+
+ abstract boolean fieldSymbolIsAppropriate(Symbol symbol);
+
+ abstract boolean classIsAppropriate(ClassSymbol classSymbol);
+ }
+}
diff --git a/services/Android.bp b/services/Android.bp
index 6d637be..74db5bd 100644
--- a/services/Android.bp
+++ b/services/Android.bp
@@ -1,3 +1,10 @@
+java_defaults {
+ name: "services_defaults",
+ plugins: [
+ "error_prone_android_framework",
+ ],
+}
+
filegroup {
name: "services-main-sources",
srcs: ["java/**/*.java"],
@@ -83,7 +90,6 @@
// Uncomment to enable output of certain warnings (deprecated, unchecked)
//javacflags: ["-Xlint"],
-
}
// native library
diff --git a/services/accessibility/Android.bp b/services/accessibility/Android.bp
index 284a2f2..21a0c74 100644
--- a/services/accessibility/Android.bp
+++ b/services/accessibility/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.accessibility",
+ defaults: ["services_defaults"],
srcs: [":services.accessibility-sources"],
libs: ["services.core"],
}
diff --git a/services/appprediction/Android.bp b/services/appprediction/Android.bp
index e14e1df..c12f62f 100644
--- a/services/appprediction/Android.bp
+++ b/services/appprediction/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.appprediction",
+ defaults: ["services_defaults"],
srcs: [":services.appprediction-sources"],
libs: ["services.core"],
}
diff --git a/services/appwidget/Android.bp b/services/appwidget/Android.bp
index 54cf6ce..83a9aa4 100644
--- a/services/appwidget/Android.bp
+++ b/services/appwidget/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.appwidget",
+ defaults: ["services_defaults"],
srcs: [":services.appwidget-sources"],
libs: ["services.core"],
}
diff --git a/services/autofill/Android.bp b/services/autofill/Android.bp
index 539eb1a..1e65e84 100644
--- a/services/autofill/Android.bp
+++ b/services/autofill/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.autofill",
+ defaults: ["services_defaults"],
srcs: [":services.autofill-sources"],
libs: ["services.core"],
}
diff --git a/services/backup/Android.bp b/services/backup/Android.bp
index f02da20..56b788e 100644
--- a/services/backup/Android.bp
+++ b/services/backup/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.backup",
+ defaults: ["services_defaults"],
srcs: [":services.backup-sources"],
libs: ["services.core"],
static_libs: ["backuplib"],
diff --git a/services/companion/Android.bp b/services/companion/Android.bp
index 9677a7d..e251042 100644
--- a/services/companion/Android.bp
+++ b/services/companion/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.companion",
+ defaults: ["services_defaults"],
srcs: [":services.companion-sources"],
libs: ["services.core"],
}
diff --git a/services/contentcapture/Android.bp b/services/contentcapture/Android.bp
index 96e2072..7006430 100644
--- a/services/contentcapture/Android.bp
+++ b/services/contentcapture/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.contentcapture",
+ defaults: ["services_defaults"],
srcs: [":services.contentcapture-sources"],
libs: ["services.core"],
}
diff --git a/services/contentsuggestions/Android.bp b/services/contentsuggestions/Android.bp
index d17f06f..3fe3cd2 100644
--- a/services/contentsuggestions/Android.bp
+++ b/services/contentsuggestions/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.contentsuggestions",
+ defaults: ["services_defaults"],
srcs: [":services.contentsuggestions-sources"],
libs: ["services.core"],
}
diff --git a/services/core/Android.bp b/services/core/Android.bp
index 77773ed..40f67c7 100644
--- a/services/core/Android.bp
+++ b/services/core/Android.bp
@@ -148,6 +148,7 @@
java_library {
name: "services.core",
+ defaults: ["services_defaults"],
static_libs: ["services.core.priorityboosted"],
}
diff --git a/services/coverage/Android.bp b/services/coverage/Android.bp
index e4f5464..df054b0 100644
--- a/services/coverage/Android.bp
+++ b/services/coverage/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.coverage",
+ defaults: ["services_defaults"],
srcs: [":services.coverage-sources"],
libs: ["jacocoagent"],
}
diff --git a/services/devicepolicy/Android.bp b/services/devicepolicy/Android.bp
index 2f6592b..7a80fb1 100644
--- a/services/devicepolicy/Android.bp
+++ b/services/devicepolicy/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.devicepolicy",
+ defaults: ["services_defaults"],
srcs: [":services.devicepolicy-sources"],
libs: [
diff --git a/services/midi/Android.bp b/services/midi/Android.bp
index 20e0083..6bce5b5 100644
--- a/services/midi/Android.bp
+++ b/services/midi/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.midi",
+ defaults: ["services_defaults"],
srcs: [":services.midi-sources"],
libs: ["services.core"],
}
diff --git a/services/net/Android.bp b/services/net/Android.bp
index bb5409b..eaf11fd 100644
--- a/services/net/Android.bp
+++ b/services/net/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.net",
+ defaults: ["services_defaults"],
srcs: [
":net-module-utils-srcs",
":services.net-sources",
diff --git a/services/people/Android.bp b/services/people/Android.bp
index d64097a..c863f1f 100644
--- a/services/people/Android.bp
+++ b/services/people/Android.bp
@@ -1,5 +1,6 @@
java_library_static {
name: "services.people",
+ defaults: ["services_defaults"],
srcs: ["java/**/*.java"],
libs: ["services.core"],
}
diff --git a/services/print/Android.bp b/services/print/Android.bp
index aad24d7..93b5ef0 100644
--- a/services/print/Android.bp
+++ b/services/print/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.print",
+ defaults: ["services_defaults"],
srcs: [":services.print-sources"],
libs: ["services.core"],
}
diff --git a/services/restrictions/Android.bp b/services/restrictions/Android.bp
index 805858f..2883095 100644
--- a/services/restrictions/Android.bp
+++ b/services/restrictions/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.restrictions",
+ defaults: ["services_defaults"],
srcs: [":services.restrictions-sources"],
libs: ["services.core"],
}
diff --git a/services/startop/Android.bp b/services/startop/Android.bp
index 093b4ec..46a81aae 100644
--- a/services/startop/Android.bp
+++ b/services/startop/Android.bp
@@ -16,6 +16,7 @@
java_library_static {
name: "services.startop",
+ defaults: ["services_defaults"],
static_libs: [
// frameworks/base/startop/iorap
diff --git a/services/systemcaptions/Android.bp b/services/systemcaptions/Android.bp
index 1ce3e66..54968c0 100644
--- a/services/systemcaptions/Android.bp
+++ b/services/systemcaptions/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.systemcaptions",
+ defaults: ["services_defaults"],
srcs: [":services.systemcaptions-sources"],
libs: ["services.core"],
}
diff --git a/services/usage/Android.bp b/services/usage/Android.bp
index 156bf33..463673f 100644
--- a/services/usage/Android.bp
+++ b/services/usage/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.usage",
+ defaults: ["services_defaults"],
srcs: [":services.usage-sources"],
libs: ["services.core"],
}
diff --git a/services/usb/Android.bp b/services/usb/Android.bp
index a9474c1..4e98409 100644
--- a/services/usb/Android.bp
+++ b/services/usb/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.usb",
+ defaults: ["services_defaults"],
srcs: [":services.usb-sources"],
libs: [
diff --git a/services/voiceinteraction/Android.bp b/services/voiceinteraction/Android.bp
index 85b96f34..47129ad 100644
--- a/services/voiceinteraction/Android.bp
+++ b/services/voiceinteraction/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.voiceinteraction",
+ defaults: ["services_defaults"],
srcs: [":services.voiceinteraction-sources"],
libs: ["services.core"],
}
diff --git a/services/wifi/Android.bp b/services/wifi/Android.bp
index f56c2cf..3975fd2 100644
--- a/services/wifi/Android.bp
+++ b/services/wifi/Android.bp
@@ -7,6 +7,7 @@
java_library_static {
name: "services.wifi",
+ defaults: ["services_defaults"],
srcs: [
":services.wifi-sources",
],