From 0460653545ef3f69c034c619f8149be3a111dab8 Mon Sep 17 00:00:00 2001 From: Giovanni Zotta Date: Mon, 8 May 2023 00:47:03 +0200 Subject: [PATCH 1/5] Initial implementation --- .../errorprone/refasterrules/ReactorRules.java | 16 ++++++++++++++++ .../refasterrules/ReactorRulesTestInput.java | 8 ++++++++ .../refasterrules/ReactorRulesTestOutput.java | 4 ++++ 3 files changed, 28 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index 5653673bca..3f0d2fc50d 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -1434,4 +1434,20 @@ Duration after(StepVerifier.LastStep step, Duration duration) { return step.verifyTimeout(duration); } } + + /** Avoid collecting when verifying only the given element is in the given {@link Flux}. */ + static final class VerifyOnlyElementInFlux { + @BeforeTemplate + Duration before(Flux flux, T object) { + return flux.collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(object)) + .verifyComplete(); + } + + @AfterTemplate + Duration after(Flux flux, T object) { + return flux.as(StepVerifier::create).expectNext(object).verifyComplete(); + } + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index 1317776baa..18e9192d80 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -454,4 +454,12 @@ Duration testStepVerifierLastStepVerifyErrorMessage() { Duration testStepVerifierLastStepVerifyTimeout() { return StepVerifier.create(Mono.empty()).expectTimeout(Duration.ZERO).verify(); } + + Duration testVerifyOnlyElementInFlux() { + return Flux.just(1) + .collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(1)) + .verifyComplete(); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index d52b8dd180..252a11ed33 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -443,4 +443,8 @@ Duration testStepVerifierLastStepVerifyErrorMessage() { Duration testStepVerifierLastStepVerifyTimeout() { return StepVerifier.create(Mono.empty()).verifyTimeout(Duration.ZERO); } + + Duration testVerifyOnlyElementInFlux() { + return Flux.just(1).as(StepVerifier::create).expectNext(1).verifyComplete(); + } } From 0b94faad5a21dc87400578ac0410452b2b368f06 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Tue, 9 May 2023 14:25:33 +0200 Subject: [PATCH 2/5] Suggestions --- .../refasterrules/ReactorRules.java | 38 +++++++++++-------- .../refasterrules/ReactorRulesTestInput.java | 23 +++++++---- .../refasterrules/ReactorRulesTestOutput.java | 10 +++-- 3 files changed, 43 insertions(+), 28 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index 3f0d2fc50d..7851875e03 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -1332,6 +1332,28 @@ StepVerifier.Step after(StepVerifier.Step step, T object) { } } + /** Avoid collecting when verifying only the given element is in the given {@link Flux}. */ + static final class FluxAsStepVerifierExpectNext { + @BeforeTemplate + StepVerifier.Step> before(Flux flux, T object) { + return flux.collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(object)); + } + + @BeforeTemplate + StepVerifier.Step> before2(Flux flux, T object) { + return flux.collect(toImmutableSet()) + .as(StepVerifier::create) + .assertNext(set -> assertThat(set).containsExactly(object)); + } + + @AfterTemplate + StepVerifier.Step after(Flux flux, T object) { + return flux.as(StepVerifier::create).expectNext(object); + } + } + /** Prefer {@link StepVerifier.LastStep#verifyComplete()} over more verbose alternatives. */ static final class StepVerifierLastStepVerifyComplete { @BeforeTemplate @@ -1434,20 +1456,4 @@ Duration after(StepVerifier.LastStep step, Duration duration) { return step.verifyTimeout(duration); } } - - /** Avoid collecting when verifying only the given element is in the given {@link Flux}. */ - static final class VerifyOnlyElementInFlux { - @BeforeTemplate - Duration before(Flux flux, T object) { - return flux.collect(toImmutableList()) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(object)) - .verifyComplete(); - } - - @AfterTemplate - Duration after(Flux flux, T object) { - return flux.as(StepVerifier::create).expectNext(object).verifyComplete(); - } - } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index 18e9192d80..74f44d061b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.refasterrules; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.MoreCollectors.toOptional; import static java.util.Comparator.reverseOrder; import static java.util.function.Function.identity; @@ -422,6 +423,20 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNextMatches("qux"::equals)); } + ImmutableSet testFluxAsStepVerifierExpectNext() { + return ImmutableSet.of( + Flux.just(1) + .collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(2)) + .verifyComplete(), + Flux.just(3) + .collect(toImmutableSet()) + .as(StepVerifier::create) + .assertNext(set -> assertThat(set).containsExactly(4)) + .verifyComplete()); + } + Duration testStepVerifierLastStepVerifyComplete() { return StepVerifier.create(Mono.empty()).expectComplete().verify(); } @@ -454,12 +469,4 @@ Duration testStepVerifierLastStepVerifyErrorMessage() { Duration testStepVerifierLastStepVerifyTimeout() { return StepVerifier.create(Mono.empty()).expectTimeout(Duration.ZERO).verify(); } - - Duration testVerifyOnlyElementInFlux() { - return Flux.just(1) - .collect(toImmutableList()) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(1)) - .verifyComplete(); - } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index 252a11ed33..1136b736a5 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -413,6 +413,12 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNext("qux")); } + ImmutableSet testFluxAsStepVerifierExpectNext() { + return ImmutableSet.of( + Flux.just(1).as(StepVerifier::create).expectNext(2).verifyComplete(), + Flux.just(3).as(StepVerifier::create).expectNext(4).verifyComplete()); + } + Duration testStepVerifierLastStepVerifyComplete() { return StepVerifier.create(Mono.empty()).verifyComplete(); } @@ -443,8 +449,4 @@ Duration testStepVerifierLastStepVerifyErrorMessage() { Duration testStepVerifierLastStepVerifyTimeout() { return StepVerifier.create(Mono.empty()).verifyTimeout(Duration.ZERO); } - - Duration testVerifyOnlyElementInFlux() { - return Flux.just(1).as(StepVerifier::create).expectNext(1).verifyComplete(); - } } From 124364485706d657ed15f9c1be900de576560ca3 Mon Sep 17 00:00:00 2001 From: Giovanni Zotta Date: Fri, 14 Jul 2023 23:02:29 +0200 Subject: [PATCH 3/5] Remove `toImmutableSet()` rule --- .../refasterrules/ReactorRules.java | 9 +-------- .../refasterrules/ReactorRulesTestInput.java | 19 ++++++------------- .../refasterrules/ReactorRulesTestOutput.java | 6 ++---- 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index 7851875e03..da8c75af12 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -1332,7 +1332,7 @@ StepVerifier.Step after(StepVerifier.Step step, T object) { } } - /** Avoid collecting when verifying only the given element is in the given {@link Flux}. */ + /** Avoid value collection when verifying that a {@link Flux} emits exactly one value. */ static final class FluxAsStepVerifierExpectNext { @BeforeTemplate StepVerifier.Step> before(Flux flux, T object) { @@ -1341,13 +1341,6 @@ StepVerifier.Step> before(Flux flux, T object) { .assertNext(list -> assertThat(list).containsExactly(object)); } - @BeforeTemplate - StepVerifier.Step> before2(Flux flux, T object) { - return flux.collect(toImmutableSet()) - .as(StepVerifier::create) - .assertNext(set -> assertThat(set).containsExactly(object)); - } - @AfterTemplate StepVerifier.Step after(Flux flux, T object) { return flux.as(StepVerifier::create).expectNext(object); diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index 74f44d061b..07bfd9d45d 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -1,7 +1,6 @@ package tech.picnic.errorprone.refasterrules; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.MoreCollectors.toOptional; import static java.util.Comparator.reverseOrder; import static java.util.function.Function.identity; @@ -423,18 +422,12 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNextMatches("qux"::equals)); } - ImmutableSet testFluxAsStepVerifierExpectNext() { - return ImmutableSet.of( - Flux.just(1) - .collect(toImmutableList()) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(2)) - .verifyComplete(), - Flux.just(3) - .collect(toImmutableSet()) - .as(StepVerifier::create) - .assertNext(set -> assertThat(set).containsExactly(4)) - .verifyComplete()); + Duration testFluxAsStepVerifierExpectNext() { + return Flux.just(1) + .collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(2)) + .verifyComplete(); } Duration testStepVerifierLastStepVerifyComplete() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index 1136b736a5..7d6cb09898 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -413,10 +413,8 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNext("qux")); } - ImmutableSet testFluxAsStepVerifierExpectNext() { - return ImmutableSet.of( - Flux.just(1).as(StepVerifier::create).expectNext(2).verifyComplete(), - Flux.just(3).as(StepVerifier::create).expectNext(4).verifyComplete()); + Duration testFluxAsStepVerifierExpectNext() { + return Flux.just(1).as(StepVerifier::create).expectNext(2).verifyComplete(); } Duration testStepVerifierLastStepVerifyComplete() { From 1d52d89c403dcb120c26e498fa3fdc1857fa1b6a Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Mon, 14 Aug 2023 09:56:19 +0200 Subject: [PATCH 4/5] Suggestions --- .../errorprone/refasterrules/ReactorRules.java | 10 ++++++---- .../refasterrules/ReactorRulesTestInput.java | 16 ++++++++++------ .../refasterrules/ReactorRulesTestOutput.java | 6 ++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index da8c75af12..c92668319f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -1332,11 +1332,13 @@ StepVerifier.Step after(StepVerifier.Step step, T object) { } } - /** Avoid value collection when verifying that a {@link Flux} emits exactly one value. */ - static final class FluxAsStepVerifierExpectNext { + /** Avoid list collection when verifying that a {@link Flux} emits exactly one value. */ + // XXX: This rule assumes that the matched collector does not drop elements. Consider introducing + // a `@Matches(DoesNotDropElements.class)` or `@NotMatches(MayDropElements.class)` guard. + static final class FluxAsStepVerifierExpectNext> { @BeforeTemplate - StepVerifier.Step> before(Flux flux, T object) { - return flux.collect(toImmutableList()) + StepVerifier.Step before(Flux flux, Collector listCollector, T object) { + return flux.collect(listCollector) .as(StepVerifier::create) .assertNext(list -> assertThat(list).containsExactly(object)); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index 07bfd9d45d..4bb35ca921 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -422,12 +422,16 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNextMatches("qux"::equals)); } - Duration testFluxAsStepVerifierExpectNext() { - return Flux.just(1) - .collect(toImmutableList()) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(2)) - .verifyComplete(); + ImmutableSet> testFluxAsStepVerifierExpectNext() { + return ImmutableSet.of( + Flux.just(1) + .collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(2)), + Flux.just(3) + .collect(toCollection(ArrayList::new)) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(4))); } Duration testStepVerifierLastStepVerifyComplete() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index 7d6cb09898..099daa1bd6 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -413,8 +413,10 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNext("qux")); } - Duration testFluxAsStepVerifierExpectNext() { - return Flux.just(1).as(StepVerifier::create).expectNext(2).verifyComplete(); + ImmutableSet> testFluxAsStepVerifierExpectNext() { + return ImmutableSet.of( + Flux.just(1).as(StepVerifier::create).expectNext(2), + Flux.just(3).as(StepVerifier::create).expectNext(4)); } Duration testStepVerifierLastStepVerifyComplete() { From db04a3e3c30d7bd63632e0661a9118e7c306b04c Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 14 Aug 2023 13:35:03 +0200 Subject: [PATCH 5/5] Drop testcase --- .../refasterrules/ReactorRulesTestInput.java | 15 +++++---------- .../refasterrules/ReactorRulesTestOutput.java | 6 ++---- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index 4bb35ca921..7f8021f70c 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -422,16 +422,11 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNextMatches("qux"::equals)); } - ImmutableSet> testFluxAsStepVerifierExpectNext() { - return ImmutableSet.of( - Flux.just(1) - .collect(toImmutableList()) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(2)), - Flux.just(3) - .collect(toCollection(ArrayList::new)) - .as(StepVerifier::create) - .assertNext(list -> assertThat(list).containsExactly(4))); + StepVerifier.Step testFluxAsStepVerifierExpectNext() { + return Flux.just(1) + .collect(toImmutableList()) + .as(StepVerifier::create) + .assertNext(list -> assertThat(list).containsExactly(2)); } Duration testStepVerifierLastStepVerifyComplete() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index 099daa1bd6..b12af508c4 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -413,10 +413,8 @@ ImmutableSet> testStepVerifierStepExpectNext() { StepVerifier.create(Mono.just("baz")).expectNext("qux")); } - ImmutableSet> testFluxAsStepVerifierExpectNext() { - return ImmutableSet.of( - Flux.just(1).as(StepVerifier::create).expectNext(2), - Flux.just(3).as(StepVerifier::create).expectNext(4)); + StepVerifier.Step testFluxAsStepVerifierExpectNext() { + return Flux.just(1).as(StepVerifier::create).expectNext(2); } Duration testStepVerifierLastStepVerifyComplete() {