Jongbin Oh

I write something to think

8 minute read

Elixir가 아닌 다른 프로그래밍 언어를 사용해 협업하다 보면 그리운 라이브러리 중 하나가 Credo이다. code review에서 정적 분석기가 거를 수 있는 것들을 수정 요청할 때 그립다. 잔소리는 기계가 해야 한다. 개인 프로젝트를 할 때는 코드 일관성이 제대로 안 지켜지고 이랬다저랬다 하는 규칙들을 볼 때 그립다. 이런 건 다 Credo가 잡아주는데 말이지.

다른 정적 분석기 다른 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.

Credo는 교육 및 코드 일관성에 중점을 둔 Elixir 언어용 정적 코드 분석 도구입니다.

코드의 리팩토링 기회, 복잡한 코드 조각을 보여주고, 일반적인 실수에 대해 경고하고, 명명 체계의 불일치를 보여주며, 필요한 경우 원하는 코딩 스타일을 적용하는 데 도움을 줄 수 있습니다.

rrrene/credo - github.com

Elixir 언어의 정적 분석기라고 하면 Dialyzer가 빠질 수 없다. 정적 분석기라고 하면 연상되는 코드의 모순을 잡는 프로그램이다. 반면 Credo는 코드의 일관성과 교육에 중점을 둔 정적 분석기이다. 예를 들어 Dialyzer는 Pattern matching이 무조건 실패해서 실행될 수 없는 코드를 진단해 준다. 반면 Credo는 length(some_list) == 0 코드는 쓸데없이 비싸니깐 Enum.empty?(some_list) 코드를 사용하라고 추천해준다. 겹치는 영역이 없어서 Dialyzer와 Credo를 둘 다 사용하는 걸 추천한다.

Credo 체크 항목 몇 개를 더 예를 든다면 아래와 같다.

  • Enum.map/2 리턴 값을 사용하지 않아서 Enum.each/2 함수로 교체
  • length(some_list) == 0 은 비싸니깐 Enum.empty?(some_list) 함수로 교체
  • 프로덕션 코드에서 IO.inspect 를 사용하지 말라
  • 중복 코드가 50줄이 넘는 코드가 있으니 리팩터링을 하라

설치

의존 라이브러리로 credo 를 추가한다. 글을 쓰는 시점에는 credo 최신 버전은 1.7.7이다.

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

의존 라이브러리를 라이브러리 매니저로 내려받고 credo mix task를 실행한다.

$ mix deps.get
$ mix credo

mix credo --all - 사이드 프로젝트에 돌린 결과

Tbot-800.ex에 추가해서 돌려봤다.

$ mix credo --all
Checking 31 source files ...

  Software Design
┃ [D] → Found a TODO tag in a comment: # TODO 한글, 영문에 따라 트윗 캐릭터 계산이 다르다. url도 다름
┃       apps/builder/lib/builder/default_impl/tweet_item_builder.ex:11 #(Builder.DefaultImpl.TweetItemBuilder.build)
...

  Code Readability
┃ [R] → Modules should have a @moduledoc tag.
┃       apps/tbot800/lib/tbot800/default_impl/twitter_process.ex:1:11 #(Tbot800.DefaultImpl.TwitterProcess)
┃       apps/tbot800/lib/tbot800.ex:1:11 #(Tbot800)
┃ [R] → Modules should have a @moduledoc tag.
┃       apps/builder/lib/builder/default_impl/html_builder.ex:1:11 #(Builder.DefaultImpl.HtmlBuilder)
...

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

Analysis took 0.08 seconds (0.04s to load, 0.04s running 55 checks on 31 files)
78 mods/funs, found 20 code readability issues, 2 software design suggestions.

Showing priority issues: ↑ ↗ →  (use `mix credo explain` to explain issues, `mix credo --help` for options).

우선순위(priority)를 ↑ ↗ → 심볼로 표시하는 게 재미있고 가시성도 좋다. credo 제안(suggestion)에 대해 자세한 설명을 보려면 어떻게 하면 될까?

mix credo explain - 자세한 설명을 보고 싶을 때

설명을 보려면 mix credo explain 뒤에 위치 포맷 문자열을 인자로 넘기면 된다.

$ mix credo explain apps/builder/lib/builder/default_impl/html_builder.ex:1:11

  Builder.DefaultImpl.HtmlBuilder
┃   [R] Category: readability
┃    →  Priority: normal
┃
┃       Modules should have a @moduledoc tag.
┃       apps/builder/lib/builder/default_impl/html_builder.ex:1:11 (Builder.DefaultImpl.HtmlBuilder)
┃
┃    __ CODE IN QUESTION
┃
┃     1 defmodule Builder.DefaultImpl.HtmlBuilder do
┃                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
┃     2   @template """
┃     3   <!DOCTYPE html>
┃
┃    __ WHY IT MATTERS
┃
┃       Every module should contain comprehensive documentation.
┃
┃           # preferred
┃
┃           defmodule MyApp.Web.Search do
┃             @moduledoc """
┃             This module provides a public API for all search queries originating
┃             in the web layer.
┃             """
┃           end
┃
┃           # also okay: explicitly say there is no documentation
┃
┃           defmodule MyApp.Web.Search do
┃             @moduledoc false
┃           end
┃
┃       Many times a sentence or two in plain english, explaining why the module
┃       exists, will suffice. Documenting your train of thought this way will help
┃       both your co-workers and your future-self.
┃
┃       Other times you will want to elaborate even further and show some
┃       examples of how the module's functions can and should be used.
┃
┃       In some cases however, you might not want to document things about a module,
┃       e.g. it is part of a private API inside your project. Since Elixir prefers
┃       explicitness over implicit behaviour, you should "tag" these modules with
┃
┃           @moduledoc false
┃
┃       to make it clear that there is no intention in documenting it.
┃
┃       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.
┃
┃    __ CONFIGURATION OPTIONS
┃
┃       To configure this check, use this tuple
┃
┃         {Credo.Check.Readability.ModuleDoc, <params>}
┃
┃       with <params> being false or any combination of these keywords:
┃
┃         ignore_names:  All modules matching this regex (or list of regexes) will be ignored.
┃                        (defaults to [~r/(\.\w+Controller|\.Endpoint|\.\w+Live(\.\w+)?|\.Repo|\.Router|\.\w+Socket|\.\w+View|\.\w+HTML|\.\w+JSON|\.Telemetry|\.Layouts|\.Mailer)$/])
┃

설명이 감동이다.

credo 체크를 억제하는 방법

오탐 혹은 고치기엔 너무 복잡한 코드를 만날 수 있다. 다행히 주석으로 credo 체크를 억제할 수 있다. 사실 정적 분석기에서 필수적으로 제공해야 하는 기능이다.

# credo:disable-for-this-file - to disable for the entire file
# credo:disable-for-next-line - to disable for the next line
# credo:disable-for-previous-line - to disable for the previous line
# credo:disable-for-lines:<count> - to disable for the given number of lines (negative for previous lines)

credo:disable-begin, credo:disable-end 쌍을 왜 추가하지 않았을까? 이런 생각이 들었다. 쌍으로 항상 동작해야 하니 휴먼 에러가 더 많이 발생한다. 또한 쌍이 맞지 않으면 경고를 해줘야 해서 구현도 번거롭다. 그것보다는 독립적으로 사용해도 동작하는 주석 세트를 제공한다. 좋은 판단이다.

어떻게 credo 실행하고 check 실패를 다뤄야 할까?

credo를 가끔 돌려서 고치는 건 의미가 없다. 말만 그렇게 하고 잊을 수 있다. 가끔이 아니라 한 달에 한 번 혹은 일주일에 한 번처럼 주기를 정하면 그래도 의미가 있다. 난 주기를 정해놓는 것보다 반드시 지켜야 하는 걸 고르고 그걸 지키지 않으면 CI에서 테스트 실패가 나는 걸 선호한다. 외울 게 없고 간단한 규칙으로 돌아가기 때문이다.

실패하면 테스트 스크립트가 멈춰야 한다. 종료 상태(exit status)가 세팅되는지 확인해야 한다.

하나라도 credo 제안이 있으면 종료 상태가 0이 아니다.

$ mix credo
...
215 mods/funs, found 3 refactoring opportunities, 69 code readability issues, 7 software design suggestions.

$ echo $?
14

무조건 종료 상태를 0으로 만드는 --mute-exit-status 옵션도 제공한다. 고쳐야 할 목록은 다 출력하지만 성공 상태로 만들 때 쓸 수 있겠다. 이번에는 쓸 일은 없다.

$ mix credo --mute-exit-status
...
215 mods/funs, found 3 refactoring opportunities, 69 code readability issues, 7 software design suggestions.

$ echo $?
0

이제 반드시 지킬 것을 어떻게 표시할지 결정해야 한다. warning 카테고리가 보여서 반드시 지킬 걸을 warning 카테고리로 모으는 건 어떨지 생각했다. 모은 다음 mix credo suggest --checks warning 과 같이 warning 카테고리만 실행하는 방법이다. 이 방법으로 내가 원하는 체크만 할 수 있지만 Software Design, Code Readability 등과 같은 카테고리를 뭉개야 한다. 정 방법이 없으면 선택해야겠다.

우선순위(priority)가 눈에 들어온다. --min-priority high 옵션을 사용하면 high 우선순위 이상만 실행한다.

$ mix credo suggest --min-priority high
...
$ echo $?
4

내가 원하는 대로 우선순위를 덮어쓸 수 있을까?

# .credo.exs

[
  {Credo.Check.Readability.AliasOrder, [priority: :high]},
]

check가 공통으로 지원하는 옵션이다. .credo.exs 파일에서 정의할 수 있다. 좋다. 이걸로 하면 되겠다.

테스트 스크립트에는 mix credo suggest --min-priority high 프로그램 실행을 넣어두고 반드시 지켜야 할 check를 선별해서 우선순위를 high로 올린다.

우선순위를 high로 설정한 check 목록

credo 설정 파일에 기본 check 목록이 모두 있으니 생성한다.

$ mix credo gen.config
* creating .credo.exs

설정 파일 생성 Mix 태스크를 제공하니 정말 편하다. 아래는 1.7.8 디폴트 설정값에서 내가 수정한 check 목록이다. 우선순위만 바꿨다. 디폴트 우선순위가 high인 check 목록에 아래 목록을 더해서 검사한다.

[
  # Readability Checks
  {Credo.Check.Readability.AliasOrder, [priority: :high]},
  {Credo.Check.Readability.MaxLineLength, [priority: :high, max_length: 120]},
  {Credo.Check.Readability.ModuleDoc, [priority: :high]},
  {Credo.Check.Readability.StringSigils, [priority: :high]},
  {Credo.Check.Readability.WithSingleClause, [priority: :high]},
  {Credo.Check.Readability.MultiAlias, [priority: :high]},
  {Credo.Check.Readability.SeparateAliasRequire, [priority: :high]},
  {Credo.Check.Readability.Specs, [priority: :high]},
  {Credo.Check.Readability.StrictModuleLayout, [priority: :high]},
  {Credo.Check.Readability.WithCustomTaggedTuple, [priority: :high]},

  ## Refactoring Opportunities
  {Credo.Check.Refactor.Apply, [priority: :high]},
  {Credo.Check.Refactor.CondStatements, [priority: :high]},
  {Credo.Check.Refactor.FilterFilter, [priority: :high]},
  {Credo.Check.Refactor.MatchInCondition, [priority: :high]},
  {Credo.Check.Refactor.Nesting, [priority: :high]},
  {Credo.Check.Refactor.RejectReject, [priority: :high]},
  {Credo.Check.Refactor.AppendSingleItem, [priority: :high]},
  {Credo.Check.Refactor.MapMap, [priority: :high]},
  {Credo.Check.Refactor.PassAsyncInTestCases, [priority: :high]},

  # Warnings
  {Credo.Check.Warning.RaiseInsideRescue, [priority: :high]},
  {Credo.Check.Warning.SpecWithStruct, [priority: :high]},
]

아래는 활성화한 목록에 대한 간단한 설명이다.

GitHub workflow에 추가

#!/bin/sh
set -e

echo "==> Running credo..."

mix credo suggest --min-priority high

GitHub Actions workflow에 credo를 돌리는 step을 추가하지 않았다. 대신 테스트 step에 호출하는 script/test 스크립트에 credo 프로그램 실행을 추가했다. 될 수 있으면 CI는 when만 결정해야 한다. 로컬에서도 편하게 호출할 수 있다.

마치며

코드 일관성은 협업할 때는 필수적이다. 다른 사람은 코드를 볼 일이 없는 개인 프로젝트에도 유용하다. 누가 짰건 코드를 읽는 시간이 많으니깐. 그래서 credo는 개인 프로젝트에도 유용하다. 많은 credo check를 살펴보며 몰랐던 지식과 습관도 배우게 된다.

링크

You May Also Enjoy

빈 리스트에 대한 all? 함수의 리턴 값은? - 공허참(vacuous truth)

3 minute read

Enum.all?/1 함수를 사용한 함수 테스트할 때였던 걸로 기억한다. 비어 있는 리스트를 넣어서 테스트했을 때, 기대하지 않은 값이 나와서 의아했다. 비어 있는 리스트를 인자로 넘겼을 때, false 값이 나올 것으로 기대했다. 하지만 true 로 나오는 것이었다. Enum.a...