[
https://issues.apache.org/jira/browse/GROOVY-7585?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King resolved GROOVY-7585.
-------------------------------
Resolution: Fixed
Fix Version/s: 2.5.0-beta-1
Latest PR merged. In summary: numerous inconsistencies with various edge cases
have been made consistent.
> ObjectRange strange semantics for mismatched arguments
> ------------------------------------------------------
>
> Key: GROOVY-7585
> URL: https://issues.apache.org/jira/browse/GROOVY-7585
> Project: Groovy
> Issue Type: Bug
> Reporter: Thibault Kruse
> Assignee: Paul King
> Fix For: 2.5.0-beta-1
>
>
> in Object Range, String is a special case considered:
> {code}
> if (from instanceof String || to instanceof String) {
> String start = from.toString();
> String end = to.toString();
> if (start.length() > end.length()) {
> throw new IllegalArgumentException("Incompatible Strings for
> Range: starting String is longer than ending string");
> }
> int length = Math.min(start.length(), end.length());
> int i;
> for (i = 0; i < length; i++) {
> if (start.charAt(i) != end.charAt(i)) break;
> }
> if (i < length - 1) {
> throw new IllegalArgumentException("Incompatible Strings for
> Range: String#next() will not reach the expected value");
> }
> ...
> {code}
> There are two semantic problems with that implementation. On the one hand
> this is plain buggy when the length of 'to' is larger than the length of
> 'from', and two strings are passed:
> {code}
> groovy:000> x = 'aa'..'aaa'
> ===> [aa]
> {code}
> This is because stepping through the strings begins with "ab", which is
> larger than 'aaa' in the comparison of
> DefaultGroovyMethods.numberAwareCompareTo().
> I assume from the rest of the code that the general assumption was that
> aa..aaa == [aa, ab, ac, ... <something> ..., aaa]
> Next, the check would not be meaningful even if the comparison worked as
> intended:
> {code}
> 'aa'..'aaa' is an infinite sequence. Assuming '_' represents
> Character.MAX_VALUE and ^ represents Character.MIN_VALUE:
> aa, ab, ac, ..., a_, a_^, ..., a_a, a_b, ... a__, a__^, ...
> {code}
> so 'aaa' is never reached. So this could only possibly work it a longer 'to'
> argument consisted of from[:-1] + Character.MAX_VALUE <n-times> + [a-z]. I
> don't think that is a relevant case any Groovy user relies on.
> So there are two "bugs" cancelling each other out somehow, preventing
> infinite String creation.
> There are more problems when mixing Types (via the unsafe constructor):
> {code}
> groovy:000> x = new ObjectRange('11', 11, true)
> ===> []
> groovy:000> x = new ObjectRange(11, '11', false)
> ===> [11, 12, 13, 14, 15 ..., 1567]
> {code}
> There is some code almost handling this nicely
> {code}
> if (from.getClass() == to.getClass()) {
> this.from = from;
> this.to = to;
> } else {
> this.from = normaliseStringType(from);
> this.to = normaliseStringType(to);
> }
> {code}
> But this merely turns Characters and single-char Strings into ints. I believe
> it would be nice here if after normalization (inside the else case!), if
> classes do not match, then this should throw an exception. Because when some
> non-Number, non-String Comparable is passed along with a single-char String (
> new MyComparable()..'1') then that string is normalized into it's int value,
> which IMO is not sane behavior. It's not documented anywhere that single-char
> Strings will be turned into ints for a range, this can be a convenience
> transformation when combined with numbers for scripting, but other than that
> it should just fail, instead of leading to unpredictable behavior anytime
> later in a system.
> It seems this has been like it is since the method was written/copied in
> 2005. So I suggest
> - changing the entry condition to AND: ( if (from instanceof String && to
> instanceof String) {)
> - to throw an exception if the length of the strings is different
> - to throw an exception if after normalization 'from' is a Number or String,
> but the class of 'to' is not similar to that of 'from'
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
