Skip to content

Commit 8865ac4

Browse files
dx404ronshapiro
authored andcommitted
Add Check for Leaking/Mutating an already-forked mutable instance (Bundle)
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242811734
1 parent b0218c1 commit 8865ac4

File tree

5 files changed

+96
-1
lines changed

5 files changed

+96
-1
lines changed

check_api/src/main/java/com/google/errorprone/matchers/Matchers.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,15 @@ public boolean matches(T t, VisitorState state) {
130130
*/
131131
@SafeVarargs
132132
public static <T extends Tree> Matcher<T> allOf(final Matcher<? super T>... matchers) {
133+
return Matchers.<T>allOf(ImmutableList.copyOf(matchers));
134+
}
135+
136+
/**
137+
* Compose several matchers together, such that the composite matches an AST node iff all the
138+
* given matchers do.
139+
*/
140+
public static <T extends Tree> Matcher<T> allOf(
141+
final Iterable<? extends Matcher<? super T>> matchers) {
133142
return new Matcher<T>() {
134143
@Override
135144
public boolean matches(T t, VisitorState state) {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright 2019 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package android.location;
18+
19+
import android.os.Bundle;
20+
21+
public class Location {
22+
23+
public Location(String provider) {
24+
throw new RuntimeException("Stub!");
25+
}
26+
27+
public Location(Location l) {
28+
throw new RuntimeException("Stub!");
29+
}
30+
31+
public android.os.Bundle getExtras() {
32+
throw new RuntimeException("Stub!");
33+
}
34+
35+
public void setExtras(Bundle extras) {
36+
throw new RuntimeException("Stub!");
37+
}
38+
}

core/src/test/java/com/google/errorprone/bugpatterns/android/testdata/stubs/android/os/Bundle.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,8 @@ public Bundle() {}
2525
public Serializable getSerializable(String key) {
2626
return null;
2727
}
28+
29+
public void putCharSequence(String key, CharSequence value) {
30+
throw new RuntimeException("Stub!");
31+
}
2832
}

core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/testdata/stubs/android/os/Bundle.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,17 @@
1616

1717
package android.os;
1818

19-
public class Bundle {}
19+
import java.io.Serializable;
20+
21+
/** Stub for android.os.Bundle */
22+
public class Bundle {
23+
public Bundle() {}
24+
25+
public Serializable getSerializable(String key) {
26+
return null;
27+
}
28+
29+
public void putCharSequence(String key, CharSequence value) {
30+
throw new RuntimeException("Stub!");
31+
}
32+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
> When passing a mutable instance into a method which makes a deep copy of the
2+
> instance, it results in discrepancies between the forked instances. It is
3+
> prone to errors if one expects the state change on the original instance to be
4+
> reflected on the method with its forked instance.
5+
6+
7+
```java {.bad}
8+
Location location = new Location("gps");
9+
Bundle bundle = new Bundle();
10+
bundle.putFloat("someFloat", 12.3f);
11+
location.setExtras(bundle);
12+
13+
// Now add more things to the bundle, but it won't modify the internal
14+
// representation stored by Location.
15+
bundle.putInt("someInt", 7);
16+
```
17+
18+
```java {.bad}
19+
private static Bundle getLocationExtras(Location location) {
20+
Bundle bundle = location.getExtras();
21+
if (bundle != null) {
22+
return bundle;
23+
}
24+
bundle = new Bundle();
25+
location.setExtras(bundle);
26+
27+
// Now leaks the bundle which is subject to modification in its
28+
// method of invocation.
29+
return bundle;
30+
}
31+
```

0 commit comments

Comments
 (0)