Skip to content

Commit 554546c

Browse files
committed
Throw if @subscribe is applied to a method that takes a primitive parameter.
Fixes #3992. RELNOTES=Prevent @subscribe being applied to a method that takes a primitive, as that will never be called. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=328195519
1 parent 9b972a2 commit 554546c

File tree

6 files changed

+68
-10
lines changed

6 files changed

+68
-10
lines changed

android/guava-tests/test/com/google/common/eventbus/EventBusTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,18 @@ public void call(String s) {
289289
assertEquals(1, calls.get());
290290
}
291291

292+
public void testPrimitiveSubscribeFails() {
293+
class SubscribesToPrimitive {
294+
@Subscribe
295+
public void toInt(int i) {}
296+
}
297+
try {
298+
bus.register(new SubscribesToPrimitive());
299+
fail("should have thrown");
300+
} catch (IllegalArgumentException expected) {
301+
}
302+
}
303+
292304
/** Records thrown exception information. */
293305
private static final class RecordingSubscriberExceptionHandler
294306
implements SubscriberExceptionHandler {

android/guava/src/com/google/common/eventbus/Subscribe.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@
2323
/**
2424
* Marks a method as an event subscriber.
2525
*
26-
* <p>The type of event will be indicated by the method's first (and only) parameter. If this
27-
* annotation is applied to methods with zero parameters, or more than one parameter, the object
28-
* containing the method will not be able to register for event delivery from the {@link EventBus}.
26+
* <p>The type of event will be indicated by the method's first (and only) parameter, which cannot
27+
* be primitive. If this annotation is applied to methods with zero parameters, or more than one
28+
* parameter, the object containing the method will not be able to register for event delivery from
29+
* the {@link EventBus}.
2930
*
3031
* <p>Unless also annotated with @{@link AllowConcurrentEvents}, event subscriber methods will be
3132
* invoked serially by each event bus that they are registered with.

android/guava/src/com/google/common/eventbus/SubscriberRegistry.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static com.google.common.base.Preconditions.checkArgument;
1818
import static com.google.common.base.Preconditions.checkNotNull;
19+
import static com.google.common.base.Throwables.throwIfUnchecked;
1920

2021
import com.google.common.annotations.VisibleForTesting;
2122
import com.google.common.base.MoreObjects;
@@ -31,6 +32,7 @@
3132
import com.google.common.collect.Lists;
3233
import com.google.common.collect.Maps;
3334
import com.google.common.collect.Multimap;
35+
import com.google.common.primitives.Primitives;
3436
import com.google.common.reflect.TypeToken;
3537
import com.google.common.util.concurrent.UncheckedExecutionException;
3638
import com.google.j2objc.annotations.Weak;
@@ -170,7 +172,12 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
170172
}
171173

172174
private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
173-
return subscriberMethodsCache.getUnchecked(clazz);
175+
try {
176+
return subscriberMethodsCache.getUnchecked(clazz);
177+
} catch (UncheckedExecutionException e) {
178+
throwIfUnchecked(e.getCause());
179+
throw e;
180+
}
174181
}
175182

176183
private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {
@@ -183,11 +190,20 @@ private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz
183190
Class<?>[] parameterTypes = method.getParameterTypes();
184191
checkArgument(
185192
parameterTypes.length == 1,
186-
"Method %s has @Subscribe annotation but has %s parameters."
193+
"Method %s has @Subscribe annotation but has %s parameters. "
187194
+ "Subscriber methods must have exactly 1 parameter.",
188195
method,
189196
parameterTypes.length);
190197

198+
checkArgument(
199+
!parameterTypes[0].isPrimitive(),
200+
"@Subscribe method %s's parameter is %s. "
201+
+ "Subscriber methods cannot accept primitives. "
202+
+ "Consider changing the parameter to %s.",
203+
method,
204+
parameterTypes[0].getName(),
205+
Primitives.wrap(parameterTypes[0]).getSimpleName());
206+
191207
MethodIdentifier ident = new MethodIdentifier(method);
192208
if (!identifiers.containsKey(ident)) {
193209
identifiers.put(ident, method);

guava-tests/test/com/google/common/eventbus/EventBusTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,18 @@ public void call(String s) {
289289
assertEquals(1, calls.get());
290290
}
291291

292+
public void testPrimitiveSubscribeFails() {
293+
class SubscribesToPrimitive {
294+
@Subscribe
295+
public void toInt(int i) {}
296+
}
297+
try {
298+
bus.register(new SubscribesToPrimitive());
299+
fail("should have thrown");
300+
} catch (IllegalArgumentException expected) {
301+
}
302+
}
303+
292304
/** Records thrown exception information. */
293305
private static final class RecordingSubscriberExceptionHandler
294306
implements SubscriberExceptionHandler {

guava/src/com/google/common/eventbus/Subscribe.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@
2323
/**
2424
* Marks a method as an event subscriber.
2525
*
26-
* <p>The type of event will be indicated by the method's first (and only) parameter. If this
27-
* annotation is applied to methods with zero parameters, or more than one parameter, the object
28-
* containing the method will not be able to register for event delivery from the {@link EventBus}.
26+
* <p>The type of event will be indicated by the method's first (and only) parameter, which cannot
27+
* be primitive. If this annotation is applied to methods with zero parameters, or more than one
28+
* parameter, the object containing the method will not be able to register for event delivery from
29+
* the {@link EventBus}.
2930
*
3031
* <p>Unless also annotated with @{@link AllowConcurrentEvents}, event subscriber methods will be
3132
* invoked serially by each event bus that they are registered with.

guava/src/com/google/common/eventbus/SubscriberRegistry.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static com.google.common.base.Preconditions.checkArgument;
1818
import static com.google.common.base.Preconditions.checkNotNull;
19+
import static com.google.common.base.Throwables.throwIfUnchecked;
1920

2021
import com.google.common.annotations.VisibleForTesting;
2122
import com.google.common.base.MoreObjects;
@@ -31,6 +32,7 @@
3132
import com.google.common.collect.Lists;
3233
import com.google.common.collect.Maps;
3334
import com.google.common.collect.Multimap;
35+
import com.google.common.primitives.Primitives;
3436
import com.google.common.reflect.TypeToken;
3537
import com.google.common.util.concurrent.UncheckedExecutionException;
3638
import com.google.j2objc.annotations.Weak;
@@ -170,7 +172,12 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
170172
}
171173

172174
private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
173-
return subscriberMethodsCache.getUnchecked(clazz);
175+
try {
176+
return subscriberMethodsCache.getUnchecked(clazz);
177+
} catch (UncheckedExecutionException e) {
178+
throwIfUnchecked(e.getCause());
179+
throw e;
180+
}
174181
}
175182

176183
private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {
@@ -183,11 +190,20 @@ private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz
183190
Class<?>[] parameterTypes = method.getParameterTypes();
184191
checkArgument(
185192
parameterTypes.length == 1,
186-
"Method %s has @Subscribe annotation but has %s parameters."
193+
"Method %s has @Subscribe annotation but has %s parameters. "
187194
+ "Subscriber methods must have exactly 1 parameter.",
188195
method,
189196
parameterTypes.length);
190197

198+
checkArgument(
199+
!parameterTypes[0].isPrimitive(),
200+
"@Subscribe method %s's parameter is %s. "
201+
+ "Subscriber methods cannot accept primitives. "
202+
+ "Consider changing the parameter to %s.",
203+
method,
204+
parameterTypes[0].getName(),
205+
Primitives.wrap(parameterTypes[0]).getSimpleName());
206+
191207
MethodIdentifier ident = new MethodIdentifier(method);
192208
if (!identifiers.containsKey(ident)) {
193209
identifiers.put(ident, method);

0 commit comments

Comments
 (0)