Introduce RandomGeneratorRules Refaster rule collection#1756
Introduce RandomGeneratorRules Refaster rule collection#1756rickie merged 9 commits intoPicnicSupermarket:masterfrom
RandomGeneratorRules Refaster rule collection#1756Conversation
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
rickie
left a comment
rickie
left a comment
There was a problem hiding this comment.
Thanks for the PR @timtebeek. We'll include this one with your other PR soon. There were some holidays that resulted in some slower reviews 😬.
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/RandomRules.java
Outdated
Show resolved
Hide resolved
| return ImmutableSet.of(); | ||
| } | ||
|
|
||
| int testRandomNextInt() { |
There was a problem hiding this comment.
We generally have 1 test method per Refaster rule :).
There is a RandomRules.class missing in the RefasterRulesTest#RULE_COLLECTIONS 😄. We'll see that the test will fail as it expects these tests to be combined in one method :)
| import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; | ||
|
|
||
| final class RandomRulesTest implements RefasterRuleCollectionTestCase { | ||
| @Override |
There was a problem hiding this comment.
I think we can simply omit this one :)
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/RandomRules.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/RandomRules.java
Outdated
Show resolved
Hide resolved
...ne-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/RandomRulesTestInput.java
Outdated
Show resolved
Hide resolved
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
Stephan202
left a comment
Stephan202
left a comment
There was a problem hiding this comment.
I added a commit; PTAL.
Suggested commit message:
Introduce `RandomGeneratorRules` Refaster rule collection (#1756)
| @BeforeTemplate | ||
| int before(Random random, int bound) { | ||
| return (int) (random.nextDouble() * bound); | ||
| } | ||
|
|
||
| @BeforeTemplate | ||
| int before2(Random random, int bound) { | ||
| return (int) Math.round(random.nextDouble() * bound); | ||
| } |
There was a problem hiding this comment.
We can even use Refaser.anyOf :)
| /** Refaster rules related to expressions dealing with {@link Random}. */ | ||
| @OnlineDocumentation | ||
| final class RandomRules { |
There was a problem hiding this comment.
Turns out these rules generalize to any RandomGenerator; I'll rename and update accordingly.
| /** Prefer {@link Random#nextInt(int)} over casting a scaled random double to int. */ | ||
| static final class RandomNextInt { |
There was a problem hiding this comment.
We can introduce a similar rule for nextLong. There it actually fixes a bug, as the conversion to double reduces the effective domain of random longs that may be produced.
|
Looks good. No mutations were possible for these changes. |
|
I see there's also |
|
Looks good. No mutations were possible for these changes. |
|
I realized that |
|
Looks good. No mutations were possible for these changes. |
|
Rebased and added one more commit :) |
Stephan202
changed the title
RandomGeneratorRules Refaster rule collection
|
Looks good. No mutations were possible for these changes. |
And document an idea.
|
Looks good. No mutations were possible for these changes. |
3 participants
Random.nextIntovernextDoubleand truncation/rounding openrewrite/rewrite-static-analysis#621