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

Add experimental multi-alias expansion warning. #644

Merged
merged 1 commit into from Apr 21, 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
1 change: 1 addition & 0 deletions .credo.exs
Expand Up @@ -141,6 +141,7 @@
#
{Credo.Check.Consistency.MultiAliasImportRequireUse, false},
{Credo.Check.Design.DuplicatedCode, false},
{Credo.Check.Readability.MultiAlias, false},
{Credo.Check.Readability.Specs, false},
{Credo.Check.Refactor.ABCSize, false},
{Credo.Check.Refactor.AppendSingleItem, false},
Expand Down
61 changes: 61 additions & 0 deletions lib/credo/check/readability/multi_alias.ex
@@ -0,0 +1,61 @@
defmodule Credo.Check.Readability.MultiAlias do
@moduledoc false

@checkdoc """
Multi alias expansion makes module uses harder to search for in large code bases.

# preferred

alias Module.Foo
alias Module.Bar

# NOT preferred

alias Module.{Foo, Bar}

Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
it easier to follow.
"""
@explanation [check: @checkdoc]

use Credo.Check, base_priority: :low

alias Credo.Code

@doc false
def run(source_file, params \\ []) do
issue_meta = IssueMeta.for(source_file, params)

Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
end

defp traverse(
{:alias, _, [{{_, _, [{:__aliases__, opts, base_alias}, :{}]}, _, [multi_alias | _]}]} =
ast,
issues,
issue_meta
) do
{:__aliases__, _, [module | []]} = multi_alias
new_issue = issue_for(issue_meta, Keyword.get(opts, :line), base_alias, module)

{ast, [new_issue | issues]}
end

defp traverse(ast, issues, _issue_meta), do: {ast, issues}

defp issue_for(issue_meta, line_no, base_alias, trigger) do
module =
base_alias
|> Module.concat()
|> Module.concat(trigger)
|> inspect()

format_issue(
issue_meta,
message: "Avoid multi aliases to #{module}; consider removing.",
trigger: trigger,
line_no: line_no
)
end
end
44 changes: 44 additions & 0 deletions test/credo/check/readability/multi_alias_test.exs
@@ -0,0 +1,44 @@
defmodule Credo.Check.Readability.MultiAliasTest do
use Credo.TestHelper

@described_check Credo.Check.Readability.MultiAlias

#
# cases NOT raising issues
#

test "it should NOT report violation" do
"""
defmodule Test do
alias App.Module1
alias App.Module2
end
"""
|> to_source_file
|> refute_issues(@described_check)
end

#
# cases raising issues
#

test "it should report a violation" do
"""
defmodule CredoSampleModule do
alias App.Module2.{Module3}
end
"""
|> to_source_file
|> assert_issue(@described_check)
end

test "it should report a violation for multiple expansions" do
"""
defmodule CredoSampleModule do
alias App.{Module1, Module2}
end
"""
|> to_source_file
|> assert_issue(@described_check)
end
end