Introduce CharSequenceRules Refaster rule collection
#1480
Conversation
| final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase { | ||
| ImmutableSet<Boolean> testCharSequenceIsEmpty() { | ||
| return ImmutableSet.of( | ||
| ((CharSequence) "foo").length() == 0, |
There was a problem hiding this comment.
Maybe there is a better way, but now we know for sure its a CharSequence :D
There was a problem hiding this comment.
new StringBuilder("foo"), str.subSequence(0, 1), ... :)
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
Stephan202
force-pushed
the
rossendrijver/char_sequence
branch
from
bb69636 to
b7754d9
Compare
| */ | ||
| static final class CharSequenceIsEmpty { | ||
| @BeforeTemplate | ||
| boolean before(CharSequence ch) { |
There was a problem hiding this comment.
| boolean before(CharSequence ch) { | |
| boolean before(CharSequence charSequence) { |
As ch generally denotes "char"`. (Unifying the variable naming approach is also on my list, somewhere ;).)
| @@ -32,6 +32,7 @@ private StringRules() {} | |||
| // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` | |||
| // subtypes. This does require a mechanism (perhaps an annotation, or a separate Maven module) to | |||
| // make sure that non-String expressions are rewritten only if client code also targets JDK 15+. | |||
| @SuppressWarnings("CharSequenceIsEmpty" /* This is a more specific template. */) | |||
There was a problem hiding this comment.
The scope of this suppression should be restricted to the @BeforeTemplate.
| // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` | ||
| // subtypes. This does require a mechanism (perhaps an annotation, or a separate Maven module) to | ||
| // make sure that non-String expressions are rewritten only if client code also targets JDK 15+. |
There was a problem hiding this comment.
This comment should be updated.
| final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase { | ||
| ImmutableSet<Boolean> testCharSequenceIsEmpty() { | ||
| return ImmutableSet.of( | ||
| ((CharSequence) "foo").length() == 0, |
There was a problem hiding this comment.
new StringBuilder("foo"), str.subSequence(0, 1), ... :)
| * Prefer {@link CharSequence#isEmpty()} over alternatives that consult the char sequence's | ||
| * length. | ||
| */ | ||
| static final class CharSequenceIsEmpty { |
There was a problem hiding this comment.
We should document this rule's relation to StringIsEmpty.
| ((CharSequence) "foo").length() != 0, | ||
| ((CharSequence) "bar").length() > 0, | ||
| ((CharSequence) "baz").length() >= 1); |
There was a problem hiding this comment.
"qux", "quux", "corge" (we inconsistently sometimes also include "quuz"; something to unify another day ;).)
|
Looks good. No mutations were possible for these changes. |
|
This aims to solve the following issue: #1394.
@timtebeek mentioned that OpenRewrite has some checks that ensure that only the correct recipes are loaded for the Java version it's running on. For us I also don't see any problems for running on our code. The only advantage is that we will be matching more now :).