credo icon indicating copy to clipboard operation
credo copied to clipboard

Analysis of checks in regards to Elixir versions

Open josevalim opened this issue 6 years ago • 4 comments

Outdated by warnings in the compiler (since v1.4):

  • name_redeclaration_by_assignment.ex
  • name_redeclaration_by_case.ex
  • name_redeclaration_by_fn.ex

Outdated by the formatter in Elixir v1.6:

  • line_endings.ex
  • space_around_operators.ex
  • space_in_parentheses.ex
  • tabs_or_spaces.ex
  • large_numbers.ex
  • max_line_length.ex
  • parentheses_in_condition.ex
  • prefer_unquoted_atoms.ex
  • redundant_blank_lines.ex
  • semicolons.ex
  • space_after_commas.ex
  • trailing_blank_line.ex
  • trailing_white_space.ex

Will be solved in Elixir v1.7:

  • lazy_logging.ex

Checks that have a high false positive rate (IMO):

  • append_single_item.ex
  • case_trivial_matches.ex
  • pipe_chain_start.ex

josevalim avatar Feb 12 '18 09:02 josevalim

It appears that lazy_logging.ex is solved via the macro in Elixir 1.6.2. https://github.com/elixir-lang/elixir/blob/v1.6.2/lib/logger/lib/logger.ex#L746-L754

coreyhaines avatar Mar 05 '18 20:03 coreyhaines

@rrrene How do you envision Credo working in relation to the Elixir formatter? If we were to take the stance that "Credo complements the Formatter, and doesn't ever try to reproduce or contradict it" (contradict meaning enforce a style which the formatter would undo), it would allow us to remove a number of checks as mentioned above, speeding up execution time, and also would help weed out some of the proposals which go against the formatter. Thoughts?

TheFirstAvenger avatar Dec 08 '19 21:12 TheFirstAvenger

Credo complements the Formatter, and doesn't ever try to [...] contradict it

@TheFirstAvenger In my mind, Credo's default config should conform with the formatter. I am not sure if this is trivial, as the formatter changes slightly over time. We should probably add a test suite that checks this.

That said, we should allow people to customize their checks without "respecting" the formatter. I think it is wrong to assume that everybody uses it, so having the defaults conform with it seems the way to go.

Thoughts?

rrrene avatar Dec 09 '19 17:12 rrrene

I guess where I am heading with this is more the idea of identifying the checks that are unnecessary if you also run mix format --check-formatted, and retiring them from Credo (maybe to a formatter-plugin that can be included if a user isn't using the formatter?). The reason for this change would be that a number of these checks are consistency and readability checks which are often the ones cited in issue reports about credo taking forever to run (e.g. this and this). If most users are using the formatter anyway, then the cycles spent to run these redundant checks are wasted.

TheFirstAvenger avatar Dec 10 '19 01:12 TheFirstAvenger