Do not consider subpatterns on Regex.split/3

Author: José Valim

CHANGELOG.md

@@ -15,6 +15,7 @@
   * [Kernel] Correctly parse unary/binary operators regardless of number of spaces
   * [Kernel] Ensure private functions are not exported
   * [Protocol] Do not expose protocol convention on `assert_impl!/2`
+  * [Regex] Do not consider subpatterns on `Regex.split/3`
   * [Stream] Implement the Inspect protocol for Streams so we do not leak the Stream representation
 
 * Soft deprecations (no warnings emitted)

lib/elixir/lib/regex.ex

@@ -321,7 +321,7 @@ defmodule Regex do
   ## Options
 
     * `:parts` - when specified, splits the string into the given number of
-      parts. If not specified, `:parts` is defaulted to `:infinity`, which will
+      parts. If not specified, `:parts` defaults to `:infinity`, which will
       split the string into the maximum number of parts possible based on the
       given pattern.
 
@@ -348,21 +348,44 @@ defmodule Regex do
 
   def split(regex, string, options \\ [])
 
+  def split(%Regex{}, "", _options), do: [""]
+
   def split(%Regex{re_pattern: compiled}, string, options) when is_binary(string) do
-    parts  = Keyword.get(options, :parts, :infinity)
-    opts   = [return: :binary, parts: zero_to_infinity(parts)]
-    splits = :re.split(string, compiled, opts)
+    case :re.run(string, compiled, [:global, capture: :first]) do
+      {:match, matches} ->
+        do_split(matches, string, 0,
+                 parts_to_index(Keyword.get(options, :parts, :infinity)),
+                 Keyword.get(options, :trim, false))
+      :nomatch ->
+        [string]
+    end
+  end
+
+  defp parts_to_index(:infinity),                      do: 0
+  defp parts_to_index(n) when is_integer(n) and n > 0, do: n
+
+  defp do_split(_, "", _index, _counter, true),       do: []
+  defp do_split(_, string, _index, 1, _trim),         do: [string]
+  defp do_split([], string, _index, _counter, _trim), do: [string]
 
-    if Keyword.get(options, :trim, false) do
-      for split <- splits, split != "", do: split
+  defp do_split([[{0, 0}]|t], string, index, counter, trim) do
+    do_split(t, string, index, counter, trim)
+  end
+
+  defp do_split([[{pos, length}]|t], string, index, counter, trim) do
+    first = pos - index
+    last  = first + length
+
+    head = binary_part(string, 0, first)
+    tail = binary_part(string, last, byte_size(string) - last)
+
+    if trim and head == "" do
+      do_split(t, tail, pos + length, counter, trim)
     else
-      splits
+      [head|do_split(t, tail, pos + length, counter - 1, trim)]
     end
   end
 
-  defp zero_to_infinity(0), do: :infinity
-  defp zero_to_infinity(n), do: n
-
   @doc ~S"""
   Receives a regex, a binary and a replacement, returns a new
   binary where the all matches are replaced by replacement.

lib/elixir/lib/string.ex

@@ -227,55 +227,53 @@ defmodule String do
   """
   @spec split(t, t | [t] | Regex.t) :: [t]
   @spec split(t, t | [t] | Regex.t, Keyword.t) :: [t]
-  def split(binary, pattern, options \\ [])
+  def split(string, pattern, options \\ [])
 
-  def split("", _pattern, _options), do: [""]
-
-  def split(binary, "", options) do
-    index =
-      case Keyword.get(options, :parts, :infinity) do
-        num when is_number(num) and num > 0 -> num
-        _ -> 0
-      end
-    split_codepoints(binary, index - 1, Keyword.get(options, :trim, false))
+  def split(string, "", options) do
+    parts = Keyword.get(options, :parts, :infinity)
+    split_codepoints(string, parts_to_index(parts), Keyword.get(options, :trim, false))
   end
 
-  def split(binary, pattern, options) do
+  def split(string, pattern, options) do
     if Regex.regex?(pattern) do
-      Regex.split(pattern, binary, options)
+      Regex.split(pattern, string, options)
     else
-      splits =
-        case Keyword.get(options, :parts, :infinity) do
-          num when is_number(num) and num > 0 ->
-            split_parts(binary, pattern, num - 1)
-          _ ->
-            :binary.split(binary, pattern, [:global])
-        end
-
-      if Keyword.get(options, :trim, false) do
-        for split <- splits, split != "", do: split
+      parts = Keyword.get(options, :parts, :infinity)
+      trim  = Keyword.get(options, :trim, false)
+      if parts == :infinity and trim == false do
+        :binary.split(string, pattern, [:global])
       else
-        splits
+        split_parts(string, pattern, parts_to_index(parts), trim)
       end
     end
   end
 
-  defp split_codepoints(binary, 0, _trim), do: [binary]
+  defp parts_to_index(:infinity),                      do: 0
+  defp parts_to_index(n) when is_integer(n) and n > 0, do: n
+
+  defp split_codepoints(binary, 1, _trim), do: [binary]
   defp split_codepoints(<<h :: utf8, t :: binary>>, count, trim),
     do: [<<h :: utf8>>|split_codepoints(t, count - 1, trim)]
   defp split_codepoints(<<h, t :: binary>>, count, trim),
     do: [<<h>>|split_codepoints(t, count - 1, trim)]
   defp split_codepoints(<<>>, _, true), do: []
   defp split_codepoints(<<>>, _, false), do: [""]
 
-  defp split_parts("", _pattern, _num),         do: [""]
-  defp split_parts(binary, pattern, num),       do: split_parts(binary, pattern, num, [])
-  defp split_parts("", _pattern, _num, parts),  do: Enum.reverse([""|parts])
-  defp split_parts(binary, _pattern, 0, parts), do: Enum.reverse([binary|parts])
-  defp split_parts(binary, pattern, num, parts) do
-    case :binary.split(binary, pattern) do
-      [head]       -> Enum.reverse([head|parts])
-      [head, rest] -> split_parts(rest, pattern, num - 1, [head|parts])
+  defp split_parts("", _pattern, _num, true),   do: []
+  defp split_parts("", _pattern, _num, _trim),  do: [""]
+  defp split_parts(string, _pattern, 1, _trim), do: [string]
+  defp split_parts(string, pattern, num, trim) do
+    case :binary.split(string, pattern) do
+      [""] when trim ->
+        []
+      [head] ->
+        [head]
+      [head, tail] ->
+        if trim and head == "" do
+          split_parts(tail, pattern, num, trim)
+        else
+          [head|split_parts(tail, pattern, num-1, trim)]
+        end
     end
   end
 

lib/elixir/test/elixir/regex_test.exs

@@ -149,14 +149,16 @@ defmodule RegexTest do
 
   test :split do
     assert Regex.split(~r",", "") == [""]
+    assert Regex.split(~r",", "", trim: true) == [""]
     assert Regex.split(~r" ", "foo bar baz") == ["foo", "bar", "baz"]
-    assert Regex.split(~r" ", "foo bar baz", parts: 0) == ["foo", "bar", "baz"]
     assert Regex.split(~r" ", "foo bar baz", parts: :infinity) == ["foo", "bar", "baz"]
     assert Regex.split(~r" ", "foo bar baz", parts: 10) == ["foo", "bar", "baz"]
     assert Regex.split(~r" ", "foo bar baz", parts: 2) == ["foo", "bar baz"]
     assert Regex.split(~r"\s", "foobar") == ["foobar"]
     assert Regex.split(~r" ", " foo bar baz ") == ["", "foo", "bar", "baz", ""]
     assert Regex.split(~r" ", " foo bar baz ", trim: true) == ["foo", "bar", "baz"]
+    assert Regex.split(~r" ", " foo bar baz ", parts: 2) == ["", "foo bar baz "]
+    assert Regex.split(~r" ", " foo bar baz ", trim: true, parts: 2) == ["foo", "bar baz "]
     assert Regex.split(~r"=", "key=") == ["key", ""]
     assert Regex.split(~r"=", "=value") == ["", "value"]
   end

lib/elixir/test/elixir/string_test.exs

@@ -23,7 +23,7 @@ defmodule StringTest do
   end
 
   test :split do
-    assert String.split("") == [""]
+    assert String.split("") == []
     assert String.split("foo bar") == ["foo", "bar"]
     assert String.split(" foo bar") == ["foo", "bar"]
     assert String.split("foo bar ") == ["foo", "bar"]
@@ -32,24 +32,29 @@ defmodule StringTest do
     assert String.split("foo" <> <<31>> <> "bar") == ["foo", "bar"]
     assert String.split("foo" <> <<194, 133>> <> "bar") == ["foo", "bar"]
 
-    assert String.split("", ",") == [""]
     assert String.split("a,b,c", ",") == ["a", "b", "c"]
     assert String.split("a,b", ".") == ["a,b"]
     assert String.split("1,2 3,4", [" ", ","]) == ["1", "2", "3", "4"]
+
+    assert String.split("", ",") == [""]
     assert String.split(" a b c ", " ") == ["", "a", "b", "c", ""]
+    assert String.split(" a b c ", " ", parts: :infinity) == ["", "a", "b", "c", ""]
+    assert String.split(" a b c ", " ", parts: 1) == [" a b c "]
+    assert String.split(" a b c ", " ", parts: 2) == ["", "a b c "]
 
+    assert String.split("", ",", trim: true) == []
     assert String.split(" a b c ", " ", trim: true) == ["a", "b", "c"]
-    assert String.split(" a b c ", " ", trim: true, parts: 0) == ["a", "b", "c"]
     assert String.split(" a b c ", " ", trim: true, parts: :infinity) == ["a", "b", "c"]
     assert String.split(" a b c ", " ", trim: true, parts: 1) == [" a b c "]
+    assert String.split(" a b c ", " ", trim: true, parts: 2) == ["a",  "b c "]
 
     assert String.split("abé", "") == ["a", "b", "é", ""]
-    assert String.split("abé", "", parts: 0) == ["a", "b", "é", ""]
+    assert String.split("abé", "", parts: :infinity) == ["a", "b", "é", ""]
     assert String.split("abé", "", parts: 1) == ["abé"]
     assert String.split("abé", "", parts: 2) == ["a", "bé"]
     assert String.split("abé", "", parts: 10) == ["a", "b", "é", ""]
     assert String.split("abé", "", trim: true) == ["a", "b", "é"]
-    assert String.split("abé", "", trim: true, parts: 0) == ["a", "b", "é"]
+    assert String.split("abé", "", trim: true, parts: :infinity) == ["a", "b", "é"]
     assert String.split("abé", "", trim: true, parts: 2) == ["a", "bé"]
   end
 

lib/elixir/unicode/unicode.ex

@@ -123,7 +123,7 @@ defmodule String.Unicode do
 
   # Split
 
-  def split(""), do: [""]
+  def split(""), do: []
 
   def split(string) when is_binary(string) do
     :lists.reverse do_split(string, "", [])