From 353b46c6df095d937a36f9c61e9e9752de61fdb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20F=C3=B6hring?= Date: Wed, 26 Jun 2019 16:28:41 +0200 Subject: [PATCH] Fix token position functions for Elixir >= 1.9.0 --- .tool-versions | 2 +- lib/credo/code/interpolation_helper.ex | 44 +++++- lib/credo/code/token.ex | 24 +++ test/credo/code/token_test.exs | 201 ++++++++++++++++++++++++- 4 files changed, 267 insertions(+), 4 deletions(-) diff --git a/.tool-versions b/.tool-versions index 2c1c1cacf..19e04d54f 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ erlang 21.2.3 -elixir 1.8.2 +elixir 1.9.0 diff --git a/lib/credo/code/interpolation_helper.ex b/lib/credo/code/interpolation_helper.ex index 340f5047f..1ba022a98 100644 --- a/lib/credo/code/interpolation_helper.ex +++ b/lib/credo/code/interpolation_helper.ex @@ -153,6 +153,8 @@ defmodule Credo.Code.InterpolationHelper do Enum.map(list, &find_interpolations(&1, source)) end + # Elixir < 1.9.0 + # # {{1, 25, 32}, [{:identifier, {1, 27, 31}, :name}]} defp find_interpolations({{_line_no, _col_start2, _}, _list} = token, source) do {line_no, col_start, line_no_end, col_end} = Token.position(token) @@ -186,7 +188,47 @@ defmodule Credo.Code.InterpolationHelper do {line_no, col_start, line_no_end, col_end + padding} end - defp find_interpolations(_value, _source), do: nil + # Elixir >= 1.9.0 + # + # {{1, 25, nil}, {1, 31, nil}, [{:identifier, {1, 27, nil}, :name}]} + defp find_interpolations( + {{_line_no, _col_start, nil}, {_line_no2, _col_start2, nil}, _list} = token, + source + ) do + {line_no, col_start, line_no_end, col_end} = Token.position(token) + + {line_no, col_start, line_no_end, col_end} + # |> IO.inspect() + + col_end = + if line_no_end > line_no && col_end == 1 do + # This means we encountered :eol and jumped in the next line. + # We need to add the closing `}`. + col_end + 1 + else + col_end + end + + line = get_line(source, line_no_end) + + # `col_end - 1` to account for the closing `}` + rest_of_line = get_rest_of_line(line, col_end - 1) + + # IO.inspect(rest_of_line, label: "rest_of_line") + + padding = determine_padding_at_start_of_line(rest_of_line, ~r/^\s*\}/) + + # -1 to remove the accounted-for `}` + padding = max(padding - 1, 0) + + # IO.inspect(padding, label: "padding") + + {line_no, col_start, line_no_end, col_end + padding} + end + + defp find_interpolations(_value, _source) do + nil + end defp determine_padding_at_start_of_line(line, regex \\ ~r/^\s+/) do regex diff --git a/lib/credo/code/token.ex b/lib/credo/code/token.ex index 4e099ae0d..fdcc97828 100644 --- a/lib/credo/code/token.ex +++ b/lib/credo/code/token.ex @@ -74,6 +74,12 @@ defmodule Credo.Code.Token do position_tuple_for_quoted_string(atom_or_charlist, line_no, col_start) end + # Elixir >= 1.9.0 tuple syntax + def position({{line_no, col_start, nil}, {_line_no2, _col_start2, nil}, atom_or_charlist}) do + position_tuple_for_quoted_string(atom_or_charlist, line_no, col_start) + end + + # Elixir < 1.9.0 tuple syntax def position({_, {line_no, col_start, _}, atom_or_charlist}) do position_tuple(atom_or_charlist, line_no, col_start) end @@ -151,6 +157,8 @@ defmodule Credo.Code.Token do Enum.reduce(list, {line_no, col_start, nil}, &reduce_to_col_end/2) end + # Elixir < 1.9.0 + # # {{1, 25, 32}, [{:identifier, {1, 27, 31}, :name}]} defp convert_to_col_end(_, _, {{line_no, col_start, _}, list}) do {line_no_end, col_end, _terminator} = convert_to_col_end(line_no, col_start, list) @@ -161,6 +169,22 @@ defmodule Credo.Code.Token do {line_no_end, col_end, :interpolation} end + # Elixir >= 1.9.0 + # + # {{1, 25, nil}, {1, 31, nil}, [{:identifier, {1, 27, nil}, :name}]} + defp convert_to_col_end( + _, + _, + {{line_no, col_start, nil}, {_line_no2, _col_start2, nil}, list} + ) do + {line_no_end, col_end, _terminator} = convert_to_col_end(line_no, col_start, list) + + # add 1 for } (closing parens of interpolation) + col_end = col_end + 1 + + {line_no_end, col_end, :interpolation} + end + defp convert_to_col_end(_, _, {:eol, {line_no, col_start, _}}) do {line_no, col_start, :eol} end diff --git a/test/credo/code/token_test.exs b/test/credo/code/token_test.exs index 7368a1b48..87034eb79 100644 --- a/test/credo/code/token_test.exs +++ b/test/credo/code/token_test.exs @@ -21,8 +21,205 @@ defmodule Credo.Code.TokenTest do @no_interpolations_source ~S[134 + 145] @no_interpolations_position {1, 7, 1, 10} - # Elixir >= 1.6.0 - if Version.match?(System.version(), ">= 1.6.0-rc") do + # Elixir >= 1.9.0 + if Version.match?(System.version(), ">= 1.9.0") do + @single_interpolations_list_string_source ~S[a = 'MyModule.SubModule.#{name}'] + @single_interpolations_list_string_position {1, 5, 1, 33} + + @tag :token_position + test "should give correct token position" do + source = @no_interpolations_source + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:int, {1, 1, 134}, '134'}, + {:dual_op, {1, 5, nil}, :+}, + {:int, {1, 7, 145}, '145'} + ] + + assert expected == tokens + + position = expected |> List.last() |> Token.position() + + assert @no_interpolations_position == position + end + + @tag :token_position + test "should give correct token position with a single interpolation" do + source = @single_interpolations_bin_string_source + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:identifier, {1, 1, nil}, :a}, + {:match_op, {1, 3, nil}, :=}, + {:bin_string, {1, 5, nil}, + [ + "MyModule.SubModule.", + {{1, 25, nil}, {1, 31, nil}, [{:identifier, {1, 27, nil}, :name}]} + ]} + ] + + assert expected == tokens + + position = expected |> List.last() |> Token.position() + + assert @single_interpolations_bin_string_position == position + end + + @tag :token_position + test "should give correct token position with a single interpolation with list string" do + source = @single_interpolations_list_string_source + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:identifier, {1, 1, nil}, :a}, + {:match_op, {1, 3, nil}, :=}, + {:list_string, {1, 5, nil}, + [ + "MyModule.SubModule.", + {{1, 25, nil}, {1, 31, nil}, [{:identifier, {1, 27, nil}, :name}]} + ]} + ] + + assert expected == tokens + + position = expected |> List.last() |> Token.position() + + assert @single_interpolations_list_string_position == position + end + + @tag :token_position + test "should give correct token position with multiple interpolations" do + source = @multiple_interpolations_source + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:identifier, {1, 1, nil}, :a}, + {:match_op, {1, 3, nil}, :=}, + {:bin_string, {1, 5, nil}, + [ + "MyModule.", + {{1, 15, nil}, {1, 40, nil}, + [ + {:paren_identifier, {1, 17, nil}, :fun}, + {:"(", {1, 20, nil}}, + {:alias, {1, 21, nil}, :Module}, + {:., {1, 27, nil}}, + {:paren_identifier, {1, 28, nil}, :value}, + {:"(", {1, 33, nil}}, + {:")", {1, 34, nil}}, + {:dual_op, {1, 36, nil}, :+}, + {:int, {1, 38, 1}, '1'}, + {:")", {1, 39, nil}} + ]}, + ".SubModule.", + {{1, 52, nil}, {1, 58, nil}, [{:identifier, {1, 54, nil}, :name}]} + ]} + ] + + assert expected == tokens + + position = expected |> List.last() |> Token.position() + + assert @multiple_interpolations_position == position + end + + @tag :to_be_implemented + @tag :token_position + test "should give correct token position with multiple interpolations in heredoc" do + source = @heredoc_interpolations_source + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:identifier, {1, 1, nil}, :def}, + {:paren_identifier, {1, 5, nil}, :fun}, + {:"(", {1, 8, nil}}, + {:")", {1, 9, nil}}, + {:do, {1, 11, nil}}, + {:eol, {1, 13, 1}}, + {:identifier, {2, 3, nil}, :a}, + {:match_op, {2, 5, nil}, :=}, + {:bin_heredoc, {2, 7, nil}, + [ + "MyModule.", + {{3, 10, 3}, + [ + {:paren_identifier, {3, 12, nil}, :fun}, + {:"(", {3, 15, nil}}, + {:alias, {3, 16, nil}, :Module}, + {:., {3, 22, nil}}, + {:paren_identifier, {3, 23, nil}, :value}, + {:"(", {3, 28, nil}}, + {:")", {3, 29, nil}}, + {:dual_op, {3, 31, nil}, :+}, + {:int, {3, 33, 1}, '1'}, + {:")", {3, 34, nil}} + ]}, + ".SubModule.", + {{3, 47, 3}, [{:identifier, {3, 49, nil}, :name}]}, + "\"\n" + ]}, + {:eol, {4, 1, 1}}, + {:end, {5, 1, nil}}, + {:eol, {5, 4, 1}} + ] + + assert expected == tokens + + position = expected |> List.last() |> Token.position() + + assert @heredoc_interpolations_position == position + end + + @tag needs_elixir: "1.7.0" + test "should give correct token position for map" do + source = ~S(%{"some-atom-with-quotes": "#{filename} world"}) + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:%{}, {1, 1, nil}}, + {:"{", {1, 2, nil}}, + {:kw_identifier_unsafe, {1, 3, nil}, ["some-atom-with-quotes"]}, + {:bin_string, {1, 28, nil}, + [{{1, 29, nil}, {1, 39, nil}, [{:identifier, {1, 31, nil}, :filename}]}, " world"]}, + {:"}", {1, 47, nil}} + ] + + assert expected == tokens + + position = expected |> Enum.take(4) |> List.last() |> Token.position() + + assert {1, 28, 1, 47} == position + end + + test "should give correct token position for map /2" do + source = ~S(%{some_atom_with_quotes: "#{filename} world"}) + tokens = Credo.Code.to_tokens(source) + + expected = [ + {:%{}, {1, 1, nil}}, + {:"{", {1, 2, nil}}, + {:kw_identifier, {1, 3, nil}, :some_atom_with_quotes}, + {:bin_string, {1, 26, nil}, + [{{1, 27, nil}, {1, 37, nil}, [{:identifier, {1, 29, nil}, :filename}]}, " world"]}, + {:"}", {1, 45, nil}} + ] + + assert expected == tokens + + position = expected |> Enum.take(4) |> List.last() |> Token.position() + + assert {1, 26, 1, 45} == position + end + end + + # + # + # + + # Elixir >= 1.6.0 && < 1.9.0 + if Version.match?(System.version(), ">= 1.6.0-rc") && + Version.match?(System.version(), "< 1.9.0") do @single_interpolations_list_string_source ~S[a = 'MyModule.SubModule.#{name}'] @single_interpolations_list_string_position {1, 5, 1, 33}