Handle Windows newlines gracefully

Of particular importance to the `no_trailing_whitespace` rule, since we were splitting lines solely based on `\n` and therefore getting a bunch of `\r`s right at the end of each line (which count as whitespace and provoked failure.) `operator_spaces` was also affected in my use case, so I added some coverage of the issue there as well.
inaka · Aug 31, 2021 · 138cae6 · 138cae6
1 parent 1cce0f9
commit 138cae6
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 6 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1,5 @@
+# Never modify line endings of our newline-covering test cases
+test/examples/fail_no_trailing_whitespace.erl -crlf
+test/examples/fail_operator_spaces.erl -crlf
+test/examples/pass_no_trailing_whitespace_elvis_attr.erl -crlf
+test/examples/pass_operator_spaces_elvis_attr.erl -crlf
diff --git a/src/elvis_project.erl b/src/elvis_project.erl
@@ -267,7 +267,7 @@ is_erlang_mk_not_git_dep(Line, Regex) ->
 
 get_erlang_mk_deps(File) ->
     {Src, _} = elvis_file:src(File),
-    Lines = binary:split(Src, <<"\n">>, [global]),
+    Lines = elvis_utils:split_all_lines(Src),
     Opts = [{capture, all_but_first, binary}],
     IsDepsLine =
       fun(Line) ->

diff --git a/src/elvis_style.erl b/src/elvis_style.erl
@@ -328,7 +328,7 @@ operator_spaces(Config, Target, RuleConfig) ->
     Tokens = ktn_code:attr(tokens, Root),
     PunctuationTokens = lists:filter(fun is_punctuation_token/1, Tokens),
 
-    Lines = binary:split(Src, <<"\n">>, [global]),
+    Lines = elvis_utils:split_all_lines(Src),
     AllNodes = OpNodes ++ PunctuationTokens,
 
     FlatMap = fun(Rule) ->
@@ -577,7 +577,7 @@ max_module_length(Config, Target, RuleConfig) ->
                     andalso (CountWhitespace
                              orelse (not line_is_whitespace(Line)))
         end,
-    Lines = case binary:split(Src, <<"\n">>, [global, trim]) of
+    Lines = case elvis_utils:split_all_lines(Src, [trim]) of
                 Ls when CountComments andalso CountWhitespace -> Ls;
                 Ls -> lists:filter(FilterFun, Ls)
             end,
@@ -603,7 +603,7 @@ max_function_length(Config, Target, RuleConfig) ->
 
     Root = get_root(Config, Target, RuleConfig),
     {Src, _} = elvis_file:src(Target),
-    Lines = binary:split(Src, <<"\n">>, [global, trim]),
+    Lines = elvis_utils:split_all_lines(Src, [trim]),
 
     IsFunction = fun(Node) -> ktn_code:type(Node) == function end,
     Functions0 = elvis_code:find(IsFunction, Root),

diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl
@@ -12,6 +12,8 @@
          %% General
          erlang_halt/1,
          to_str/1,
+         split_all_lines/1,
+         split_all_lines/2,
 
          %% Output
          info/1,
@@ -42,15 +44,15 @@
 -spec check_lines(binary(), fun(), term()) ->
     [elvis_result:item()].
 check_lines(Src, Fun, Args) ->
-    Lines = binary:split(Src, <<"\n">>, [global]),
+    Lines = split_all_lines(Src),
     check_lines(Lines, Fun, Args, [], 1).
 
 %% @doc Checks each line calling fun and providing the previous and next
 %%      lines based on the context tuple {Before, After}.
 -spec check_lines_with_context(binary(), fun(), term(), line_content()) ->
     [elvis_result:item()].
 check_lines_with_context(Src, Fun, Args, Ctx) ->
-    Lines = binary:split(Src, <<"\n">>, [global]),
+    Lines = split_all_lines(Src),
     LinesContext = context(Lines, Ctx),
     check_lines(LinesContext, Fun, Args, [], 1).
 
@@ -121,6 +123,14 @@ to_str(Arg) when is_integer(Arg) ->
 to_str(Arg) when is_list(Arg) ->
     Arg.
 
+-spec split_all_lines(binary()) -> [binary()].
+split_all_lines(Binary) ->
+    split_all_lines(Binary, []).
+
+-spec split_all_lines(binary(), list()) -> [binary()].
+split_all_lines(Binary, Opts) ->
+    binary:split(Binary, [<<"\r\n">>, <<"\n">>], [global | Opts]).
+
 %% @doc Takes a line, a character and a count, returning the indentation level
 %%      invalid if the number of character is not a multiple of count.
 -spec indentation(binary() | string(), char(), integer()) ->

diff --git a/test/examples/fail_no_trailing_whitespace.erl b/test/examples/fail_no_trailing_whitespace.erl
@@ -1,6 +1,7 @@
 -module(fail_no_trailing_whitespace).
 
 -export([one/0, two/0, three/0, four/0, five/0]).
+-export([six/0, seven/0, eight_and_rocking_alright/0]).
 
 one() ->
     %% Following line ends with a tab
@@ -19,3 +20,15 @@ four() -> %% Previous (supposedly blank) line has a space
 
 five() ->
     ok. %% This function should be fine
+
+six() ->
+    %% Following line ends with a Windows newline ("\r\n")
+    ok.
+
+seven() ->
+    %% Previous line ends with a Windows newline ("\r\n")
+    ok.
+
+%% Previous (supposedly blank) line has a Windows newline ("\r\n")
+eight_and_rocking_alright() ->
+    ok.
diff --git a/test/examples/fail_operator_spaces.erl b/test/examples/fail_operator_spaces.erl
@@ -15,6 +15,7 @@
         , function9/0
         , tag_filters/2
         , unicode_characters/0
+        , windows_newlines/0
         , this/0
         , this/1
         ]).
@@ -86,6 +87,11 @@ unicode_characters() ->
   <<"ß"/utf8>> = <<"\\o337">>,
   ok.
 
+windows_newlines() ->
+    <<_/bytes>> = <<"Foo",
+                    "bar">>,
+    ok.
+
 -spec this()
 -> should_not_crash.
 this()

diff --git a/test/examples/pass_no_trailing_whitespace_elvis_attr.erl b/test/examples/pass_no_trailing_whitespace_elvis_attr.erl
@@ -1,6 +1,7 @@
 -module(pass_no_trailing_whitespace_elvis_attr).
 
 -export([one/0, two/0, three/0, four/0, five/0]).
+-export([six/0, seven/0, eight_and_rocking_alright/0]).
 
 -elvis([{elvis_text_style, no_trailing_whitespace, disable}]).
 -elvis([{elvis_text_style, no_tabs, disable}]).
@@ -22,3 +23,15 @@ four() -> %% Previous (supposedly blank) line has a space
 
 five() ->
     ok. %% This function should be fine
+
+six() ->
+    %% Following line ends with a Windows newline ("\r\n")
+    ok.
+
+seven() ->
+    %% Previous line ends with a Windows newline ("\r\n")
+    ok.
+
+%% Previous (supposedly blank) line has a Windows newline ("\r\n")
+eight_and_rocking_alright() ->
+    ok.
diff --git a/test/examples/pass_operator_spaces_elvis_attr.erl b/test/examples/pass_operator_spaces_elvis_attr.erl
@@ -15,6 +15,7 @@
         , function6/0
         , function7/0
         , tag_filters/2
+        , windows_newlines/0
         , unicode_characters/0
         ]).
 
@@ -78,3 +79,8 @@ unicode_characters() ->
   <<"©"/utf8>> = <<"\\u00A9">> ,
   <<"ß"/utf8>> = <<"\\o337">> ,
   ok.
+
+windows_newlines() ->
+    <<_/bytes>> = <<"Foo" ,
+                    "bar">> ,
+    ok.