tpl: Refactor and fix substr logic

Fix miscalculations when start is negative.  Results should now match
PHP substr.

Fixes #7993

diff --git a/tpl/strings/strings.go b/tpl/strings/strings.go
index c87b532d398f9fad04ca05c719ef53e1f468ba6b..9427d8e8a2dc818bc0eb9ac7b601df885bde34a2 100644 (file)
--- a/tpl/strings/strings.go
+++ b/tpl/strings/strings.go
@@ -279,64 +279,73 @@ func (ns *Namespace) Split(a interface{}, delimiter string) ([]string, error) {
 // if length is given and is negative, then that many characters will be omitted from
 // the end of string.
 func (ns *Namespace) Substr(a interface{}, nums ...interface{}) (string, error) {
-       aStr, err := cast.ToStringE(a)
+       s, err := cast.ToStringE(a)
        if err != nil {
                return "", err
        }
 
-       var start, length int
+       asRunes := []rune(s)
+       rlen := len(asRunes)
 
-       asRunes := []rune(aStr)
+       var start, length int
 
        switch len(nums) {
        case 0:
-               return "", errors.New("too less arguments")
+               return "", errors.New("too few arguments")
        case 1:
                if start, err = cast.ToIntE(nums[0]); err != nil {
-                       return "", errors.New("start argument must be integer")
+                       return "", errors.New("start argument must be an integer")
                }
-               length = len(asRunes)
+               length = rlen
        case 2:
                if start, err = cast.ToIntE(nums[0]); err != nil {
-                       return "", errors.New("start argument must be integer")
+                       return "", errors.New("start argument must be an integer")
                }
                if length, err = cast.ToIntE(nums[1]); err != nil {
-                       return "", errors.New("length argument must be integer")
+                       return "", errors.New("length argument must be an integer")
                }
        default:
                return "", errors.New("too many arguments")
        }
 
-       if start < -len(asRunes) {
+       if rlen == 0 {
+               return "", nil
+       }
+
+       if start < 0 {
+               start += rlen
+       }
+
+       // start was originally negative beyond rlen
+       if start < 0 {
                start = 0
        }
-       if start > len(asRunes) {
-               return "", fmt.Errorf("start position out of bounds for %d-byte string", len(aStr))
+
+       if start > rlen-1 {
+               return "", fmt.Errorf("start position out of bounds for %d-byte string", rlen)
        }
 
-       var s, e int
-       if start >= 0 && length >= 0 {
-               s = start
-               e = start + length
-       } else if start < 0 && length >= 0 {
-               s = len(asRunes) + start - length + 1
-               e = len(asRunes) + start + 1
-       } else if start >= 0 && length < 0 {
-               s = start
-               e = len(asRunes) + length
-       } else {
-               s = len(asRunes) + start
-               e = len(asRunes) + length
+       end := rlen
+
+       if length < 0 {
+               end += length
+       } else if length > 0 {
+               end = start + length
+       }
+
+       if start >= end {
+               return "", fmt.Errorf("calculated start position greater than end position: %d > %d", start, end)
        }
 
-       if s > e {
-               return "", fmt.Errorf("calculated start position greater than end position: %d > %d", s, e)
+       if end < 0 {
+               return "", nil
        }
-       if e > len(asRunes) {
-               e = len(asRunes)
+
+       if end > rlen {
+               end = rlen
        }
 
-       return string(asRunes[s:e]), nil
+       return string(asRunes[start:end]), nil
 }
 
 // Title returns a copy of the input s with all Unicode letters that begin words

diff --git a/tpl/strings/strings_test.go b/tpl/strings/strings_test.go
index 6a14acd782c80877de4c2ed2f039b74f3eb68e61..bb90200c01dcf8bdd1008d1d1276345fa15858a5 100644 (file)
--- a/tpl/strings/strings_test.go
+++ b/tpl/strings/strings_test.go
@@ -441,8 +441,11 @@ func TestSubstr(t *testing.T) {
        }{
                {"abc", 1, 2, "bc"},
                {"abc", 0, 1, "a"},
-               {"abcdef", -1, 2, "ef"},
-               {"abcdef", -3, 3, "bcd"},
+               {"abcdef", -1, 2, "f"},
+               {"abcdef", -3, 3, "def"},
+               {"abcdef", -1, nil, "f"},
+               {"abcdef", -2, nil, "ef"},
+               {"abcdef", -3, 1, "d"},
                {"abcdef", 0, -1, "abcde"},
                {"abcdef", 2, -1, "cde"},
                {"abcdef", 4, -4, false},
@@ -480,12 +483,12 @@ func TestSubstr(t *testing.T) {
                }
 
                if b, ok := test.expect.(bool); ok && !b {
-                       c.Assert(err, qt.Not(qt.IsNil))
+                       c.Assert(err, qt.Not(qt.IsNil), qt.Commentf("%v", test))
                        continue
                }
 
                c.Assert(err, qt.IsNil)
-               c.Assert(result, qt.Equals, test.expect)
+               c.Assert(result, qt.Equals, test.expect, qt.Commentf("%v", test))
        }
 
        _, err = ns.Substr("abcdef")