Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@ static final class RandomGeneratorNextDouble {
}
}

/**
* Prefer {@link RandomGenerator#nextDouble(double origin, double bound)} over alternatives that
* may silently yield an ununiform domain of values.
*/
// XXX: This rule assumes that `a` is not an expensive or side-effectful expression.
// XXX: The replacement code throws an `IllegalArgumentException` in more cases than the original
// code, but only in situations that are likely unintended.
static final class RandomGeneratorNextDoubleWithOrigin {
@BeforeTemplate
double before(RandomGenerator random, double a, double b) {
return a + random.nextDouble(b);
}

@AfterTemplate
double after(RandomGenerator random, double a, double b) {
return random.nextDouble(a, a + b);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document CodeRabbit's feedback about a having side-effects or being expensive to compute.

}
}

/** Prefer {@link RandomGenerator#nextInt(int)} over more contrived alternatives. */
static final class RandomGeneratorNextInt {
@BeforeTemplate
Expand All @@ -46,6 +65,25 @@ int after(RandomGenerator random, int bound) {
}
}

/**
* Prefer {@link RandomGenerator#nextInt(int origin, int bound)} over alternatives that may
* silently yield values outside the intended domain.
*/
// XXX: This rule assumes that `a` is not an expensive or side-effectful expression.
// XXX: The replacement code throws an `IllegalArgumentException` in more cases than the original
// code, but only in situations that are likely unintended.
static final class RandomGeneratorNextIntWithOrigin {
@BeforeTemplate
int before(RandomGenerator random, int a, int b) {
return a + random.nextInt(b);
}

@AfterTemplate
int after(RandomGenerator random, int a, int b) {
return random.nextInt(a, a + b);
}
}

/**
* Prefer {@link RandomGenerator#nextLong(long)} over more contrived alternatives.
*
Expand Down Expand Up @@ -75,4 +113,23 @@ long after(RandomGenerator random, long bound) {
return random.nextLong(bound);
}
}

/**
* Prefer {@link RandomGenerator#nextLong(long origin, long bound)} over more contrived
* alternatives.
*/
// XXX: This rule assumes that `a` is not an expensive or side-effectful expression.
// XXX: The replacement code throws an `IllegalArgumentException` in more cases than the original
// code, but only in situations that are likely unintended.
static final class RandomGeneratorNextLongWithOrigin {
@BeforeTemplate
long before(RandomGenerator random, long a, long b) {
return a + random.nextLong(b);
}

@AfterTemplate
long after(RandomGenerator random, long a, long b) {
return random.nextLong(a, a + b);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,28 @@ ImmutableSet<Double> testRandomGeneratorNextDouble() {
new SecureRandom().nextDouble() * 3.0);
}

double testRandomGeneratorNextDoubleWithOrigin() {
return 1.0 + new Random().nextDouble(2.0);
}

ImmutableSet<Integer> testRandomGeneratorNextInt() {
return ImmutableSet.of(
(int) new Random().nextDouble(1), (int) Math.round(new SplittableRandom().nextDouble(2)));
}

int testRandomGeneratorNextIntWithOrigin() {
return 1 + new Random().nextInt(2);
}

ImmutableSet<Long> testRandomGeneratorNextLong() {
return ImmutableSet.of(
(long) new Random().nextDouble((double) 1L),
Math.round(new SplittableRandom().nextDouble((double) 2L)),
(long) new SecureRandom().nextDouble(3L),
Math.round(ThreadLocalRandom.current().nextDouble(4L)));
}

long testRandomGeneratorNextLongWithOrigin() {
return 1L + new Random().nextLong(2L);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,27 @@ ImmutableSet<Double> testRandomGeneratorNextDouble() {
new SecureRandom().nextDouble(3.0));
}

double testRandomGeneratorNextDoubleWithOrigin() {
return new Random().nextDouble(1.0, 1.0 + 2.0);
}

ImmutableSet<Integer> testRandomGeneratorNextInt() {
return ImmutableSet.of(new Random().nextInt(1), new SplittableRandom().nextInt(2));
}

int testRandomGeneratorNextIntWithOrigin() {
return new Random().nextInt(1, 1 + 2);
}

ImmutableSet<Long> testRandomGeneratorNextLong() {
return ImmutableSet.of(
new Random().nextLong(1L),
new SplittableRandom().nextLong(2L),
new SecureRandom().nextLong(3L),
ThreadLocalRandom.current().nextLong(4L));
}

long testRandomGeneratorNextLongWithOrigin() {
return new Random().nextLong(1L, 1L + 2L);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import com.sun.source.tree.TypeCastTree;
import com.sun.source.tree.UnaryTree;

/** A matcher of expressions that may a non-trivial amount of computation. */
/** A matcher of expressions that may require a non-trivial amount of computation. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this while briefly considering adding @NotMatches(RequiresComputation.class).

public final class RequiresComputation implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;

Expand Down
Loading