ci: adjust credo lint rules (#84)

.credo.exs

@@ -46,7 +46,7 @@
       # If you want to enforce a style guide and need a more traditional linting
       # experience, you can change `strict` to `true` below:
       #
-      strict: false,
+      strict: true,
       #
       # To modify the timeout for parsing files, change this value:
       #
@@ -75,68 +75,83 @@
           {Credo.Check.Consistency.SpaceAroundOperators, []},
           {Credo.Check.Consistency.SpaceInParentheses, []},
           {Credo.Check.Consistency.TabsOrSpaces, []},
+          {Credo.Check.Consistency.MultiAliasImportRequireUse, []},
 
           #
           ## 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: 2]},
-
-          # FIXMEs are allowed.
-          # {Credo.Check.Design.TagFIXME, []},
-
-          # 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.TagFIXME, []}
           {Credo.Check.Design.TagTODO, []},
+          {Credo.Check.Design.SkipTestWithoutComment, []},
+          {Credo.Check.Design.DuplicatedCode, []},
 
           #
           ## Readability Checks
           #
           {Credo.Check.Readability.AliasOrder, []},
+          {Credo.Check.Readability.BlockPipe, []},
           {Credo.Check.Readability.FunctionNames, []},
+          {Credo.Check.Readability.ImplTrue, []},
           {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.MultiAlias, []},
+          {Credo.Check.Readability.OneArityFunctionInPipe, []},
           {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.SeparateAliasRequire, []},
+          {Credo.Check.Readability.SingleFunctionToBlockPipe, []},
           {Credo.Check.Readability.SpaceAfterCommas, []},
+          {Credo.Check.Readability.Specs, []},
+          {Credo.Check.Readability.StrictModuleLayout, []},
           {Credo.Check.Readability.StringSigils, []},
           {Credo.Check.Readability.TrailingBlankLine, []},
           {Credo.Check.Readability.TrailingWhiteSpace, []},
           {Credo.Check.Readability.UnnecessaryAliasExpansion, []},
           {Credo.Check.Readability.VariableNames, []},
+          {Credo.Check.Readability.WithCustomTaggedTuple, []},
           {Credo.Check.Readability.WithSingleClause, []},
 
           #
           ## Refactoring Opportunities
           #
+          {Credo.Check.Refactor.ABCSize, []},
+          {Credo.Check.Refactor.AppendSingleItem, []},
           {Credo.Check.Refactor.Apply, []},
           {Credo.Check.Refactor.CondStatements, []},
           {Credo.Check.Refactor.CyclomaticComplexity, []},
+          {Credo.Check.Refactor.DoubleBooleanNegation, []},
           {Credo.Check.Refactor.FilterCount, []},
           {Credo.Check.Refactor.FilterFilter, []},
+          {Credo.Check.Refactor.FilterReject, []},
           {Credo.Check.Refactor.FunctionArity, []},
+          {Credo.Check.Refactor.IoPuts, []},
           {Credo.Check.Refactor.LongQuoteBlocks, []},
           {Credo.Check.Refactor.MapJoin, []},
+          {Credo.Check.Refactor.MapMap, []},
           {Credo.Check.Refactor.MatchInCondition, []},
+          {Credo.Check.Refactor.ModuleDependencies, []},
           {Credo.Check.Refactor.NegatedConditionsInUnless, []},
           {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+          {Credo.Check.Refactor.NegatedIsNil, []},
           {Credo.Check.Refactor.Nesting, []},
+          {Credo.Check.Refactor.PassAsyncInTestCases, []},
+          {Credo.Check.Refactor.PipeChainStart, []},
           {Credo.Check.Refactor.RedundantWithClauseResult, []},
+          {Credo.Check.Refactor.RejectFilter, []},
           {Credo.Check.Refactor.RejectReject, []},
           {Credo.Check.Refactor.UnlessWithElse, []},
+          {Credo.Check.Refactor.UtcNowTruncate, []},
+          {Credo.Check.Refactor.VariableRebinding, []},
           {Credo.Check.Refactor.WithClauses, []},
 
           #
@@ -148,12 +163,16 @@
           {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
           {Credo.Check.Warning.IExPry, []},
           {Credo.Check.Warning.IoInspect, []},
+          {Credo.Check.Warning.LeakyEnvironment, []},
+          {Credo.Check.Warning.MapGetUnsafePass, []},
           {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, []},
+          {Credo.Check.Warning.MixEnv, []},
           {Credo.Check.Warning.OperationOnSameValues, []},
           {Credo.Check.Warning.OperationWithConstantResult, []},
           {Credo.Check.Warning.RaiseInsideRescue, []},
           {Credo.Check.Warning.SpecWithStruct, []},
           {Credo.Check.Warning.UnsafeExec, []},
+          {Credo.Check.Warning.UnsafeToAtom, []},
           {Credo.Check.Warning.UnusedEnumOperation, []},
           {Credo.Check.Warning.UnusedFileOperation, []},
           {Credo.Check.Warning.UnusedKeywordOperation, []},
@@ -165,54 +184,14 @@
           {Credo.Check.Warning.WrongTestFileExtension, []}
         ],
         disabled: [
-          #
-          # Checks scheduled for next check update (opt-in for now)
-          {Credo.Check.Refactor.UtcNowTruncate, []},
-
-          #
-          # 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.OneArityFunctionInPipe, []},
           {Credo.Check.Readability.OnePipePerLine, []},
-          {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.PassAsyncInTestCases, []},
-          {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, []},
+          {Credo.Check.Warning.LazyLogging, []}
 
-          #
           # Custom checks can be created using `mix credo.gen.check`.
-          #
         ]
       }
     }

lib/flick/application.ex

@@ -1,11 +1,12 @@
+# credo:disable-for-this-file Credo.Check.Refactor.ModuleDependencies
 defmodule Flick.Application do
   # See https://hexdocs.pm/elixir/Application.html
   # for more information on OTP Applications
   @moduledoc false
 
   use Application
 
-  @impl true
+  @impl Application
   def start(_type, _args) do
     Appsignal.Phoenix.LiveView.attach()
 
@@ -30,7 +31,7 @@ defmodule Flick.Application do
 
   # Tell Phoenix to update the endpoint configuration
   # whenever the application is updated.
-  @impl true
+  @impl Application
   def config_change(changed, _new, removed) do
     FlickWeb.Endpoint.config_change(changed, removed)
     :ok

lib/flick/ranked_voting.ex

@@ -28,17 +28,17 @@ defmodule Flick.RankedVoting do
           {:ok, Ballot.t()}
           | {:error, Ecto.Changeset.t(Ballot.t())}
           | {:error, :can_not_update_published_ballot}
-  def update_ballot(%Ballot{published_at: published_at}, _attrs)
-      when not is_nil(published_at) do
-    {:error, :can_not_update_published_ballot}
-  end
 
   def update_ballot(%Ballot{published_at: nil} = ballot, attrs) do
     ballot
     |> change_ballot(attrs)
     |> Repo.update()
   end
 
+  def update_ballot(_ballot, _attrs) do
+    {:error, :can_not_update_published_ballot}
+  end
+
   @doc """
   Returns a boolean value indicating whether the given
   `Flick.RankedVoting.Ballot` entity can be updated.
@@ -59,17 +59,16 @@ defmodule Flick.RankedVoting do
           | {:error, :ballot_already_published}
   def publish_ballot(ballot, published_at \\ DateTime.utc_now())
 
-  def publish_ballot(%Ballot{published_at: published_at}, _published_at)
-      when not is_nil(published_at) do
-    {:error, :ballot_already_published}
-  end
-
-  def publish_ballot(%Ballot{} = ballot, published_at) do
+  def publish_ballot(%Ballot{published_at: nil} = ballot, published_at) do
     ballot
     |> change_ballot(%{published_at: published_at})
     |> Repo.update()
   end
 
+  def publish_ballot(_ballot, _published_at) do
+    {:error, :ballot_already_published}
+  end
+
   @doc """
   Returns a list of all `Flick.RankedVoting.Ballot` entities.
   """
@@ -172,7 +171,7 @@ defmodule Flick.RankedVoting do
   * `:action` - An optional atom applied to the changeset, useful for forms that
     look to a changeset's action to influence form behavior.
   """
-  @spec change_vote(Vote.t() | Vote.struct_t(), map()) :: Ecto.Changeset.t(Vote.t())
+  @spec change_vote(Vote.t() | Vote.struct_t(), map(), keyword()) :: Ecto.Changeset.t(Vote.t())
   def change_vote(%Vote{} = vote, attrs, opts \\ []) do
     opts = Keyword.validate!(opts, action: nil)
 

lib/flick/ranked_voting/ballot.ex

@@ -70,6 +70,7 @@ defmodule Flick.RankedVoting.Ballot do
     |> Enum.map(&String.trim/1)
   end
 
+  @spec validate_published_at(Ecto.Changeset.t()) :: Ecto.Changeset.t()
   def validate_published_at(%Ecto.Changeset{data: %__MODULE__{id: nil}} = changeset) do
     # We do not want "new" ballots to be created as already published.
     validate_change(changeset, :published_at, fn :published_at, _updated_value ->

lib/flick/ranked_voting/vote.ex

@@ -105,15 +105,17 @@ defmodule Flick.RankedVoting.Vote do
     ballot = Flick.RankedVoting.get_ballot!(get_field(changeset, :ballot_id))
     possible_answers = Ballot.possible_answers_as_list(ballot.possible_answers) ++ ["", nil]
 
-    Enum.reduce(new_ranked_answers, [], fn changeset, acc ->
+    new_ranked_answers
+    |> Enum.reduce([], fn changeset, acc ->
       ranked_answer_value = get_field(changeset, :value)
 
       if Enum.member?(possible_answers, ranked_answer_value) do
         acc
       else
-        acc ++ [ranked_answer_value]
+        [ranked_answer_value] ++ acc
       end
     end)
+    |> Enum.reverse()
   end
 
   defp validate_ranked_answers_are_not_duplicated(%Changeset{valid?: false} = changeset) do
@@ -163,6 +165,8 @@ defmodule Flick.RankedVoting.Vote do
     changeset
   end
 
+  # FIXME: Refactor to reduce complexity.
+  # credo:disable-for-next-line Credo.Check.Refactor.ABCSize
   defp validate_first_ranked_answers_has_valid_value(%Changeset{changes: changes} = changeset)
        when is_map_key(changes, :ranked_answers) do
     ranked_answer_changesets =

lib/flick/release.ex

@@ -11,6 +11,7 @@ defmodule Flick.Release do
   """
   @app :flick
 
+  @spec migrate() :: [any()]
   def migrate do
     ensure_started()
 
@@ -19,6 +20,7 @@ defmodule Flick.Release do
     end
   end
 
+  @spec rollback(Ecto.Repo.t(), integer()) :: {:ok, any(), list()}
   def rollback(repo, version) do
     ensure_started()
     {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version))

lib/flick_web.ex

@@ -1,3 +1,4 @@
+# credo:disable-for-this-file Credo.Check.Refactor.ModuleDependencies
 defmodule FlickWeb do
   @moduledoc """
   The entrypoint for defining your web interface, such
@@ -17,8 +18,10 @@ defmodule FlickWeb do
   those modules here.
   """
 
+  @spec static_paths() :: [String.t()]
   def static_paths, do: ~w(assets fonts images favicon.ico robots.txt)
 
+  @spec router() :: Macro.t()
   def router do
     quote do
       use Phoenix.Router, helpers: false
@@ -30,12 +33,14 @@ defmodule FlickWeb do
     end
   end
 
+  @spec channel() :: Macro.t()
   def channel do
     quote do
       use Phoenix.Channel
     end
   end
 
+  @spec controller() :: Macro.t()
   def controller do
     quote do
       use Phoenix.Controller,
@@ -49,6 +54,7 @@ defmodule FlickWeb do
     end
   end
 
+  @spec live_view() :: Macro.t()
   def live_view do
     quote do
       use Phoenix.LiveView,
@@ -60,6 +66,7 @@ defmodule FlickWeb do
     end
   end
 
+  @spec live_component() :: Macro.t()
   def live_component do
     quote do
       use Phoenix.LiveComponent
@@ -68,6 +75,7 @@ defmodule FlickWeb do
     end
   end
 
+  @spec html() :: Macro.t()
   def html do
     quote do
       use Phoenix.Component
@@ -97,6 +105,7 @@ defmodule FlickWeb do
     end
   end
 
+  @spec verified_routes() :: Macro.t()
   def verified_routes do
     quote do
       use Phoenix.VerifiedRoutes,

lib/flick_web/components/core_components.ex

@@ -1,3 +1,4 @@
+# credo:disable-for-this-file Credo.Check.Readability.Specs
 defmodule FlickWeb.CoreComponents do
   @moduledoc """
   Provides core UI components.
@@ -16,8 +17,8 @@ defmodule FlickWeb.CoreComponents do
   """
   use Phoenix.Component
 
-  alias Phoenix.LiveView.JS
   import FlickWeb.Gettext
+  alias Phoenix.LiveView.JS
 
   @doc """
   Renders a modal.

lib/flick_web/controllers/error_html.ex

@@ -13,6 +13,7 @@ defmodule FlickWeb.ErrorHTML do
   # The default is to render a plain text page based on
   # the template name. For example, "404.html" becomes
   # "Not Found".
+  @spec render(String.t(), map()) :: String.t()
   def render(template, _assigns) do
     Phoenix.Controller.status_message_from_template(template)
   end