Adds credo checks to CI (#66)

* Adds credo
* Fix all credo issues and warnings
* Update CI to use credo

.credo.exs

@@ -0,0 +1,211 @@
+# This file contains the configuration for Credo and you are probably reading
+# this after creating it with `mix credo.gen.config`.
+#
+# If you find anything wrong or unclear in this file, please report an
+# issue on GitHub: https://github.com/rrrene/credo/issues
+#
+%{
+  #
+  # You can have as many configs as you like in the `configs:` field.
+  configs: [
+    %{
+      #
+      # Run any config using `mix credo -C <name>`. If no config name is given
+      # "default" is used.
+      #
+      name: "default",
+      #
+      # These are the files included in the analysis:
+      files: %{
+        #
+        # You can give explicit globs or simply directories.
+        # In the latter case `**/*.{ex,exs}` will be used.
+        #
+        included: [
+          "lib/",
+          "src/",
+          "test/",
+          "web/",
+          "apps/*/lib/",
+          "apps/*/src/",
+          "apps/*/test/",
+          "apps/*/web/"
+        ],
+        excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/"]
+      },
+      #
+      # Load and configure plugins here:
+      #
+      plugins: [],
+      #
+      # If you create your own checks, you must specify the source files for
+      # them here, so they can be loaded by Credo before running the analysis.
+      #
+      requires: [],
+      #
+      # If you want to enforce a style guide and need a more traditional linting
+      # experience, you can change `strict` to `true` below:
+      #
+      strict: false,
+      #
+      # To modify the timeout for parsing files, change this value:
+      #
+      parse_timeout: 5000,
+      #
+      # If you want to use uncolored output by default, you can change `color`
+      # to `false` below:
+      #
+      color: true,
+      #
+      # You can customize the parameters of any check by adding a second element
+      # to the tuple.
+      #
+      # To disable a check put `false` as second element:
+      #
+      #     {Credo.Check.Design.DuplicatedCode, false}
+      #
+      checks: %{
+        enabled: [
+          #
+          ## Consistency Checks
+          #
+          {Credo.Check.Consistency.ExceptionNames, []},
+          {Credo.Check.Consistency.LineEndings, []},
+          {Credo.Check.Consistency.ParameterPatternMatching, []},
+          {Credo.Check.Consistency.SpaceAroundOperators, []},
+          {Credo.Check.Consistency.SpaceInParentheses, []},
+          {Credo.Check.Consistency.TabsOrSpaces, []},
+
+          #
+          ## Design Checks
+          #
+          # You can customize the priority of any check
+          # Priority values are: `low, normal, high, higher`
+          #
+          {Credo.Check.Design.AliasUsage,
+           [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
+
+          #
+          ## Readability Checks
+          #
+          {Credo.Check.Readability.AliasOrder, []},
+          {Credo.Check.Readability.FunctionNames, []},
+          {Credo.Check.Readability.LargeNumbers, []},
+          {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]},
+          {Credo.Check.Readability.ModuleAttributeNames, []},
+          {Credo.Check.Readability.ModuleDoc, []},
+          {Credo.Check.Readability.ModuleNames, []},
+          {Credo.Check.Readability.ParenthesesInCondition, []},
+          {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
+          {Credo.Check.Readability.PipeIntoAnonymousFunctions, []},
+          {Credo.Check.Readability.PredicateFunctionNames, []},
+          {Credo.Check.Readability.PreferImplicitTry, []},
+          {Credo.Check.Readability.RedundantBlankLines, []},
+          {Credo.Check.Readability.Semicolons, []},
+          {Credo.Check.Readability.SpaceAfterCommas, []},
+          {Credo.Check.Readability.StringSigils, []},
+          {Credo.Check.Readability.TrailingBlankLine, []},
+          {Credo.Check.Readability.TrailingWhiteSpace, []},
+          {Credo.Check.Readability.UnnecessaryAliasExpansion, []},
+          {Credo.Check.Readability.VariableNames, []},
+          {Credo.Check.Readability.WithSingleClause, []},
+
+          #
+          ## Refactoring Opportunities
+          #
+          {Credo.Check.Refactor.Apply, []},
+          {Credo.Check.Refactor.CondStatements, []},
+          {Credo.Check.Refactor.CyclomaticComplexity, []},
+          {Credo.Check.Refactor.FunctionArity, []},
+          {Credo.Check.Refactor.LongQuoteBlocks, []},
+          {Credo.Check.Refactor.MatchInCondition, []},
+          {Credo.Check.Refactor.MapJoin, []},
+          {Credo.Check.Refactor.NegatedConditionsInUnless, []},
+          {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+          {Credo.Check.Refactor.Nesting, []},
+          {Credo.Check.Refactor.UnlessWithElse, []},
+          {Credo.Check.Refactor.WithClauses, []},
+          {Credo.Check.Refactor.FilterFilter, []},
+          {Credo.Check.Refactor.RejectReject, []},
+          {Credo.Check.Refactor.RedundantWithClauseResult, []},
+
+          #
+          ## Warnings
+          #
+          {Credo.Check.Warning.ApplicationConfigInModuleAttribute, []},
+          {Credo.Check.Warning.BoolOperationOnSameValues, []},
+          {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+          {Credo.Check.Warning.IExPry, []},
+          {Credo.Check.Warning.IoInspect, []},
+          {Credo.Check.Warning.OperationOnSameValues, []},
+          {Credo.Check.Warning.OperationWithConstantResult, []},
+          {Credo.Check.Warning.RaiseInsideRescue, []},
+          {Credo.Check.Warning.SpecWithStruct, []},
+          {Credo.Check.Warning.WrongTestFileExtension, []},
+          {Credo.Check.Warning.UnusedEnumOperation, []},
+          {Credo.Check.Warning.UnusedFileOperation, []},
+          {Credo.Check.Warning.UnusedKeywordOperation, []},
+          {Credo.Check.Warning.UnusedListOperation, []},
+          {Credo.Check.Warning.UnusedPathOperation, []},
+          {Credo.Check.Warning.UnusedRegexOperation, []},
+          {Credo.Check.Warning.UnusedStringOperation, []},
+          {Credo.Check.Warning.UnusedTupleOperation, []},
+          {Credo.Check.Warning.UnsafeExec, []}
+        ],
+        disabled: [
+          #
+          # Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`)
+
+          # You can also customize the exit_status of each check.
+          # If you don't want TODO comments to cause `mix credo` to fail, just
+          # set this value to 0 (zero).
+          #
+          {Credo.Check.Design.TagTODO, [exit_status: 2]},
+          {Credo.Check.Design.TagFIXME, []},
+
+          #
+          # Controversial and experimental checks (opt-in, just move the check to `:enabled`
+          #   and be sure to use `mix credo --strict` to see low priority checks)
+          #
+          {Credo.Check.Consistency.MultiAliasImportRequireUse, []},
+          {Credo.Check.Consistency.UnusedVariableNames, []},
+          {Credo.Check.Design.DuplicatedCode, []},
+          {Credo.Check.Design.SkipTestWithoutComment, []},
+          {Credo.Check.Readability.AliasAs, []},
+          {Credo.Check.Readability.BlockPipe, []},
+          {Credo.Check.Readability.ImplTrue, []},
+          {Credo.Check.Readability.MultiAlias, []},
+          {Credo.Check.Readability.NestedFunctionCalls, []},
+          {Credo.Check.Readability.SeparateAliasRequire, []},
+          {Credo.Check.Readability.SingleFunctionToBlockPipe, []},
+          {Credo.Check.Readability.SinglePipe, []},
+          {Credo.Check.Readability.Specs, []},
+          {Credo.Check.Readability.StrictModuleLayout, []},
+          {Credo.Check.Readability.WithCustomTaggedTuple, []},
+          {Credo.Check.Refactor.ABCSize, []},
+          {Credo.Check.Refactor.AppendSingleItem, []},
+          {Credo.Check.Refactor.DoubleBooleanNegation, []},
+          {Credo.Check.Refactor.FilterReject, []},
+          {Credo.Check.Refactor.IoPuts, []},
+          {Credo.Check.Refactor.MapMap, []},
+          {Credo.Check.Refactor.ModuleDependencies, []},
+          {Credo.Check.Refactor.NegatedIsNil, []},
+          {Credo.Check.Refactor.PipeChainStart, []},
+          {Credo.Check.Refactor.RejectFilter, []},
+          {Credo.Check.Refactor.VariableRebinding, []},
+          {Credo.Check.Warning.LazyLogging, []},
+          {Credo.Check.Warning.LeakyEnvironment, []},
+          {Credo.Check.Warning.MapGetUnsafePass, []},
+          {Credo.Check.Warning.MixEnv, []},
+          {Credo.Check.Warning.UnsafeToAtom, []}
+
+          # {Credo.Check.Refactor.MapInto, []},
+
+          #
+          # Custom checks can be created using `mix credo.gen.check`.
+          #
+        ]
+      }
+    }
+  ]
+}

.github/workflows/ci.yml

@@ -1,54 +1,60 @@
 name: CI
 
 on:
-  pull_request:
   push:
     branches:
-      - main
+      - "*"
 
 jobs:
   lint:
-    runs-on: ubuntu-20.04
+    runs-on: ${{ matrix.os }}
     env:
-      MIX_ENV: test
+      MIX_ENV: dev
     name: Lint
+    strategy:
+      matrix:
+        os: ["ubuntu-20.04"]
+        elixir: ["1.13"]
+        otp: ["24"]
     steps:
       - uses: actions/checkout@v2
-      - uses: erlef/setup-elixir@v1
+      - uses: erlef/setup-beam@v1
+        with:
+          otp-version: ${{ matrix.otp }}
+          elixir-version: ${{ matrix.elixir }}
+      - uses: actions/cache@v2
         with:
-          otp-version: "24"
-          elixir-version: "1.12"
-      - run: mix deps.get && mix deps.unlock --check-unused
+          path: deps
+          key: ${{ matrix.os }}-otp_${{ matrix.otp }}-elixir_${{ matrix.elixir }}-mix_${{ hashFiles('**/mix.lock') }}
+          restore-keys: ${{ matrix.os }}-otp_${{ matrix.otp }}-elixir_${{ matrix.elixir }}-mix_
+      - run: mix deps.get
+      - run: mix deps.compile
       - run: mix format --check-formatted
+      - run: mix deps.unlock --check-unused
+      - run: mix credo --strict --all
+
   test:
-    runs-on: ubuntu-20.04
+    runs-on: ${{ matrix.os }}
+    name: Test Elixir ${{ matrix.elixir }}, OTP ${{ matrix.otp }}, OS ${{ matrix.os }}
     env:
       MIX_ENV: test
-    name: Ubuntu / OTP ${{matrix.otp}} / Elixir ${{matrix.elixir}}
     strategy:
       fail-fast: false
       matrix:
-        include:
-          - elixir: "1.12"
-            otp: "24"
-          - elixir: "1.12"
-            otp: "23"
-          - elixir: "1.11"
-            otp: "24"
-          - elixir: "1.11"
-            otp: "23"
-          - elixir: "1.10"
-            otp: "24"
-          - elixir: "1.10"
-            otp: "23"
-          - elixir: "1.8"
-            otp: "21"
+        os: ["ubuntu-20.04"]
+        elixir: ["1.13", "1.12", "1.11"]
+        otp: ["24", "23", "22"]
     steps:
       - uses: actions/checkout@v2
-      - uses: erlef/setup-elixir@v1
+      - uses: erlef/setup-beam@v1
+        with:
+          otp-version: ${{ matrix.otp }}
+          elixir-version: ${{ matrix.elixir }}
+      - uses: actions/cache@v2
         with:
-          otp-version: ${{matrix.otp}}
-          elixir-version: ${{matrix.elixir}}
+          path: deps
+          key: ${{ matrix.os }}-otp_${{ matrix.otp }}-elixir_${{ matrix.elixir }}-mix_${{ hashFiles('**/mix.lock') }}
+          restore-keys: ${{ matrix.os }}-otp_${{ matrix.otp }}-elixir_${{ matrix.elixir }}-mix_
       - run: mix deps.get --only test
       - run: mix deps.compile
       - run: mix compile

lib/ecto/adapters/sqlite3.ex

@@ -165,12 +165,14 @@ defmodule Ecto.Adapters.SQLite3 do
   def storage_down(options) do
     db_path = Keyword.fetch!(options, :database)
 
-    with :ok <- File.rm(db_path) do
-      File.rm(db_path <> "-shm")
-      File.rm(db_path <> "-wal")
-      :ok
-    else
-      _ -> {:error, :already_down}
+    case File.rm(db_path) do
+      :ok ->
+        File.rm(db_path <> "-shm")
+        File.rm(db_path <> "-wal")
+        :ok
+
+      _otherwise ->
+        {:error, :already_down}
     end
   end
 
@@ -194,7 +196,7 @@ defmodule Ecto.Adapters.SQLite3 do
   end
 
   @impl Ecto.Adapter.Migration
-  def supports_ddl_transaction?(), do: false
+  def supports_ddl_transaction?, do: false
 
   @impl Ecto.Adapter.Migration
   def lock_for_migrations(_meta, _options, fun) do

lib/ecto/adapters/sqlite3/codec.ex

@@ -26,19 +26,15 @@ defmodule Ecto.Adapters.SQLite3.Codec do
   def decimal_decode(nil), do: {:ok, nil}
 
   def decimal_decode(x) when is_float(x) do
-    try do
-      {:ok, Decimal.from_float(x)}
-    catch
-      Decimal.Error -> :error
-    end
+    {:ok, Decimal.from_float(x)}
+  catch
+    Decimal.Error -> :error
   end
 
   def decimal_decode(x) when is_binary(x) or is_integer(x) do
-    try do
-      {:ok, Decimal.new(x)}
-    catch
-      Decimal.Error -> :error
-    end
+    {:ok, Decimal.new(x)}
+  catch
+    Decimal.Error -> :error
   end
 
   def decimal_decode(_), do: :error

lib/ecto/adapters/sqlite3/connection.ex

@@ -3,6 +3,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
 
   @behaviour Ecto.Adapters.SQL.Connection
 
+  alias Ecto.Adapters.SQL
   alias Ecto.Migration.Constraint
   alias Ecto.Migration.Index
   alias Ecto.Migration.Reference
@@ -19,7 +20,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
   defp default_opts(opts) do
     opts
     |> Keyword.put_new(:journal_mode, :wal)
-    |> Keyword.put_new(:cache_size, -64000)
+    |> Keyword.put_new(:cache_size, -64_000)
     |> Keyword.put_new(:temp_store, :memory)
     |> Keyword.put_new(:pool_size, 5)
   end
@@ -320,7 +321,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
   def explain_query(conn, query, params, opts) do
     case query(conn, build_explain_query(query), params, opts) do
       {:ok, %Exqlite.Result{} = result} ->
-        {:ok, Ecto.Adapters.SQL.format_table(result)}
+        {:ok, SQL.format_table(result)}
 
       error ->
         error