Skip to content

Commit

Permalink
Merge pull request #11 from plataformatec/lm-credo-0.8.x
Browse files Browse the repository at this point in the history
Update Credo to 0.8.x
  • Loading branch information
lucasmazza committed Aug 9, 2017
2 parents f8b59c8 + c32482b commit 2672620
Show file tree
Hide file tree
Showing 12 changed files with 121 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ after_success: "./script/push"
services:
- docker
elixir:
- 1.3.2
- 1.5.0
cache:
directories:
- _build
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM msaraiva/elixir-dev:1.3.4
FROM aeons/elixir-dev:1.5.0
MAINTAINER Plataformatec <opensource@plataformatec.com.br>

WORKDIR /usr/src/engine
Expand Down
4 changes: 2 additions & 2 deletions lib/engine_credo/cli.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ defmodule EngineCredo.CLI do
where the source files are located.
"""

alias EngineCredo.{Config,Runner,Formatter}
alias EngineCredo.{Config, Runner, Formatter}

@lint {~r/Inspect/, false}
def main(argv) do
config = apply(Config, :read, Enum.take(argv, 2))

Expand All @@ -17,6 +16,7 @@ defmodule EngineCredo.CLI do
|> Formatter.print
rescue
error ->
# credo:disable-for-next-line Credo.Check.Warning.IoInspect
IO.puts(:stderr, Exception.format(:error, error))
System.halt(1)
end
Expand Down
114 changes: 67 additions & 47 deletions lib/engine_credo/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,88 +11,108 @@ defmodule EngineCredo.Config do
@engine_config_file Application.get_env(:engine_credo, :engine_config_file)

defstruct source_code_path: nil,
engine_config: nil,
credo_config: nil,
execution: nil,
source_files: [],
invalid_files: []

def read, do: read(%__MODULE__{})
def read, do: read(@source_code_path, @engine_config_file)

def read(source_code_path) when is_binary(source_code_path) do
read(%__MODULE__{source_code_path: source_code_path})
end
def read(source_code_path, engine_config_file) do
engine_config = read_engine_config(engine_config_file)

def read(%__MODULE__{source_code_path: nil} = config) do
read(%{config | source_code_path: @source_code_path})
load(source_code_path, engine_config)
end

def read(%__MODULE__{engine_config: nil} = config) do
read(%{config | engine_config: read_engine_config(@engine_config_file)})
end
defp load(path, engine_config) do
execution = build_execution(path, engine_config)

def read(%__MODULE__{source_code_path: path, engine_config: engine_config} = config) do
credo_config =
path
|> Credo.ConfigFile.read_or_default(nil, true) # true required for safe loading of `.credo.exs`
|> cast_to_config
|> include_files_from(engine_config, path)
{source_files, invalid_files} = find_source_files(execution)
execution =
execution
|> load_inline_configuration(source_files)
|> load_comment_configuration(source_files)

%__MODULE__{
source_code_path: path,
source_files: source_files,
invalid_files: invalid_files,
execution: execution
}
end

{source_files, invalid_files} = find_source_files(credo_config)
inline_configuration = find_inline_configuration(source_files)
defp read_engine_config(engine_config_file) do
engine_config_file
|> File.read!
|> Poison.decode!
end

%{config | credo_config: %{credo_config | lint_attribute_map: inline_configuration}, source_files: source_files, invalid_files: invalid_files}
defp build_execution(path, engine_config) do
path
|> read_config_file
|> create_struct
|> include_files_from(path, engine_config)
|> boostrap
end

def read(source_code_path, engine_config_file) do
engine_config = read_engine_config(engine_config_file)
read(%__MODULE__{source_code_path: source_code_path, engine_config: engine_config})
defp read_config_file(path) do
Credo.ConfigFile.read_or_default(path, nil, true) # true required for safe loading of `.credo.exs`.
end

def cast_to_config(%Credo.ConfigFile{} = config_file) do
%Credo.Config{
defp create_struct(%Credo.ConfigFile{} = config_file) do
%Credo.Execution{
files: config_file.files,
checks: config_file.checks,
requires: config_file.requires,
strict: config_file.strict,
color: false,
check_for_updates: false
color: false
}
end

defp read_engine_config(config_file) do
config_file
|> File.read!
|> Poison.Parser.parse!
end

defp include_files_from(config, engine_config, source_path) do
files_from_engine_config =
defp include_files_from(execution, path, engine_config) do
include_paths =
engine_config["include_paths"]
|> List.wrap
|> Enum.map(&Path.join(source_path, &1))
|> Enum.map(&Path.join(path, &1))

update_in config.files[:included], fn files ->
update_in execution.files[:included], fn files ->
files
|> Enum.concat(files_from_engine_config)
|> Enum.concat(include_paths)
|> Enum.uniq
end
end

# Filter out malformed Elixir files and also valid Elixir files with an unknown
# file extension (`.ex` or `.exs`).
defp find_source_files(config) do
config
defp find_source_files(execution) do
execution
|> Credo.Sources.find
|> Enum.filter(&String.ends_with?(&1.filename, [".ex", ".exs"]))
|> Enum.partition(&(&1.valid?))
|> Enum.split_with(&(&1.valid?))
end

# TODO: Remove this once stop supporting the inline attribute configuration.
defp load_inline_configuration(execution, source_files) do
lint_configuration =
source_files
|> Credo.Check.FindLintAttributes.run(execution, [])
|> Enum.into(%{})

%Credo.Execution{execution | lint_attribute_map: lint_configuration}
end

defp load_comment_configuration(execution, source_files) do
comment_configuration =
source_files
|> Credo.Check.ConfigCommentFinder.run(execution, [])
|> Enum.into(%{})

%Credo.Execution{execution | config_comment_map: comment_configuration}
end

defp find_inline_configuration(source_files) do
source_files
|> Credo.Check.FindLintAttributes.run([])
|> Enum.reduce(%{}, fn(source_file, inline_configuration) ->
Map.put(inline_configuration, source_file.filename, source_file.lint_attributes)
end)
defp boostrap(execution) do
execution
|> Credo.Execution.Issues.start_server
# TODO: Remove this once stop supporting the inline attribute configuration.
|> Credo.CLI.Output.UI.use_colors
end
end
6 changes: 3 additions & 3 deletions lib/engine_credo/issue_categories.ex
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
defmodule EngineCredo.IssueCategories do
@moduledoc """
Mapping of categories from Credo to Code Climate.
Additional configuration is provided by the Code Climate engine config, which
is a JSON file (`/config.json`) residing at the container root by default.
"""
@after_compile __MODULE__

Expand Down Expand Up @@ -56,6 +53,7 @@ defmodule EngineCredo.IssueCategories do
Credo.Check.Refactor.DoubleBooleanNegation => "Style",
Credo.Check.Refactor.VariableRebinding => "Clarity",
Credo.Check.Refactor.AppendSingleItem => "Clarity",
Credo.Check.Refactor.LongQuoteBlocks => "Clarity",

Credo.Check.Warning.BoolOperationOnSameValues => "Bug Risk",
Credo.Check.Warning.IExPry => "Bug Risk",
Expand All @@ -76,12 +74,14 @@ defmodule EngineCredo.IssueCategories do
Credo.Check.Warning.UnusedTupleOperation => "Bug Risk",
Credo.Check.Warning.LazyLogging => "Style",
Credo.Check.Warning.MapGetUnsafePass => "Bug Risk",
Credo.Check.Warning.RaiseInsideRescue => "Clarity",

# Deprecated checks
Credo.Check.Refactor.CaseTrivialMatches => "Clarity"
}

@helper_modules [
Credo.Check.ConfigCommentFinder,
Credo.Check.Design.TagHelper,
Credo.Check.FindLintAttributes,
Credo.Check.Warning.UnusedFunctionReturnHelper
Expand Down
18 changes: 10 additions & 8 deletions lib/engine_credo/runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ defmodule EngineCredo.Runner do
`EngineCredo.Issue` for proper output formatting.
"""

alias EngineCredo.{Issue,Config}
alias EngineCredo.{Issue, Config}
alias Credo.CLI.Filter
alias Credo.Execution

def check(%Config{credo_config: config, source_files: files, source_code_path: path_prefix}) do
{checked_source_files, _} = Credo.Check.Runner.run(files, config)
def check(%Config{execution: execution, source_files: files, source_code_path: path_prefix}) do
:ok = Credo.Check.Runner.run(files, execution)

extract_issues(checked_source_files, path_prefix, config)
issues = Execution.get_issues(execution)

extract_issues(issues, path_prefix, execution)
end

defp extract_issues(source_files, path_prefix, config) do
source_files
|> Enum.flat_map(&(&1.issues))
|> Filter.valid_issues(config)
defp extract_issues(issues, path_prefix, execution) do
issues
|> Filter.valid_issues(execution)
|> Enum.map(&Issue.convert(&1, path_prefix))
end
end
9 changes: 5 additions & 4 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule EngineCredo.Mixfile do
def project do
[app: :engine_credo,
version: "0.1.0",
elixir: "~> 1.3",
elixir: "~> 1.5",
escript: escript(),
deps: deps()]
end
Expand All @@ -13,7 +13,7 @@ defmodule EngineCredo.Mixfile do
#
# Type "mix help compile.app" for more information
def application do
[applications: [:logger, :credo, :poison]]
[extra_applications: [:logger]]
end

# Dependencies can be Hex packages:
Expand All @@ -27,8 +27,9 @@ defmodule EngineCredo.Mixfile do
# Type "mix help deps" for more examples and options
defp deps do
[
{:credo, "~> 0.4"},
{:poison, "~> 2.2"}
{:credo, "~> 0.8"},
{:poison, "~> 2.2"},
{:briefly, "~> 0.3", only: :test}
]
end

Expand Down
5 changes: 3 additions & 2 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
%{"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []},
"credo": {:hex, :credo, "0.7.3", "9827ab04002186af1aec014a811839a06f72aaae6cd5eed3919b248c8767dbf3", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, optional: false]}]},
%{"briefly": {:hex, :briefly, "0.3.0", "16e6b76d2070ebc9cbd025fa85cf5dbaf52368c4bd896fb482b5a6b95a540c2f", [:mix], []},
"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []},
"credo": {:hex, :credo, "0.8.5", "3002d41c8a9452bd69cb1c217a6e17bf6fd9d8549932fe9f99a51646587c2a65", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, optional: false]}]},
"poison": {:hex, :poison, "2.2.0", "4763b69a8a77bd77d26f477d196428b741261a761257ff1cf92753a0d4d24a63", [:mix], []}}
27 changes: 18 additions & 9 deletions test/engine_credo/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule EngineCredo.ConfigTest do
alias EngineCredo.Config

test "configures the credo engine for a given path" do
%{credo_config: config} = Config.read
%{execution: execution} = Config.read

expected_included_paths = [
"test/fixtures/project_root/lib/**/*.{ex,exs}",
Expand All @@ -13,20 +13,20 @@ defmodule EngineCredo.ConfigTest do
"test/fixtures/project_root/apps"
]

assert expected_included_paths == config.files.included
assert Enum.member?(config.checks, {Credo.Check.Warning.IExPry})
assert expected_included_paths == execution.files.included
assert Enum.member?(execution.checks, {Credo.Check.Warning.IExPry})
end

test "merges paths from the engine config" do
engine_config = %{
config_path = write_config %{
"include_paths" => [
"src/",
"extra/",
"other/no_issues.exs"
]
}

%{credo_config: config} = Config.read(%Config{engine_config: engine_config})
%{execution: execution} = Config.read(source_code_path(), config_path)

expected_included_paths = [
"test/fixtures/project_root/lib/**/*.{ex,exs}",
Expand All @@ -37,15 +37,16 @@ defmodule EngineCredo.ConfigTest do
"test/fixtures/project_root/other/no_issues.exs"
]

assert expected_included_paths == config.files.included
assert expected_included_paths == execution.files.included
end

test "finds elixir files to check" do
%{source_files: files} = Config.read

found_files = [
"test/fixtures/project_root/lib/design_issues.exs",
"test/fixtures/project_root/lib/ignore_via_attribute.exs"
"test/fixtures/project_root/lib/ignore_via_attribute.exs",
"test/fixtures/project_root/lib/ignore_via_comment.exs"
]

assert found_files == Enum.map(files, &(&1.filename))
Expand All @@ -63,16 +64,24 @@ defmodule EngineCredo.ConfigTest do
end

test "filters out valid files with unknown extensions" do
engine_config = %{
config_path = write_config %{
"include_paths" => ["valid_elixir_invalid_extension.txt"]
}

%{source_files: files} = Config.read(%Config{engine_config: engine_config})
%{source_files: files} = Config.read(source_code_path(), config_path)

contains_invalid_file = Enum.any?(files,
&(&1.filename == "test/fixtures/project_root/valid_elixir_invalid_extension.txt")
)

refute contains_invalid_file
end

defp write_config(config) do
{:ok, path} = Briefly.create
File.write!(path, Poison.encode!(config))
path
end

defp source_code_path, do: Application.get_env(:engine_credo, :source_code_path)
end
4 changes: 2 additions & 2 deletions test/engine_credo/formatter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ defmodule EngineCredo.FormatterTest do

import ExUnit.CaptureIO

alias EngineCredo.{Config,Formatter,Runner}
alias EngineCredo.{Config, Formatter, Runner}

test "prints issues as JSON separated by \\0 and \\n" do
output = capture_io(fn ->
Expand All @@ -29,6 +29,6 @@ defmodule EngineCredo.FormatterTest do
second_error = "Invalid file detected test/fixtures/project_root/lib/non_utf8.exs\n"
expected_output = first_error <> second_error

assert expected_output == output
assert String.contains?(output, expected_output)
end
end
2 changes: 1 addition & 1 deletion test/engine_credo/runner_test.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule EngineCredo.RunnerTest do
use ExUnit.Case

alias EngineCredo.{Config,Runner}
alias EngineCredo.{Config, Runner}

test "finds credo issues for the given source files" do
checked_files =
Expand Down
Loading

0 comments on commit 2672620

Please sign in to comment.