From 71a1770456794c204caf1468355eda357d98c858 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 1 Oct 2020 14:50:31 -0600 Subject: [PATCH] Guide behavior changes towards CompatChanges. Each SDK level often has dozens of different behavior changes, which can be difficult for large app developers to adjust to during preview or beta releases. For this reason, android.app.compat.CompatChanges was introduced as a new best-practice for adding behavior changes. During a preview or beta release, developers can temporarily opt-out of each individual change to aid debugging. This opt-out is only available during preview of beta releases, and cannot be adjusted on finalized builds. Bug: 169879376 Test: atest error_prone_android_framework_test Change-Id: Ib3b2e2139e084b0fa1bcbb5e89dd55e7ca4bfa00 --- .../android/CompatChangeChecker.java | 109 ++++++++++++++++++ .../errorprone/matchers/FieldMatchers.java | 15 ++- .../android/CompatChangeCheckerTest.java | 89 ++++++++++++++ errorprone/tests/res/android/os/Build.java | 1 + 4 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java create mode 100644 errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java new file mode 100644 index 0000000000000..8829b0d3c6491 --- /dev/null +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/CompatChangeChecker.java @@ -0,0 +1,109 @@ +/* + * 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.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.not; + +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.Matcher; +import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.ExpressionTree; + +/** + * Each SDK level often has dozens of different behavior changes, which can be + * difficult for large app developers to adjust to during preview or beta + * releases. For this reason, {@code android.app.compat.CompatChanges} was + * introduced as a new best-practice for adding behavior changes. + *

+ * During a preview or beta release, developers can temporarily opt-out of each + * individual change to aid debugging. This opt-out is only available during + * preview of beta releases, and cannot be adjusted on finalized builds. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "AndroidFrameworkCompatChange", + summary = "Verifies that behavior changes use the modern compatibility framework", + severity = WARNING) +public final class CompatChangeChecker extends BugChecker implements BinaryTreeMatcher { + private static final Matcher VERSION_CODE = + anyFieldInClass("android.os.Build.VERSION_CODES"); + + // Ship has already sailed on these SDK levels; not worth fixing + private static final Matcher LEGACY_VERSION_CODE = anyOf( + staticField("android.os.Build.VERSION_CODES", "BASE"), + staticField("android.os.Build.VERSION_CODES", "BASE_1_1"), + staticField("android.os.Build.VERSION_CODES", "CUPCAKE"), + staticField("android.os.Build.VERSION_CODES", "DONUT"), + staticField("android.os.Build.VERSION_CODES", "ECLAIR"), + staticField("android.os.Build.VERSION_CODES", "ECLAIR_0_1"), + staticField("android.os.Build.VERSION_CODES", "ECLAIR_MR1"), + staticField("android.os.Build.VERSION_CODES", "FROYO"), + staticField("android.os.Build.VERSION_CODES", "GINGERBREAD"), + staticField("android.os.Build.VERSION_CODES", "GINGERBREAD_MR1"), + staticField("android.os.Build.VERSION_CODES", "HONEYCOMB"), + staticField("android.os.Build.VERSION_CODES", "HONEYCOMB_MR1"), + staticField("android.os.Build.VERSION_CODES", "HONEYCOMB_MR2"), + staticField("android.os.Build.VERSION_CODES", "ICE_CREAM_SANDWICH"), + staticField("android.os.Build.VERSION_CODES", "ICE_CREAM_SANDWICH_MR1"), + staticField("android.os.Build.VERSION_CODES", "JELLY_BEAN"), + staticField("android.os.Build.VERSION_CODES", "JELLY_BEAN_MR1"), + staticField("android.os.Build.VERSION_CODES", "JELLY_BEAN_MR2"), + staticField("android.os.Build.VERSION_CODES", "KITKAT"), + staticField("android.os.Build.VERSION_CODES", "KITKAT_WATCH"), + staticField("android.os.Build.VERSION_CODES", "L"), + staticField("android.os.Build.VERSION_CODES", "LOLLIPOP"), + staticField("android.os.Build.VERSION_CODES", "LOLLIPOP_MR1"), + staticField("android.os.Build.VERSION_CODES", "M"), + staticField("android.os.Build.VERSION_CODES", "N"), + staticField("android.os.Build.VERSION_CODES", "N_MR1"), + 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")); + + private static final Matcher MODERN_VERSION_CODE = + allOf(VERSION_CODE, not(LEGACY_VERSION_CODE)); + + private static final Matcher INVALID = anyOf( + binaryTree(MODERN_VERSION_CODE, anything()), + binaryTree(anything(), MODERN_VERSION_CODE)); + + @Override + public Description matchBinary(BinaryTree tree, VisitorState state) { + if (INVALID.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Behavior changes should use CompatChanges.isChangeEnabled() " + + "instead of direct SDK checks to ease developer transitions; " + + "see go/compat-framework for more details") + .build(); + + } + return Description.NO_MATCH; + } +} diff --git a/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java b/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java index 46f0fb2e534c3..08969d60671a1 100644 --- a/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java +++ b/errorprone/java/com/google/errorprone/matchers/FieldMatchers.java @@ -36,7 +36,8 @@ public final class FieldMatchers { return new FieldReferenceMatcher() { @Override boolean classIsAppropriate(ClassSymbol classSymbol) { - return classSymbol.getQualifiedName().contentEquals(className); + return classSymbol != null + && classSymbol.getQualifiedName().contentEquals(className); } @Override @@ -50,12 +51,14 @@ public final class FieldMatchers { return new FieldReferenceMatcher() { @Override boolean classIsAppropriate(ClassSymbol classSymbol) { - return classSymbol.getQualifiedName().contentEquals(className); + return classSymbol != null + && classSymbol.getQualifiedName().contentEquals(className); } @Override boolean fieldSymbolIsAppropriate(Symbol symbol) { - return symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName); + return symbol != null + && symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName); } }; } @@ -64,12 +67,14 @@ public final class FieldMatchers { return new FieldReferenceMatcher() { @Override boolean classIsAppropriate(ClassSymbol classSymbol) { - return classSymbol.getQualifiedName().contentEquals(className); + return classSymbol != null + && classSymbol.getQualifiedName().contentEquals(className); } @Override boolean fieldSymbolIsAppropriate(Symbol symbol) { - return !symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName); + return symbol != null + && !symbol.isStatic() && symbol.getSimpleName().contentEquals(fieldName); } }; } diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java new file mode 100644 index 0000000000000..105633e4de28e --- /dev/null +++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/CompatChangeCheckerTest.java @@ -0,0 +1,89 @@ +/* + * 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 com.google.errorprone.CompilationTestHelper; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CompatChangeCheckerTest { + private CompilationTestHelper compilationHelper; + + @Before + public void setUp() { + compilationHelper = CompilationTestHelper.newInstance( + CompatChangeChecker.class, getClass()); + } + + @Test + public void testSimple() { + compilationHelper + .addSourceFile("/android/os/Build.java") + .addSourceLines("Example.java", + "import android.os.Build;", + "public class Example {", + " void test(int targetSdkVersion) {", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion < Build.VERSION_CODES.R) { }", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion <= Build.VERSION_CODES.R) { }", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion > Build.VERSION_CODES.R) { }", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion >= Build.VERSION_CODES.R) { }", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion == Build.VERSION_CODES.R) { }", + " // BUG: Diagnostic contains:", + " if (targetSdkVersion != Build.VERSION_CODES.R) { }", + " }", + "}") + .doTest(); + } + + @Test + public void testObscure() { + compilationHelper + .addSourceFile("/android/os/Build.java") + .addSourceLines("Example.java", + "import static android.os.Build.VERSION_CODES.R;", + "public class Example {", + " void test(int targetSdkVersion) {", + " // BUG: Diagnostic contains:", + " boolean indirect = R > targetSdkVersion;", + " }", + "}") + .doTest(); + } + + @Test + public void testLegacyIgnored() { + compilationHelper + .addSourceFile("/android/os/Build.java") + .addSourceLines("Example.java", + "import android.os.Build;", + "public class Example {", + " void test(int targetSdkVersion) {", + " if (targetSdkVersion < Build.VERSION_CODES.DONUT) { }", + " }", + "}") + .doTest(); + } +} diff --git a/errorprone/tests/res/android/os/Build.java b/errorprone/tests/res/android/os/Build.java index bbf7ef2172b54..0b51bdba07310 100644 --- a/errorprone/tests/res/android/os/Build.java +++ b/errorprone/tests/res/android/os/Build.java @@ -19,5 +19,6 @@ package android.os; public class Build { public static class VERSION_CODES { public static final int DONUT = 4; + public static final int R = 30; } }