Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Readability.FunctionNames #676

Merged
merged 5 commits into from Jul 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 25 additions & 8 deletions lib/credo/check/readability/function_names.ex
Expand Up @@ -2,7 +2,7 @@ defmodule Credo.Check.Readability.FunctionNames do
@moduledoc false

@checkdoc """
Function and macro names are always written in snake_case in Elixir.
Function, macro, and guard names are always written in snake_case in Elixir.

# snake_case

Expand All @@ -19,7 +19,7 @@ defmodule Credo.Check.Readability.FunctionNames do
it easier to follow.
"""
@explanation [check: @checkdoc]
@def_ops [:def, :defp, :defmacro]
@def_ops [:def, :defp, :defmacro, :defmacrop, :defguard, :defguardp]

use Credo.Check, base_priority: :high

Expand All @@ -29,7 +29,19 @@ defmodule Credo.Check.Readability.FunctionNames do
def run(%SourceFile{} = source_file, params \\ []) do
issue_meta = IssueMeta.for(source_file, params)

Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
source_file
|> Credo.Code.prewalk(&traverse(&1, &2, issue_meta), empty_issues())
|> issues_list()
end

defp empty_issues(), do: %{}

defp add_issue(issues, name, arity, issue), do: Map.put_new(issues, {name, arity}, issue)

defp issues_list(issues) do
issues
|> Map.values()
|> Enum.sort_by(& &1.line_no)
end

for op <- @def_ops do
Expand All @@ -49,26 +61,31 @@ defmodule Credo.Check.Readability.FunctionNames do

defp issues_for_definition(body, issues, issue_meta) do
case Enum.at(body, 0) do
{name, meta, nil} ->
issues_for_name(name, meta, issues, issue_meta)
{:when, _when_meta, [{name, meta, args} | _guard]} ->
issues_for_name(name, args, meta, issues, issue_meta)

{name, meta, args} when is_atom(name) ->
issues_for_name(name, args, meta, issues, issue_meta)

_ ->
issues
end
end

defp issues_for_name(name, meta, issues, issue_meta) do
defp issues_for_name(name, args, meta, issues, issue_meta) do
if name |> to_string |> Name.snake_case?() do
issues
else
[issue_for(issue_meta, meta[:line], name) | issues]
issue = issue_for(issue_meta, meta[:line], name)
arity = length(args || [])
add_issue(issues, name, arity, issue)
end
end

defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "Function/macro names should be written in snake_case.",
message: "Function/macro/guard names should be written in snake_case.",
trigger: trigger,
line_no: line_no
)
Expand Down
96 changes: 95 additions & 1 deletion test/credo/check/readability/function_names_test.exs
Expand Up @@ -15,6 +15,10 @@ defmodule Credo.Check.Readability.FunctionNamesTest do
end
defmacro credo_sample_macro do
end
defmacrop credo_sample_macro_p do
end
defguard credo_sample_guard(x) when is_integer(x)
defguardp credo_sample_guard(x) when is_integer(x)
"""
|> to_source_file
|> refute_issues(@described_check)
Expand Down Expand Up @@ -63,10 +67,100 @@ defmodule Credo.Check.Readability.FunctionNamesTest do

test "it should report a violation /4" do
"""
defmacro credo_Sample_Function do
defmacro credo_Sample_Macro do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /5" do
"""
defmacrop credo_Sample_Macro_p do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /6" do
"""
def credoSampleFunction() do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /7" do
"""
def credoSampleFunction(x, y) do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /8" do
"""
defmacro credoSampleMacro() do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /9" do
"""
defmacro credoSampleMacro(x, y) do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /10" do
"""
def credo_SampleFunction when 1 == 1 do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /11" do
"""
def credo_SampleFunction(x, y) when x == y do
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /12" do
"""
defguard credo_SampleGuard(x) when is_integer(x)
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /13" do
"""
defguardp credo_SampleGuard(x) when is_integer(x)
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation /14" do
"""
def credoSampleFunction(0), do: :ok
def credoSampleFunction(1), do: :ok
def credoSampleFunction(2), do: :ok
def credoSampleFunction(_), do: :ok
"""
|> to_source_file
|> assert_issue(@described_check)
end
end