Skip to content

Commit

Permalink
Add WARNING check for Parcelable Creator absence
Browse files Browse the repository at this point in the history
RELNOTES: Adds a new check ParcelableCreator which enforces the spec for Parcelable implementation as illustrated on https://developer.android.com/reference/android/os/Parcelable.html

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=192971191
  • Loading branch information
sumitbhagwani authored and eaftan committed Apr 16, 2018
1 parent dbb1cb9 commit a6a8d8b
Show file tree
Hide file tree
Showing 7 changed files with 400 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* 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.bugpatterns.android;

import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.isDirectImplementationOf;
import static com.google.errorprone.matchers.Matchers.not;

import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.Category;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.ClassType;
import com.sun.tools.javac.code.Types;
import java.util.List;
import javax.lang.model.element.Modifier;

/**
* BugPattern to detect classes which implement android.os.Parcelable but don't have public static
* CREATOR.
*
* @author Sumit Bhagwani ([email protected])
*/
@BugPattern(
name = "ParcelableCreator",
summary = "Detects classes which implement Parcelable but don't have CREATOR",
category = Category.ANDROID,
severity = SeverityLevel.WARNING,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION
)
public class ParcelableCreator extends BugChecker implements ClassTreeMatcher {

/** Matches if a non-public non-abstract class/interface is subtype of android.os.Parcelable */
private static final Matcher<ClassTree> PARCELABLE_MATCHER =
allOf(isDirectImplementationOf("android.os.Parcelable"), not(hasModifier(Modifier.ABSTRACT)));

private static final Matcher<VariableTree> PARCELABLE_CREATOR_MATCHER =
allOf(
Matchers.isSubtypeOf("android.os.Parcelable$Creator"),
hasModifier(Modifier.STATIC),
hasModifier(Modifier.PUBLIC));

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!PARCELABLE_MATCHER.matches(tree, state)) {
return Description.NO_MATCH;
}

Symbol parcelableCreatorSymbol = state.getSymbolFromString("android.os.Parcelable$Creator");
if (parcelableCreatorSymbol == null) {
return Description.NO_MATCH;
}

ClassType classType = ASTHelpers.getType(tree);
for (Tree member : tree.getMembers()) {
if (member.getKind() != Kind.VARIABLE) {
continue;
}

VariableTree variableTree = (VariableTree) member;
if (PARCELABLE_CREATOR_MATCHER.matches(variableTree, state)) {
if (isVariableClassCreator(variableTree, state, classType, parcelableCreatorSymbol)) {
return Description.NO_MATCH;
}
}
}

return describeMatch(tree);
}

private static boolean isVariableClassCreator(
VariableTree variableTree,
VisitorState state,
ClassType classType,
Symbol parcelableCreatorSymbol) {
Tree typeTree = variableTree.getType();
Type type = ASTHelpers.getType(typeTree);
Types types = state.getTypes();
Type superType = types.asSuper(type, parcelableCreatorSymbol);
if (superType == null) {
return false;
}
List<Type> typeArguments = superType.getTypeArguments();
if (typeArguments.isEmpty()) {
return false;
}
return ASTHelpers.isSubtype(classType, Iterables.getOnlyElement(typeArguments), state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@
import com.google.errorprone.bugpatterns.android.HardCodedSdCardPath;
import com.google.errorprone.bugpatterns.android.IsLoggableTagLength;
import com.google.errorprone.bugpatterns.android.MislabeledAndroidString;
import com.google.errorprone.bugpatterns.android.ParcelableCreator;
import com.google.errorprone.bugpatterns.android.RectIntersectReturnValueIgnored;
import com.google.errorprone.bugpatterns.android.RestrictToEnforcer;
import com.google.errorprone.bugpatterns.android.StaticOrDefaultInterfaceMethod;
Expand Down Expand Up @@ -519,6 +520,7 @@ public static ScannerSupplier errorChecks() {
OverridesGuiceInjectableMethod.class,
OverrideThrowableToString.class,
ParameterName.class,
ParcelableCreator.class,
PreconditionsInvalidPlaceholder.class,
ProtoFieldPreconditionsCheckNotNull.class,
QualifierOrScopeOnInjectMethod.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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.bugpatterns.android;

import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Tests for {@link ParcelableCreator}.
*
* @author [email protected] (Sumit Bhagwani)
*/
@RunWith(JUnit4.class)
public class ParcelableCreatorTest {

private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(ParcelableCreator.class, getClass())
.addSourceFile("testdata/stubs/android/os/Parcel.java")
.addSourceFile("testdata/stubs/android/os/Parcelable.java");

@Test
public void testCases() throws Exception {
compilationHelper
.addSourceFile("ParcelableCreatorPositiveCases.java")
.addSourceFile("ParcelableCreatorNegativeCases.java")
.doTest();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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.bugpatterns.android.testdata;

import android.os.Parcel;
import android.os.Parcelable;
import android.os.Parcelable.ClassLoaderCreator;
import com.google.errorprone.bugpatterns.android.ParcelableCreator;

/** @author [email protected] (Sumit Bhagwani) */
public class ParcelableCreatorNegativeCases {

public static class PublicParcelableClass implements Parcelable {

public static final Parcelable.Creator<PublicParcelableClass> CREATOR =
new Parcelable.Creator<PublicParcelableClass>() {
public PublicParcelableClass createFromParcel(Parcel in) {
return new PublicParcelableClass();
}

public PublicParcelableClass[] newArray(int size) {
return new PublicParcelableClass[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}

public static class PublicParcelableClassWithClassLoaderCreator implements Parcelable {

public static final ClassLoaderCreator<PublicParcelableClassWithClassLoaderCreator> CREATOR =
new ClassLoaderCreator<PublicParcelableClassWithClassLoaderCreator>() {
@Override
public PublicParcelableClassWithClassLoaderCreator createFromParcel(
Parcel source, ClassLoader loader) {
return new PublicParcelableClassWithClassLoaderCreator();
}

@Override
public PublicParcelableClassWithClassLoaderCreator createFromParcel(Parcel source) {
return new PublicParcelableClassWithClassLoaderCreator();
}

@Override
public PublicParcelableClassWithClassLoaderCreator[] newArray(int size) {
return new PublicParcelableClassWithClassLoaderCreator[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}

public interface EmptyInterface {}

public static class EmptyInterfaceAndParcelableImpl implements EmptyInterface, Parcelable {
public static final Parcelable.Creator<EmptyInterface> CREATOR =
new Parcelable.Creator<EmptyInterface>() {
public EmptyInterfaceAndParcelableImpl createFromParcel(Parcel in) {
return new EmptyInterfaceAndParcelableImpl();
}

public EmptyInterfaceAndParcelableImpl[] newArray(int size) {
return new EmptyInterfaceAndParcelableImpl[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* 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.bugpatterns.android.testdata;

import android.os.Parcel;
import android.os.Parcelable;

/** @author [email protected] (Sumit Bhagwani) */
public class ParcelableCreatorPositiveCases {

// BUG: Diagnostic contains: ParcelableCreator
public static class ParcelableClassWithoutStaticCreator implements Parcelable {

public final Parcelable.Creator<ParcelableClassWithoutStaticCreator> CREATOR =
new Parcelable.Creator<ParcelableClassWithoutStaticCreator>() {
public ParcelableClassWithoutStaticCreator createFromParcel(Parcel in) {
return new ParcelableClassWithoutStaticCreator();
}

public ParcelableClassWithoutStaticCreator[] newArray(int size) {
return new ParcelableClassWithoutStaticCreator[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}

// BUG: Diagnostic contains: ParcelableCreator
public static class ParcelableClassWithoutPublicCreator implements Parcelable {

static final Parcelable.Creator<ParcelableClassWithoutPublicCreator> CREATOR =
new Parcelable.Creator<ParcelableClassWithoutPublicCreator>() {
public ParcelableClassWithoutPublicCreator createFromParcel(Parcel in) {
return new ParcelableClassWithoutPublicCreator();
}

public ParcelableClassWithoutPublicCreator[] newArray(int size) {
return new ParcelableClassWithoutPublicCreator[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}

// BUG: Diagnostic contains: ParcelableCreator
public static class ParcelableClassWithCreatorOfWrongType implements Parcelable {

public static final Parcelable.Creator<ParcelableClassWithoutPublicCreator> CREATOR =
new Parcelable.Creator<ParcelableClassWithoutPublicCreator>() {
public ParcelableClassWithoutPublicCreator createFromParcel(Parcel in) {
return new ParcelableClassWithoutPublicCreator();
}

public ParcelableClassWithoutPublicCreator[] newArray(int size) {
return new ParcelableClassWithoutPublicCreator[size];
}
};

public int describeContents() {
return 0;
}

public void writeToParcel(Parcel dest, int flags) {
// no op
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ interface Creator<T> {
int describeContents();

void writeToParcel(Parcel dest, int flags);

interface ClassLoaderCreator<T> extends Creator<T> {
T createFromParcel(Parcel source, ClassLoader loader);
}
}
Loading

0 comments on commit a6a8d8b

Please sign in to comment.