rrrene/credo

A static code analysis tool for the Elixir language with a focus on code consistency and teaching.

Credo

Credo is a static code analysis tool for the Elixir language with a focus on teaching and code consistency.

It can show you refactoring opportunities in your code, complex code fragments, warn you about common mistakes, show inconsistencies in your naming scheme and - if needed - help you enforce a desired coding style.

Installation and Usage

The easiest way to add Credo to your project is by using Mix.

Add :credo as a dependency to your project's mix.exs:

defp deps do
  [
    {:credo, "~> 1.7", only: [:dev, :test], runtime: false}
  ]
end

And run:

$ mix deps.get

$ mix credo

Documentation

Documentation is available on Hexdocs

Integrations

IDE/Editor

Some IDEs and editors are able to run Credo in the background and mark issues inline.

• IntelliJ Elixir - Elixir plugin for JetBrains IDEs (IntelliJ IDEA, Rubymine, PHPStorm, PyCharm, etc)
• linter-elixir-credo - Package for Atom editor (by @smeevil)
• Elixir Linter (Credo) - VSCode extension (by @pantajoe)
• flycheck - Emacs syntax checking extension
• kakoune - Config for linting support in Kakoune editor
• Neovim via null-ls - diagnostics builtin

Automated Code Review

• Codacy - checks your code from style to security, duplication, complexity, and also integrates with coverage.
• SourceLevel - tracks how your code changes over time and have this information accessible to your whole team.
• Stickler CI - checks your code for style and best practices across your entire stack.

Contributing

1. Fork it!
2. Create your feature branch (git checkout -b my-new-feature)
3. Commit your changes (git commit -am 'Add some feature')
4. Push to the branch (git push origin my-new-feature)
5. Create new Pull Request

Author

René Föhring (@rrrene)

License

Credo is released under the MIT License. See the LICENSE file for further details.

credo's Issues

Crash when Credo.Check.Design.AliasUsage.find_aliases/2 calls to_string

Versions

Elixir: 1.2.3
Credo: 0.3.8

Error

Checking 51 source files ...
** (EXIT from #PID<0.47.0>) an exception was raised:
    ** (Protocol.UndefinedError) protocol String.Chars not implemented for {:__MODULE__, [line: 7], nil}
        (elixir) lib/string/chars.ex:3: String.Chars.impl_for!/1
        (elixir) lib/string/chars.ex:17: String.Chars.to_string/1
        (elixir) lib/enum.ex:1473: Enum."-reduce/3-lists^foldl/2-0-"/3
        (elixir) lib/enum.ex:1058: Enum.join/2
        lib/credo/check/design/alias_usage.ex:119: Credo.Check.Design.AliasUsage.find_aliases/2
        (elixir) lib/macro.ex:188: anonymous fn/4 in Macro.do_traverse/4
        (elixir) lib/enum.ex:1151: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
        (elixir) lib/enum.ex:1151: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3

0 mods/funs, found nothing

I'm new to Elixir (and relatively new to programming in general) so I might be missing something obvious, but when I run mix credo I get a result of Checking 4 source files and then 0 mods/funs, found nothing.

Is there something else I need to be doing to get this to work?

UnicodeConversionError - Error message doesn't tell where the error is occurring

I'm getting the following

sashaafm@LXLE:~/Documents/nomad$ mix credo --explain
Checking 8 source files ...
** (UnicodeConversionError) invalid encoding starting at <<211, 32, 69, 83, 84, 193, 32, 80, 65, 82, 65, 32, 77, 89, 83, 81, 76, 32, 35, 35, 35, 35, 10, 32, 32, 100, 101, 102, 112, 32, 98, 115, 32, 100, 111, 10, 32, 32, 32, 32, 34, 34, 34, 10, 32, 32, 32, 32, 32, 32, ...>>
(elixir) lib/string.ex:1634: String.to_char_list/1
lib/credo/code.ex:62: Credo.Code.to_tokens/1
lib/credo/check/consistency/space_around_operators/without_space.ex:16: Credo.Check.Consistency.SpaceAroundOperators.WithoutSpace.property_values_for/2
lib/credo/check/consistency/helper.ex:152: anonymous fn/4 in Credo.Check.Consistency.Helper.collect_property_values/3
(elixir) lib/enum.ex:1473: Enum."-reduce/3-lists^foldl/2-0-"/3
lib/credo/check/consistency/helper.ex:144: Credo.Check.Consistency.Helper.property_list_for/3
lib/credo/check/consistency/helper.ex:138: Credo.Check.Consistency.Helper.create_property_tuples/3
(elixir) lib/enum.ex:1088: Enum."-map/2-lists^map/1-0-"/2

I don't understand where the error is coming from? It wasn't warning about anything in the last commit and I didn't do many changes. I can try to give more information if needed.

Sublime text plugin

This is a great tool!

Is there anything in the works around getting a sublime / atom linter setup?

[noob elixir question] mix credo fails with "undefined function Bunt.puts"

I am learning Elixir since a couple of days so I don't have much background to debug this. My goal was to use credo while I learn the language after I heard good feedback on it 😉

I have added credo to my mix.exs and then I ran the following commands:

~/Code/elixir/hello_elixir  15:18:03
$ mix deps.get                                                                                                                                                          ruby-2.2.3
Running dependency resolution
All dependencies up to date

~/Code/elixir/hello_elixir  15:18:11
$ mix deps                                                                                                                                                              ruby-2.2.3
* bunt 0.1.5 (Hex package) (mix)
  locked at 0.1.5 (bunt)
  ok
* credo 0.3.10 (Hex package) (mix)
  locked at 0.3.10 (credo)
  ok

~/Code/elixir/hello_elixir  15:18:15
$ mix credo                                                                                                                                                             ruby-2.2.3
** (UndefinedFunctionError) undefined function Bunt.puts/1 (module Bunt is not available)
    Bunt.puts("Checking 1 source file ...")
    lib/credo/cli/command/suggest.ex:19: Credo.CLI.Command.Suggest.run/2
    lib/credo/cli.ex:53: Credo.CLI.main/1
    (mix) lib/mix/cli.ex:58: Mix.CLI.run_task/2
    (elixir) lib/code.ex:363: Code.require_file/2

Incorrect behaviour of Unusued*Operations in a comprehension

Here's a minimal example:

for x <- [1..3, 5..10], sum=Enum.sum(x), do: sum

This reports [W] ↗ There should be no unused return values for Enum.sum()

I've actually seen this result with a string function, but it looks like all UnusedOperations are affected.

Ignore white-space after a trailing comma in a list

I found to be good practice to leave lists to contain a trailing comma,
@options [foo: 1, bar: 2, ]
so it's implied that more elements can be added to this list,

but when running mix credo --strict I will get a warning:

[C] ↗ There is no whitespace around parentheses/brackets most of the time, but here there is.

The warning is gone when I remove the trailing white space
would it be possible to ignore the trailing space when it's presided by a comma, and just judge the list by whether it starts with a white-space or not.

thank you for creating such a beautiful tool

Config - Excluding directory results in error

adding any directory to the excluded array, eg. lib/mix/tasks/, in .credo.exs, causes this error:

(elixir) lib/regex.ex:158: Regex.match?("./lib/mix/tasks", "lib/test_api.ex")                    
(elixir) lib/enum.ex:2279: Enum.do_any?/2                                                          
(elixir) lib/enum.ex:1476: anonymous fn/3 in Enum.reject/2                                         
(elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3                                     
(elixir) lib/enum.ex:1476: Enum.reject/2                                                           
lib/credo/sources.ex:7: Credo.Sources.find/1                                                       
lib/credo/cli/command/list.ex:26: anonymous fn/1 in Credo.CLI.Command.List.load_and_validate_source_files/1                                                                                           
(stdlib) timer.erl:166: :timer.tc/1 

Great tool! Thanks!

Multi alias use is not detected

Elixir 1.2 added support for multi use aliases. Credo should pick this up and not warn on missing aliases.

Here is an example that should be allowed and not have any warnings.

defmodule Sample.App do
  @moduledoc false

  alias Sample.App.{One, Two}

  def foo do
    {One.one, Two.two}
  end
end

defmodule Sample.App.One do
  @moduledoc false
  def one, do: "One"
end

defmodule Sample.App.Two do
  @moduledoc false
  def two, do: "Two"
end

However, this will give the following messages

  Software Design
┃
┃ [D] ↓ Nested modules could be aliased at the top of the invoking module.
┃       lib/sample.ex:7:22 (Sample.foo)
┃ [D] ↓ Nested modules could be aliased at the top of the invoking module.
┃       lib/sample.ex:7:6 (Sample.foo)

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.07 seconds (0.00s to load, 0.07s running checks)
6 mods/funs, found 2 software design suggestions.

Question: Is there a way to force 2 spaces?

so 3 or 4 spaces will fail?

Pipe chain should start with a raw value.

The following line of code triggers a "refactoring opportunity":

do_something() |> then_something_else() |> and_last_step()

# [F] → Pipe chain should start with a raw value.

It feels like this a valid use case for a pipe, and that credo should say anything about this code. Is that right?

Apologies if this was discussed previously, I searched on the repo but couldn't find anything.

Variable `sigil_r` has same name as the `Kernel.sigil_r` macro.

using a case with regex matching results in a warning.

Example :
case email do
~r/@/ -> something
_ -> other
end

results in :
Variable sigil_r has same name as the Kernel.sigil_r macro.

Exception raised for TODO check

First of all, this is very slick. Thanks for doing this. I noticed an error is raised with some of my todos.

Fails:
Repo.preload(:comments)# TODO blah blah

Works:
Repo.preload(:comments) # TODO blah blah

I am pretty sure it is just the lack of space after the # that is causing the issue

Credo: 0.1.4

Exception:

Checking 44 source files ...
** (EXIT from #PID<0.46.0>) an exception was raised:
    ** (Regex.CompileError) unmatched parentheses at position 2
        (elixir) lib/regex.ex:141: Regex.compile!/2
        lib/credo/source_file.ex:35: Credo.SourceFile.column/3
        lib/credo/check/design/tag_todo.ex:24: Credo.Check.Design.TagTODO.format_issue/2
        (elixir) lib/enum.ex:1043: anonymous fn/3 in Enum.map/2
        (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3
        (elixir) lib/enum.ex:1043: Enum.map/2
        (elixir) lib/enum.ex:836: anonymous fn/3 in Enum.flat_map/2
        (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3

22:14:05.238 [error] Task #PID<0.184.0> started from #PID<0.46.0> terminating
** (Regex.CompileError) unmatched parentheses at position 2
    (elixir) lib/regex.ex:141: Regex.compile!/2
    lib/credo/source_file.ex:35: Credo.SourceFile.column/3
    lib/credo/check/design/tag_todo.ex:24: Credo.Check.Design.TagTODO.format_issue/2
    (elixir) lib/enum.ex:1043: anonymous fn/3 in Enum.map/2
    (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3
    (elixir) lib/enum.ex:1043: Enum.map/2
    (elixir) lib/enum.ex:836: anonymous fn/3 in Enum.flat_map/2
    (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3
Function: #Function<6.103509261/0 in Credo.Check.Runner.run/2>
    Args: []

22:14:05.245 [error] GenServer Credo.Supervisor terminating
** (Regex.CompileError) unmatched parentheses at position 2
    (elixir) lib/regex.ex:141: Regex.compile!/2
    lib/credo/source_file.ex:35: Credo.SourceFile.column/3
    lib/credo/check/design/tag_todo.ex:24: Credo.Check.Design.TagTODO.format_issue/2
    (elixir) lib/enum.ex:1043: anonymous fn/3 in Enum.map/2
    (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3
    (elixir) lib/enum.ex:1043: Enum.map/2
    (elixir) lib/enum.ex:836: anonymous fn/3 in Enum.flat_map/2
    (elixir) lib/enum.ex:1387: Enum."-reduce/3-lists^foldl/2-0-"/3
Last message: {:EXIT, #PID<0.46.0>, {%Regex.CompileError{message: "unmatched parentheses at position 2"}, [{Regex, :compile!, 2, [file: 'lib/regex.ex', line: 141]}, {Credo.SourceFile, :column, 3, [file: 'lib/credo/source_file.ex', line: 35]}, {Credo.Check.Design.TagTODO, :format_issue, 2, [file: 'lib/credo/check/design/tag_todo.ex', line: 24]}, {Enum, :"-map/2-fun-0-", 3, [file: 'lib/enum.ex', line: 1043]}, {Enum, :"-reduce/3-lists^foldl/2-0-", 3, [file: 'lib/enum.ex', line: 1387]}, {Enum, :map, 2, [file: 'lib/enum.ex', line: 1043]}, {Enum, :"-flat_map/2-fun-1-", 3, [file: 'lib/enum.ex', line: 836]}, {Enum, :"-reduce/3-lists^foldl/2-0-", 3, [file: 'lib/enum.ex', line: 1387]}]}}
State: {:state, {:local, Credo.Supervisor}, :one_for_one, [{:child, #PID<0.166.0>, Credo.Service.SourceFileCodeOnly, {Credo.Service.SourceFileCodeOnly, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileCodeOnly]}, {:child, #PID<0.165.0>, Credo.Service.SourceFileWithoutStringAndSigils, {Credo.Service.SourceFileWithoutStringAndSigils, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileWithoutStringAndSigils]}], :undefined, 3, 5, [], Supervisor.Default, {:ok, {{:one_for_one, 3, 5}, [{Credo.Service.SourceFileWithoutStringAndSigils, {Credo.Service.SourceFileWithoutStringAndSigils, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileWithoutStringAndSigils]}, {Credo.Service.SourceFileCodeOnly, {Credo.Service.SourceFileCodeOnly, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileCodeOnly]}]}}}```

mix do credo, <anything> does only run credo

When using mix do only the items up to and including credo are run.

eg:

$ mix do credo, dialyzer
Checking 1 source file ...

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.1 seconds (0.00s to load, 0.1s running checks)
7 mods/funs, found no issues.

Showing priority issues: ↑ ↗ →  (use `--strict` to show all issues, `--help` for options).

while

$ mix do dialyzer, credo
Starting Dialyzer
dialyzer --no_check_plt --plt /home/nobbz/.dialyxir_core_18_1.2.0.plt -Wunmatched_returns -Werror_handling -Wrace_conditions -Wunderspecs /home/nobbz/projects/elixir/foo/_build/dev/lib/foo/ebin
  Proceeding with analysis... done in 0m1.25s
done (passed successfully)

Checking 1 source file ...

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.1 seconds (0.00s to load, 0.1s running checks)
7 mods/funs, found no issues.

Credo can support find functions that is directly call in module ?

Fully qualified function call is ok in hot upgrade,
Add a new feature that could scan what directly functions call exists in a module

@lint rules ignores macros

I am using Credo with Phoenix application and I have two models that have the same fields and validations, but different semantics. For example:

  @lint {Credo.Check.Design.DuplicatedCode, false} #this doesn't work
  embedded_schema do
    field :field1, :integer
    field :field2
    field :field3
  end

  @required_fields [:field1, :field2, :field3]
  @optional_fields []

  @lint {Credo.Check.Design.DuplicatedCode, false} #this works fine
  def changeset(model, params \\ :empty) do
    model
    |> cast(params, @required_fields, @optional_fields)
    |> validate_number(:field1, greater_than_or_equal_to: 0)
  end

In the near future the two models will probably have different fields, so I want to keep them separate. The @lint attribute works fine with a function, but it doesn't work for macros.
It would be great if there was possibility to disable duplicated code check for macros the same way as for functions.

"readability" and "refactoring" tags are the same when using `--oneline`

When I run mix credo --one-line I get both readability checks and refactoring checks tagged with [R].

I'm currently writing a script to extract credo's output to format comments for Gitlab merge requests.
It would be great if I was able to differentiate between these categories while using the --one-line option. This would make my scripts much simpler.

I would suggest changing the refactoring tags to [F]. If you're okay with the change, I would gladly send a PR.

Also, thanks for the great project 👍

Parse from stdin

Hi,

I would like to suggest to allow Credo to parse source from stdin.
The reasoning behind this is that it would make it easier for editors to make use of Credo.
For example with Spacemacs and Flycheck.

I know its possible to use it with Flycheck at the moment, but it requires the file to be saved. If you use continues flychecking it will thus also continuous safe the file either in the original or make a temp file to pass along to Credo to let it do its job.

If we can pass the buffer's contents from Spacemacs directly to Credo via stdin, we can bypass the saving/creation of temp files.

I've already create a pull request supporting this behaviour where you can use it as follows.

Just passing in source via stdin :

$ echo 'IO.puts( "hello world");' |  mix credo --format=flycheck --stdin
# stdin:1: C: There is no whitespace around parentheses/brackets most of the time, but here there is.

Passing in source via stdin and using a filepath to map:

$ echo 'IO.puts( "hello world");' |  mix credo --format=flycheck --stdin /path/representing/the_current/source.ex
# /path/representing/the_current/source.ex:1: C: There is no whitespace around parentheses/brackets most of the time, but here there is.

Do note with the last option passed as filename is a stub that is just used to prefix the error so certain editors can annotate the original file.

You can find the pull request here : #50

False positive when pattern matching functions parameters with sigils

The following code gives an incorrect warning:

defmodule Foobarbaz do
  def foo(~w(a b c)) do
    IO.puts("a b c")
  end
  def foo(_) do
    IO.puts("not a b c")
  end
end

warning:

┃ [W] ↗ Parameter `sigil_w` has same name as the `Kernel.sigil_w` macro.
┃       lib/foobarbaz.ex:2 (Foobarbaz.foo)

Thank you for your help!

Ambiguous variables should probably check arity

Josè has changed the generators in phoenix to use struct as parameter instead of model:

def changeset(struct, params \\ %{}) do

Credo doesn't like that:

[W] ↗ Parameter `struct` has same name as the `Kernel.struct` function.

So i asked Jose about it and he told me that Kernel only implements struct/2, so it is ok to use struct/0 here.

I think it would be right that NameRedeclarationByAssignment is actually checking the exact method. So not just struct but struct/2

changing only some rules

Hello
how can I change some rules without copy/paste all .credo.exs file ?
For example I would like disable Credo.Check.Readability.ModuleDoc and change Credo.Check.Readability.MaxLineLength, priority: :low, max_length: 80 to 100. but I would like to keep all other rules.

[.credo.exs] Support excluding `_build` in umbrella e.g. "apps/myapp/_build"

Tried setting excluded in .credo.exs with **/_build/ but it didn't work. _build folders in apps/ folders are still being evaluated.

%{
  configs: [
    %{
      name: "default",
      files: %{
        included: ["lib/", "src/", "web/", "apps/"],
        excluded: ["**/_build/"]
      },
...

Problem: suggested alias is not possible

In exzmq, we have the following modules: Exzmq.Socket and Exzmq.Tcp.Socket. Now the following code still recommends to alias Tcp.Socket, which would conflict with the first Exzmq.Socket alias:

defmodule Test do
  alias Exzmq.Socket
  alias Exzmq.Tcp

  def just_an_example do
    Socket.test1  # Exzmq.Socket.test
    Tcp.Socket.test2 # Exzmq.Tcp.Socket.test – how can this be further aliased?
  end

end

We do not want to alias as TcpSocket.

Aliasing modules is an anti-feature

┃ [D] → Nested modules could be aliased at the top of the invoking module.
┃ lib/iex/server.ex:85:17 (IEx.Server.take_over)

I really don't agree with this one at all. It's stuff like this being the "default" that have largely
put me off using Rubocop.

I think the defaults should be things that are largely uncontroversial. Having it is fine, but
IMHO the trade offs involved in using module aliases are huge. You'll never get universal
consensus on any issue, but I think the tool should err on the side of caution as the default.

I'm not sure what the right interface to all this is. Maybe checks could be tagged as "opinionated"
and you could have a flag like

mix credo --full

Warn when module contains function of same name but different arity of behaviour callback

Example:

defmodule Example
  use GenServer

  def handle_call(:x, state) do
    # Notice the missing "from" parameter
    {:reply, :y, state}
  end
end

Should warn on Example.handle_call/2 being similar to Example.handle_call/3 since the @behaviour added via use GenServer has a @callback handle_call/3.

unused return value false positive?

Hello,
When running mix credo, I'm getting:

┃ [W] ↗ There should be no unused return values for Enum.flat_map().
┃       lib/exuser_update_service/config.ex:89:13 (EXUserUpdateService.Config.testcase)

Code in question:

  def testcase(configs) do
    if Enum.empty?(configs) do
      {:error, "No config"}
    else
      Logger.info("Configuring Workers") # having a line here that does anything seems to trigger it.
      {:ok, Enum.flat_map(configs, fn config ->
          make_configs(config[:topic], config[:partitions])
      end)}
    end
  end

  def make_configs(topic, partitions) do
    Enum.map(partitions, fn partition_num ->
      %{partition: partition_num, topic: topic}
    end)
  end

I am actually using the return value (returning it), but it seems like having the extra line after the else is triggering the warning somehow? If I remove that Logger line, the warning goes away.

example output (showing that the return values are used)

iex(3)> confs = [[topic: "topic1", partitions: [1, 2, 3]], [topic: "topic2", partitions: [1, 2, 3]]]
[[topic: "topic1", partitions: [1, 2, 3]],
 [topic: "topic2", partitions: [1, 2, 3]]]
iex(4)> Config.testcase(confs)
{:ok,
 [%{partition: 1, topic: "topic1"}, %{partition: 2, topic: "topic1"},
  %{partition: 3, topic: "topic1"}, %{partition: 1, topic: "topic2"},
  %{partition: 2, topic: "topic2"}, %{partition: 3, topic: "topic2"}]}

Project Roadmap to v1.0

I've seen promising Elixir projects getting a good start and then they somehow fade away. Maintainers get dragged away, no clear path was set and then the projects started meandering, not updating when Elixir reached 1.0, no longer responding to issues, the project merely survives because "this idea is great" ...

I don't want that to happen here and so I am continuing a tradition started in the ElixirStatus project: Below you find a rough "Roadmap to 1.0".

Please be aware of two things:

• all of this is up for discussion, which is why I publish this as a GitHub Issue instead of a blog post
• the version numbers are not set in stone, they are just meant as rough "milestones"

With that in mind, here we go:

0.1.x - getting off the ground

• fixing bugs
• improving the README
• fixing the UI (esp. wording)

0.2.x - more checks and utility

• more checks
  I still have a list of checks I want to implement and think it is good to "get this done" in order to appeal to a wider audience
• error codes
  I have an idea for a simple yet effective way to include error codes (or exit statuses) so that you can differentiate between issues in your CI builds, to e.g. react differently to Warnings than to Readability issues. By default, "Software design" issues like "TODO: ..." comments will not result in an error, but you will be able to customize this to your needs. there is an error when you would see issues in the output. But you can customize that on a per check basis!

0.3.x - formats and ignore options

• there is a "formatter API" to support implementation of different formatters (file list, Flycheck, ...)
• we have a way to disable Credo for specific functions

0.4.x - community

• the idea is that at this point, all checks I personally want to see in Credo have been implemented, so this will be a great moment to "let go" a bit and encourage community contributions even more
• to achieve this I want to make it dead simple to (a) create your own closed-source checks, but at the same time make it super-easy to contribute your checks back to the community
• all the utility stuff living under the Credo.Code.* namespace will be polished to provide a kind of toolbox for creating checks

0.5.0

0.6.0

0.7.0

0.8.x

0.9.x

• introduce Credo.Execution struct

0.10.x - almost final changes - ⚡ (we are here)

• refactor internals to support post 1.0 changes
• fix known bugs

0.11.x - final changes

• finalize structure of Credo.Execution struct
• freeze the public API
• improve docs of the public API
• according to this vaporware roadmap, this will be the last release before 1.0

---

Lastly, there are things this project could benefit from which are on my radar but did not fit well into the above list.

This includes trivial things like a nice project homepage (because a project about educating developers should at least be able to educate them about its existence, right?), but this also includes specificly non-trivial things like speeding Credo up on medium to large repos (e.g. through smarter code, caching, you name it) or adding a way to ignore certain issues for certain lines of code (as suggested in #5).

---

TL;DR There is a plan. Please feel free to provide encouraging words and critical thoughts below.

Credo.Check.Refactor.MatchInCondition explained

A brand new phoenix project triggers this warning, and when I went to explain it all it says is that it shouldn't be done. Can you explain why this is considered bad form or why it should be avoided?

Heres the function phoenix generates in the error helper:

  def error_tag(form, field) do
    if error = form.errors[field] do
      content_tag :span, translate_error(error), class: "help-block"
    end
  end

credo suggest fails if there's one and only one defexception

If target files contain just one defexception in total, mix credo suggest can detect no functions/modules.

Example:

# lib/example.ex (let's say no other file is in lib/)

defmodule I_am_snake_case_module_check_me_credo do
end

defmodule TheError do
  defexception [:message]
end

# If uncomment this, credo will start functioning again
#
#defmodule AnotherError do
#  defexception [:message]
#end

$ mix credo
Checking 1 source file ...

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.03 seconds (0.00s to load, 0.03s running checks)
0 mods/funs, found no issues.

Showing priority issues: ↑ ↗ →  (use `--strict` to show all issues, `--help` for options).

$ uncomment_that_thing
$ mix credo
Checking 1 source file ...

  Code Readability                                                                                                                                                                                          
┃ 
┃ [R] ↗ Module names should be written in PascalCase.
┃       lib/example.ex:1:11 (I_am_snake_case_module_check_me_credo)

(--- snip ---)

Analysis took 0.04 seconds (0.00s to load, 0.04s running checks)
3 mods/funs, found 4 code readability issues.

Showing priority issues: ↑ ↗ →  (use `--strict` to show all issues, `--help` for options).

Tested with credo 0.2.4 on on Elixir 1.1.1, Erlang/OTP 18.

Here's clonable working example: https://gitlab.com/uasi/20151223-credo-defexception

consistency issue bug with parenthesis spaces

Credo reports that "There are no spaces after/before parentheses, but here there are." for this line:

body = URI.encode_query %{ query: options.query }

Changing the code to this removes the consistency issue notification:

body = URI.encode_query %{query: options.query} 

Obviously, those are not parenthesis :)

Missing word in `mix credo categories` help text

https://github.com/rrrene/credo/blob/master/lib/credo/cli/output/categories.ex#L28

The above line should read:
of your code, but how easy it is to digest.

It's missing the word how.

Release tags

Hi,

Could you please tag releases as they come up? It would help me build ports for FreeBSD :)

Credo throws an error when run over Phoenix

I cloned the Phoenix repo to test out credo. After executing

mix credo --all  --format=oneline

I got the following error:

** (EXIT from #PID<0.47.0>) an exception was raised:
    ** (Protocol.UndefinedError) protocol Enumerable not implemented for nil
        (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
        (elixir) lib/enum.ex:116: Enumerable.reduce/3
        (elixir) lib/enum.ex:679: Enum.fetch/2
        (elixir) lib/enum.ex:311: Enum.at/3
        lib/credo/check/readability/function_names.ex:43: Credo.Check.Readability.FunctionNames.issues_for_definition/3
        lib/credo/check/readability/function_names.ex:35: Credo.Check.Readability.FunctionNames.traverse/3
        (elixir) lib/macro.ex:188: anonymous fn/4 in Macro.do_traverse/4
        (elixir) lib/enum.ex:1151: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3

12:57:43.336 [error] Task #PID<0.113.0> started from #PID<0.47.0> terminating
** (Protocol.UndefinedError) protocol Enumerable not implemented for nil
    (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
    (elixir) lib/enum.ex:116: Enumerable.reduce/3
    (elixir) lib/enum.ex:679: Enum.fetch/2
    (elixir) lib/enum.ex:311: Enum.at/3
    lib/credo/check/readability/function_names.ex:43: Credo.Check.Readability.FunctionNames.issues_for_definition/3
    lib/credo/check/readability/function_names.ex:35: Credo.Check.Readability.FunctionNames.traverse/3
    (elixir) lib/macro.ex:188: anonymous fn/4 in Macro.do_traverse/4
    (elixir) lib/enum.ex:1151: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
Function: #Function<8.71295517/0 in Credo.Check.Runner.run/2>
    Args: []

12:57:43.351 [error] GenServer Credo.Supervisor terminating
** (Protocol.UndefinedError) protocol Enumerable not implemented for nil
    (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
    (elixir) lib/enum.ex:116: Enumerable.reduce/3
    (elixir) lib/enum.ex:679: Enum.fetch/2
    (elixir) lib/enum.ex:311: Enum.at/3
    lib/credo/check/readability/function_names.ex:43: Credo.Check.Readability.FunctionNames.issues_for_definition/3
    lib/credo/check/readability/function_names.ex:35: Credo.Check.Readability.FunctionNames.traverse/3
    (elixir) lib/macro.ex:188: anonymous fn/4 in Macro.do_traverse/4
    (elixir) lib/enum.ex:1151: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
Last message: {:EXIT, #PID<0.47.0>, {%Protocol.UndefinedError{description: nil, protocol: Enumerable, value: nil}, [{Enumerable, :impl_for!, 1, [file: 'lib/enum.ex', line: 1]}, {Enumerable, :reduce, 3, [file: 'lib/enum.ex', line: 116]}, {Enum, :fetch, 2, [file: 'lib/enum.ex', line: 679]}, {Enum, :at, 3, [file: 'lib/enum.ex', line: 311]}, {Credo.Check.Readability.FunctionNames, :issues_for_definition, 3, [file: 'lib/credo/check/readability/function_names.ex', line: 43]}, {Credo.Check.Readability.FunctionNames, :traverse, 3, [file: 'lib/credo/check/readability/function_names.ex', line: 35]}, {Macro, :"-do_traverse/4-fun-0-", 4, [file: 'lib/macro.ex', line: 188]}, {Enum, :"-map_reduce/3-lists^mapfoldl/2-0-", 3, [file: 'lib/enum.ex', line: 1151]}]}}
State: {:state, {:local, Credo.Supervisor}, :one_for_one, [{:child, #PID<0.66.0>, Credo.Service.SourceFileScopes, {Credo.Service.SourceFileScopes, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileScopes]}, {:child, #PID<0.65.0>, Credo.Service.SourceFileCodeOnly, {Credo.Service.SourceFileCodeOnly, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileCodeOnly]}, {:child, #PID<0.64.0>, Credo.Service.SourceFileWithoutStringAndSigils, {Credo.Service.SourceFileWithoutStringAndSigils, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileWithoutStringAndSigils]}], :undefined, 3, 5, [], Supervisor.Default, {:ok, {{:one_for_one, 3, 5}, [{Credo.Service.SourceFileWithoutStringAndSigils, {Credo.Service.SourceFileWithoutStringAndSigils, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileWithoutStringAndSigils]}, {Credo.Service.SourceFileCodeOnly, {Credo.Service.SourceFileCodeOnly, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileCodeOnly]}, {Credo.Service.SourceFileScopes, {Credo.Service.SourceFileScopes, :start_link, []}, :permanent, 5000, :worker, [Credo.Service.SourceFileScopes]}]}}}

Incorrect operator detected when used inside a binary

Credo is yelling at me about this (below is an example only):

def gen_name(name) when is_binary(name),
do: "#{String.replace_suffix(name, "-test", "")}_name"

Invoked as gen_name("my-test") == "my_name".

[C] ↗ lib/project/text.ex:155:40 There are spaces around operators most of the time, but not here.

Assuming that it's looking at the - in the binary and getting mad that there are no spaces, but naturally I can't add spaces because that would change the output.

I can disable, sure, just reporting in case this is a bug which needs fixing.

Compilation failure

I may be using this incorrectly, but I tried installing this on two projects and it fails during compilation on both projects. Here's the error:

== Compilation error on file lib/credo/code/module.ex ==
** (ArgumentError) argument error
    :erlang.++(nil, [:when])
    lib/credo/code/module.ex:41: (module)
    (stdlib) erl_eval.erl:669: :erl_eval.do_apply/6

could not compile dependency :credo, "mix compile" failed. You can recompile this dependency with "mix deps.compile credo", update it with "mix deps.update credo" or clean it with "mix deps.clean credo"

To reproduce I add the dependency, run mix deps.get and then mix compile. Thanks for working on this. I'm super excited to give it a try! :D

Matches when pattern matching on binaries return false consistency report

A situation like:

@spec foo(binary, list(integer)) :: binary | no_return
def foo(data, length_acc) do
  len = length_acc |> Enum.sum
  <<data::size(len)-binary, _::binary>>
end

Would result in There are spaces around operators most of the time, but not here. on the <<data::size(len)-binary, _::binary>>-line

Incorrectly identifies -1 as an operator requiring space

I have the following in one of my modules

@max -1

This incorrectly identifies an issue of:

┃ [C] ↗ There are spaces around operators most of the time, but not here.
┃       lib/guardian/permissions.ex:125:7 (Guardian.Permissions)

Do not require @moduledoc for exception modules

By default, modules containing defexception should be exempt from the Readability.ModuleDoc.

Persistent handling of issues

I really like that this tool is all about presenting, vs enforcing, various code issues. But if I, or people in my team, decides that something is not a problem it would be very nice if credo could remember that.

Maybe something like credo ignore <type|issueno> <source:lineno:columnno> and a suitable file to store it. To actually implement this I would either store the entire offending line, or perhaps a hash of it. Then use this to detect if the line has changed and reenable the check.

There probably needs an corresponding credo clean command to clear out old ignored lines. This should be separate so that running the credo commands doesn't remove ignores while editing.

I would also recommend adding this to version control to share the knowledge and track which & why certain issues have been ignored.

exit code should not be 0 when issues found

otherwise builds on ci will pass when should not

Incorrect consistency report

Credo is looking great! Got this somewhat confusing consistency error though:

  Consistency                                                                             
┃ 
┃ [C] ↗ Exception modules should be named consistently. It seems your strategy is to 
┃       prefix them with `Syntax`, but `GraphQL.SyntaxError` does not follow that 
┃       convention."
┃       lib/graphql/error/syntax_error.ex:1:11 (GraphQL.SyntaxError)

The only thing I can see that might be wrong is that the module name doesn't match the folder structure.

Code can be found here: https://github.com/joshprice/graphql-elixir/blob/master/lib/graphql/error/syntax_error.ex

Incorrect space around operators warning in pipe

  def to_timestamp(datetime) do
    datetime
    |> :calendar.datetime_to_gregorian_seconds
    |> -(@epoch)
  end

I get the There are spaces around operators most of the time, but not here. report for the last line of the pipeline.

Cryptic error when there is no issue in the given line

Thank you for an amazing piece of software! I have a bug to report:

Description

When you run:

mix credo lib/chapter_sampler.ex:6

and there is no issue in the given line, you will get quite a cryptic error:

Checking 1 source file ...
** (UndefinedFunctionError) undefined function nil.scope/0
    nil.scope()
    Credo.CLI.Output.Explain.print_issues/7
    lib/credo/cli/command/explain.ex:22: Credo.CLI.Command.Explain.run/2
    lib/credo/cli.ex:53: Credo.CLI.main/1
    (mix) lib/mix/cli.ex:58: Mix.CLI.run_task/2

Real life case

First I run mix credo --strict and it reported an issue in lib/chapter_sampler.ex:6. Then I wanted to get the details of this issue, but I didn't specify the --strict switch and I got the above error. The issue was one of these "strict" ones.

If/else blocks should not have a negated condition in `if`.

In writing if/else statements, I would normally prefer to place the short shorter block in front, and leave the larger block behind. If it's written the other way round, the reader might miss the else clause.

For example,

if ! valid(item) do
  {:error, :invalid_item}
else
  ... processing ...
  ...
  {:ok, result}
end

It is not every time we have the opportunity to rewrite the valid/1 method to invalid/1 such that there isn't a negation in the if clause. And we probably agree in not using unless/else even in such situations.

False negative on "unused return values" ?

Hi. The code at photomattmills/ecto_neo4j#4 reports a warning:

┃ [W] ↗ There should be no unused return values for Enum.map().
┃       lib/ecto_neo4j.ex:61:15 (Ecto.Neo4j.sort_column)

That I believe is incorrect. The context is a value returned from an if check being the last evaluated expression in a function:

    if hd(cols) do
      coercer = fn({name, type}) -> coerce(type, col[Atom.to_string(name)]) end 
      cols |> Enum.map(coercer)
    else
      [nil]
    end

It looks like the presence of the [nil] and end lines below the Enum.map/2 is confusing it into thinking that there are additional expressions leading the map value to be discarded.

Pipe chain should start with a raw value.

Thanks for Credo. :)

I'm not sure whether the above is a good refactoring opportunity.

For example, the code below triggers the above, and seems like a pretty common and valid style.

result = Stream.chunk(1..100, 10) |> Enum.take(3)

result = 
  if map[:foo] do
    map[:foo]
  else
    map[:default]
  end
  |> String.to_integer

Wrong refactory opportunity on `defmacro __using__(_)`

Credo warns about too complex functions when using defmacro __using__ which is unnecessary to split up.

Is "Case statements should not only contain `true` and `false`." a good default?

Suggesting an if..else is less strict, which could potentially hide bugs because if tests for truthy rather than true.

case Map.fetch!(map, key) do
  true -> :good
  false -> :bad
end

Situations where one might use a case like this are when a map or keyword have been passed from another application.

Consistency error in regex

Hello!

Awesome project!

I'm running it on my application and one of the warnings I get is this one

  Consistency                                                                                                                                                                                                                      
┃ 
┃ [C] ↗ There is no whitespace around parentheses/brackets most of the time, but here there is.
┃       web/models/user.ex:24 (Hosting.User)

Thing is, the 24th line in user.ex is

@username_regex ~r/^[A-z0-9 ]+$/

Thing is, this is regex. I want to match the whitespace. I think this is a bug :)

Michal Hojgr

"Space around operators most of the time, but not here." Issue

┃    __ CODE IN QUESTION
┃
┃       |> Date.shift(days: n * -1)
┃                               ^
┃
┃    __ WHY IT MATTERS
┃
┃       Don't use spaces after operators like `+`, `-`, `*` and `/`. This is the
┃       **preferred** way, although other styles are possible, as long as it is
┃       applied consistently.

I do prefer space around my operators, but I do also like to have negative numbers. -20 * -13 also seems acceptable to me as well. Thoughts?