From 0c0ac67c0b221bd424787325f3648716035e4001 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 25 Sep 2020 10:59:55 -0600 Subject: [PATCH] Add checker for inefficient Parcel usage. Parcelable data can be transported in many ways (some of which can be very inefficient) so this checker guides developers towards using high-performance best-practices. Bug: 154436100, 155703208 Test: atest error_prone_android_framework_test Change-Id: I253b5e1088c9bf9c3cf0d684cf73134f3bbf27ab --- .../android/ParcelablePerformanceChecker.java | 123 ++++++++++++++++++ .../ParcelablePerformanceCheckerTest.java | 105 +++++++++++++++ errorprone/tests/res/android/os/Parcel.java | 57 ++++++++ .../tests/res/android/os/Parcelable.java | 21 +++ 4 files changed, 306 insertions(+) create mode 100644 errorprone/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceChecker.java create mode 100644 errorprone/tests/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceCheckerTest.java create mode 100644 errorprone/tests/res/android/os/Parcel.java create mode 100644 errorprone/tests/res/android/os/Parcelable.java diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceChecker.java new file mode 100644 index 0000000000000..d5243164abdc1 --- /dev/null +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceChecker.java @@ -0,0 +1,123 @@ +/* + * 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.enclosingClass; +import static com.google.errorprone.matchers.Matchers.enclosingMethod; +import static com.google.errorprone.matchers.Matchers.instanceMethod; +import static com.google.errorprone.matchers.Matchers.isSubtypeOf; +import static com.google.errorprone.matchers.Matchers.methodInvocation; +import static com.google.errorprone.matchers.Matchers.methodIsNamed; + +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.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; + +/** + * Parcelable data can be transported in many ways (some of which can be very + * inefficient) so this checker guides developers towards using high-performance + * best-practices. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "AndroidFrameworkParcelablePerformance", + summary = "Verifies Parcelable performance best-practices", + severity = WARNING) +public final class ParcelablePerformanceChecker extends BugChecker + implements MethodInvocationTreeMatcher { + private static final Matcher INSIDE_WRITE_TO_PARCEL = allOf( + enclosingClass(isSubtypeOf("android.os.Parcelable")), + enclosingMethod(methodIsNamed("writeToParcel"))); + + private static final Matcher WRITE_STRING = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeString")); + private static final Matcher WRITE_STRING_ARRAY = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeStringArray")); + + private static final Matcher WRITE_VALUE = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeValue")); + private static final Matcher WRITE_PARCELABLE = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeParcelable")); + + private static final Matcher WRITE_LIST = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeList")); + private static final Matcher WRITE_PARCELABLE_LIST = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeParcelableList")); + private static final Matcher WRITE_PARCELABLE_ARRAY = methodInvocation( + instanceMethod().onExactClass("android.os.Parcel").named("writeParcelableArray")); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (INSIDE_WRITE_TO_PARCEL.matches(tree, state)) { + if (WRITE_STRING.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'writeString8()' to improve " + + "efficiency; sending as UTF-8 can double throughput") + .build(); + } + if (WRITE_STRING_ARRAY.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'writeString8Array()' to improve " + + "efficiency; sending as UTF-8 can double throughput") + .build(); + } + + if (WRITE_VALUE.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use strongly-typed methods to improve " + + "efficiency; saves 4 bytes for type and overhead of " + + "Parcelable class name") + .build(); + } + if (WRITE_PARCELABLE.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'item.writeToParcel()' to improve " + + "efficiency; saves overhead of Parcelable class name") + .build(); + } + + if (WRITE_LIST.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'writeTypedList()' to improve " + + "efficiency; saves overhead of repeated Parcelable class name") + .build(); + } + if (WRITE_PARCELABLE_LIST.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'writeTypedList()' to improve " + + "efficiency; saves overhead of repeated Parcelable class name") + .build(); + } + if (WRITE_PARCELABLE_ARRAY.matches(tree, state)) { + return buildDescription(tree) + .setMessage("Recommended to use 'writeTypedArray()' to improve " + + "efficiency; saves overhead of repeated Parcelable class name") + .build(); + } + } + return Description.NO_MATCH; + } +} diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceCheckerTest.java new file mode 100644 index 0000000000000..75c76e32f00e5 --- /dev/null +++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ParcelablePerformanceCheckerTest.java @@ -0,0 +1,105 @@ +/* + * 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 ParcelablePerformanceCheckerTest { + private CompilationTestHelper compilationHelper; + + @Before + public void setUp() { + compilationHelper = CompilationTestHelper.newInstance( + ParcelablePerformanceChecker.class, getClass()); + } + + @Test + public void testString() { + compilationHelper + .addSourceFile("/android/os/Parcel.java") + .addSourceFile("/android/os/Parcelable.java") + .addSourceLines("FooInfo.java", + "import android.os.Parcel;", + "import android.os.Parcelable;", + "public class FooInfo implements Parcelable {", + " public void writeToParcel(Parcel dest, int flags) {", + " // BUG: Diagnostic contains:", + " dest.writeString(toString());", + " dest.writeString8(toString());", + " // BUG: Diagnostic contains:", + " dest.writeStringArray(new String[0]);", + " dest.writeString8Array(new String[0]);", + " }", + "}") + .doTest(); + } + + @Test + public void testSingle() { + compilationHelper + .addSourceFile("/android/os/Parcel.java") + .addSourceFile("/android/os/Parcelable.java") + .addSourceLines("FooInfo.java", + "import android.os.Parcel;", + "import android.os.Parcelable;", + "public class FooInfo implements Parcelable {", + " public void writeToParcel(Parcel dest, int flags) {", + " // BUG: Diagnostic contains:", + " dest.writeValue(this);", + " this.writeToParcel(dest, flags);", + " // BUG: Diagnostic contains:", + " dest.writeParcelable(this, flags);", + " this.writeToParcel(dest, flags);", + " }", + "}") + .doTest(); + } + + @Test + public void testList() { + compilationHelper + .addSourceFile("/android/os/Parcel.java") + .addSourceFile("/android/os/Parcelable.java") + .addSourceLines("FooInfo.java", + "import android.os.Parcel;", + "import android.os.Parcelable;", + "import java.util.List;", + "import java.util.ArrayList;", + "public class FooInfo implements Parcelable {", + " public void writeToParcel(Parcel dest, int flags) {", + " List list = new ArrayList();", + " Parcelable[] array = new Parcelable[0];", + " // BUG: Diagnostic contains:", + " dest.writeList(list);", + " dest.writeTypedList(list, flags);", + " // BUG: Diagnostic contains:", + " dest.writeParcelableList(list, flags);", + " dest.writeTypedList(list, flags);", + " // BUG: Diagnostic contains:", + " dest.writeParcelableArray(array, flags);", + " dest.writeTypedArray(array, flags);", + " }", + "}") + .doTest(); + } +} diff --git a/errorprone/tests/res/android/os/Parcel.java b/errorprone/tests/res/android/os/Parcel.java new file mode 100644 index 0000000000000..bafa23626fb28 --- /dev/null +++ b/errorprone/tests/res/android/os/Parcel.java @@ -0,0 +1,57 @@ +/* + * 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 android.os; + +import java.util.List; + +public class Parcel { + public void writeString(String val) { + throw new UnsupportedOperationException(); + } + public void writeString8(String val) { + throw new UnsupportedOperationException(); + } + public final void writeStringArray(String[] val) { + throw new UnsupportedOperationException(); + } + public final void writeString8Array(String[] val) { + throw new UnsupportedOperationException(); + } + + public final void writeValue(Object v) { + throw new UnsupportedOperationException(); + } + public final void writeParcelable(Parcelable p, int flags) { + throw new UnsupportedOperationException(); + } + + public final void writeList(List val) { + throw new UnsupportedOperationException(); + } + public final void writeParcelableList(List val, int flags) { + throw new UnsupportedOperationException(); + } + public void writeTypedList(List val, int flags) { + throw new UnsupportedOperationException(); + } + public final void writeParcelableArray(T[] value, int flags) { + throw new UnsupportedOperationException(); + } + public final void writeTypedArray(T[] val, int flags) { + throw new UnsupportedOperationException(); + } +} diff --git a/errorprone/tests/res/android/os/Parcelable.java b/errorprone/tests/res/android/os/Parcelable.java new file mode 100644 index 0000000000000..217690d69fd65 --- /dev/null +++ b/errorprone/tests/res/android/os/Parcelable.java @@ -0,0 +1,21 @@ +/* + * 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 android.os; + +public interface Parcelable { + public void writeToParcel(Parcel dest, int flags); +}