Skip to content

Commit

Permalink
Make error-prone checker add custom scopes to default scopes (#308)
Browse files Browse the repository at this point in the history
This adds a new flag `isScopeExclusive` (open to suggestions) that will make the classes supplied in `TypesWithScope` exclusive and will only perform the check on those classes. 

In the absence of that flag, we will *add* the custom scope classes to the `DEFAULT_CLASSES_WITH_LIFECYCLE` instead of ignoring the default ones. 

Part of #298
  • Loading branch information
ShaishavGandhi authored and ZacSweers committed Dec 7, 2018
1 parent f430cd7 commit 1456c73
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@

/**
* Checker for subscriptions not binding to lifecycle in components with lifecycle.
* Use -XepOpt:TypesWithScope flag to add support for custom components with lifecycle.
* Use -XepOpt:TypesWithScope flag to add support for custom types with scope.
* Use -XepOpt:OverrideScopes to only run the EP check on your custom types with scope.
* The sample configuration for Conductor:
* <pre><code>
* -XepOpt:TypesWithScope=com.bluelinelabs.conductor.Controller,android.app.Activity
* -XepOpt:OverrideScopes=<true|false>
* </code></pre>
*/
@AutoService(BugChecker.class)
Expand Down Expand Up @@ -92,9 +94,10 @@ public UseAutoDispose() {
public UseAutoDispose(ErrorProneFlags flags) {
Optional<ImmutableSet<String>> inputClasses =
flags.getList("TypesWithScope").map(ImmutableSet::copyOf);
Optional<Boolean> overrideScopes = flags.getBoolean("OverrideScopes");

ImmutableSet<String> classesWithLifecycle = inputClasses.orElse(DEFAULT_CLASSES_WITH_LIFECYCLE);
matcher = allOf(SUBSCRIBE_METHOD, matcher(classesWithLifecycle));
ImmutableSet<String> classesWithScope = getClassesWithScope(inputClasses, overrideScopes);
matcher = allOf(SUBSCRIBE_METHOD, matcher(classesWithScope));
lenient = flags.getBoolean("Lenient").orElse(false);
}

Expand All @@ -114,6 +117,32 @@ public UseAutoDispose(ErrorProneFlags flags) {
return "https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker";
}

/**
* Return the lifecycle classes on which to apply the Error-Prone check.
*
* @param inputClasses the custom scopes defined by user.
* @param overrideScopes whether the custom scopes are exclusive.
* @return the classes on which to apply the error-prone check.
*/
private static ImmutableSet<String> getClassesWithScope(Optional<ImmutableSet<String>> inputClasses,
Optional<Boolean> overrideScopes) {
if (inputClasses.isPresent()) {
if (overrideScopes.isPresent() && overrideScopes.get()) {
// The custom scopes are exclusive, just return that.
return inputClasses.get();
} else {
// The custom scopes aren't exclusive, so bundle them together with default scopes.
return ImmutableSet.<String>builder()
.addAll(DEFAULT_CLASSES_WITH_LIFECYCLE)
.addAll(inputClasses.get())
.build();
}
} else {
// No custom scopes. Return default scopes.
return DEFAULT_CLASSES_WITH_LIFECYCLE;
}
}

private static Matcher<ExpressionTree> matcher(Set<String> classesWithLifecycle) {
return (Matcher<ExpressionTree>) (tree, state) -> {
ClassTree enclosingClass = findEnclosingNode(state.getPath(), ClassTree.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ public final class UseAutoDisposeTest {
compilationHelper.addSourceFile("UseAutoDisposeDefaultClassPositiveCases.java").doTest();
}

@Test public void test_autodisposePositiveCasesWithDefaultClassGivenCustomTypes() {
compilationHelper.setArgs(ImmutableList.of("-XepOpt:TypesWithScope"
+ "=com.uber.autodispose.errorprone.ComponentWithLifecycle"));
compilationHelper.addSourceFile("UseAutoDisposeDefaultClassPositiveCases.java").doTest();
}

@Test public void test_autodisposeNegativeCasesWithDefaultClassGivenExclusiveScope() {
compilationHelper.setArgs(ImmutableList.of("-XepOpt:TypesWithScope"
+ "=com.uber.autodispose.errorprone.ComponentWithLifecycle", "-XepOpt:OverrideScopes"));
compilationHelper.addSourceFile("UseAutoDisposeNegativeCasesExcluded.java").doTest();
}

@Test public void test_autodisposePositiveCasesWithCustomClass() {
compilationHelper.setArgs(ImmutableList.of("-XepOpt:TypesWithScope"
+ "=com.uber.autodispose.errorprone.ComponentWithLifecycle"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/*
* Copyright (C) 2018. Uber Technologies
*
* 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.uber.autodispose.errorprone;

import com.uber.autodispose.AutoDispose;
import com.uber.autodispose.lifecycle.CorrespondingEventsFunction;
import com.uber.autodispose.lifecycle.LifecycleEndedException;
import com.uber.autodispose.lifecycle.LifecycleScopeProvider;
import com.uber.autodispose.lifecycle.LifecycleScopes;
import com.uber.autodispose.lifecycle.TestLifecycleScopeProvider;
import com.uber.autodispose.lifecycle.TestLifecycleScopeProvider.TestLifecycle;
import io.reactivex.Completable;
import io.reactivex.CompletableSource;
import io.reactivex.Flowable;
import io.reactivex.Maybe;
import io.reactivex.Observable;
import io.reactivex.Single;
import io.reactivex.annotations.CheckReturnValue;
import io.reactivex.annotations.Nullable;
import io.reactivex.observers.TestObserver;
import io.reactivex.subjects.BehaviorSubject;
import io.reactivex.subscribers.TestSubscriber;
import org.reactivestreams.Subscriber;

import static com.uber.autodispose.AutoDispose.autoDisposable;

/**
* Cases that use {@link AutoDispose} and should not fail the {@link UseAutoDispose} check.
*/
public class UseAutoDisposeNegativeCasesExcluded
implements LifecycleScopeProvider<TestLifecycle> {

private final BehaviorSubject<TestLifecycle> lifecycleSubject =
BehaviorSubject.create();

/**
* @return a sequence of lifecycle events.
*/
@CheckReturnValue public Observable<TestLifecycle> lifecycle() {
return lifecycleSubject.hide();
}

/**
* @return a sequence of lifecycle events. It's recommended to back this with a static instance to
* avoid unnecessary object allocation.
*/
@CheckReturnValue
public CorrespondingEventsFunction<TestLifecycle> correspondingEvents() {
return testLifecycle -> {
switch (testLifecycle) {
case STARTED:
return TestLifecycle.STOPPED;
case STOPPED:
throw new LifecycleEndedException();
default:
throw new IllegalStateException("Unknown lifecycle event.");
}
};
}

/**
* @return the last seen lifecycle event, or {@code null} if none.
*/
@Nullable public TestLifecycle peekLifecycle() {
return lifecycleSubject.getValue();
}

@Override public CompletableSource requestScope() throws Exception {
return LifecycleScopes.resolveScopeFromLifecycle(this);
}

public void observable_subscribeWithAutoDispose() {
Observable.just(1)
.as(autoDisposable(this))
.subscribe();
}

public void single_subscribeWithAutoDispose() {
Single.just(1)
.subscribe();
}

public void completable_subscribeWithAutoDispose() {
Completable.complete()
.subscribe();
}

public void maybe_subscribeWithAutoDispose() {
Maybe.just(1)
.subscribe();
}

public void flowable_subscribeWithAutoDispose() {
Flowable.just(1)
.subscribe();
}

public void parallelFlowable_subscribeWithAutoDispose() {
Subscriber<Integer>[] subscribers = new Subscriber[] {};
Flowable.just(1, 2)
.parallel(2)
.subscribe(subscribers);
}

public void observable_subscribeVoidSubscribe() {
Observable.just(1)
.subscribe(new TestObserver<>());
}

public void single_subscribeVoidSubscribe() {
Single.just(1)
.subscribe(new TestObserver<>());
}

public void completable_subscribeVoidSubscribe() {
Completable.complete()
.subscribe(new TestObserver<>());
}

public void maybe_subscribeVoidSubscribe() {
Maybe.just(1)
.subscribe(new TestObserver<>());
}

public void flowable_subscribeVoidSubscribe() {
Flowable.just(1)
.subscribe(new TestSubscriber<>());
}

public void observable_subscribeWith() {
Observable.just(1)
.subscribeWith(new TestObserver<>());
}

public void single_subscribeWith() {
Single.just(1)
.subscribeWith(new TestObserver<>());
}

public void completable_subscribeWith() {
Completable.complete()
.subscribeWith(new TestObserver<>());
}

public void maybe_subscribeWith() {
Maybe.just(1)
.subscribeWith(new TestObserver<>());
}

public void flowable_subscribeWith() {
Flowable.just(1)
.subscribeWith(new TestSubscriber<>());
}
}

0 comments on commit 1456c73

Please sign in to comment.