Add Credo (#56)

.credo.exs

@@ -0,0 +1,199 @@
+# 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/",
+          # exclude exercise analysis because of snippets in `feature do form do...`
+          # they can be unusual, e.g. `_ignore || _ignore`
+          "lib/elixir_analyzer/test_suite/",
+          # exclude exercise analysis test because of example solutions used in tests
+          # those mimic student solutions and might be "bad code" on purpose
+          "test/elixir_analyzer/test_suite/"
+        ]
+      },
+      #
+      # 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: [
+        #
+        ## 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]},
+        # 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, []},
+
+        #
+        ## 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.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, []},
+
+        #
+        ## Refactoring Opportunities
+        #
+        {Credo.Check.Refactor.CondStatements, []},
+        {Credo.Check.Refactor.CyclomaticComplexity, []},
+        {Credo.Check.Refactor.FunctionArity, []},
+        {Credo.Check.Refactor.LongQuoteBlocks, []},
+        # {Credo.Check.Refactor.MapInto, []},
+        {Credo.Check.Refactor.MatchInCondition, []},
+        {Credo.Check.Refactor.NegatedConditionsInUnless, []},
+        {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+        {Credo.Check.Refactor.Nesting, []},
+        {Credo.Check.Refactor.UnlessWithElse, []},
+        {Credo.Check.Refactor.WithClauses, []},
+
+        #
+        ## Warnings
+        #
+        {Credo.Check.Warning.ApplicationConfigInModuleAttribute, []},
+        {Credo.Check.Warning.BoolOperationOnSameValues, []},
+        {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+        {Credo.Check.Warning.IExPry, []},
+        {Credo.Check.Warning.IoInspect, []},
+        # {Credo.Check.Warning.LazyLogging, []},
+        {Credo.Check.Warning.MixEnv, false},
+        {Credo.Check.Warning.OperationOnSameValues, []},
+        {Credo.Check.Warning.OperationWithConstantResult, []},
+        {Credo.Check.Warning.RaiseInsideRescue, []},
+        {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, []},
+
+        #
+        # Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`)
+
+        #
+        # Controversial and experimental checks (opt-in, just replace `false` with `[]`)
+        #
+        {Credo.Check.Consistency.MultiAliasImportRequireUse, false},
+        {Credo.Check.Consistency.UnusedVariableNames, false},
+        {Credo.Check.Design.DuplicatedCode, false},
+        {Credo.Check.Readability.AliasAs, false},
+        {Credo.Check.Readability.BlockPipe, false},
+        {Credo.Check.Readability.ImplTrue, false},
+        {Credo.Check.Readability.MultiAlias, false},
+        {Credo.Check.Readability.SeparateAliasRequire, false},
+        {Credo.Check.Readability.SinglePipe, false},
+        {Credo.Check.Readability.Specs, false},
+        {Credo.Check.Readability.StrictModuleLayout, false},
+        {Credo.Check.Readability.WithCustomTaggedTuple, false},
+        {Credo.Check.Refactor.ABCSize, false},
+        {Credo.Check.Refactor.AppendSingleItem, false},
+        {Credo.Check.Refactor.DoubleBooleanNegation, false},
+        {Credo.Check.Refactor.ModuleDependencies, false},
+        {Credo.Check.Refactor.NegatedIsNil, false},
+        {Credo.Check.Refactor.PipeChainStart, false},
+        {Credo.Check.Refactor.VariableRebinding, false},
+        {Credo.Check.Warning.LeakyEnvironment, false},
+        {Credo.Check.Warning.MapGetUnsafePass, false},
+        {Credo.Check.Warning.UnsafeToAtom, false}
+
+        #
+        # Custom checks can be created using `mix credo.gen.check`.
+        #
+      ]
+    }
+  ]
+}

.github/workflows/elixir_test.yml

@@ -18,3 +18,5 @@ jobs:
           mix deps.get
       - name: Run Tests
         run: mix test --exclude external
+      - name: Run Credo
+        run: mix credo

.tool-versions

@@ -0,0 +1,2 @@
+elixir 1.11.3-otp-23
+erlang 23.2.6

lib/elixir_analyzer.ex

@@ -12,7 +12,7 @@ defmodule ElixirAnalyzer do
   import ElixirAnalyzer.Summary, only: [summary: 2]
 
   # defaults
-  @exercise_config Application.get_env(:elixir_analyzer, :exercise_config)
+  @exercise_config Application.compile_env(:elixir_analyzer, :exercise_config)
   @output_file "analysis.json"
   @meta_config ".meta/config.json"
 
@@ -179,12 +179,12 @@ defmodule ElixirAnalyzer do
   # - check if the file exists
   # - read in the code
   # - compile
-  defp check(submission = %Submission{halted: true}, _params) do
+  defp check(%Submission{halted: true} = submission, _params) do
     Logger.warning("Check not performed, halted previously")
     submission
   end
 
-  defp check(submission = %Submission{}, _params) do
+  defp check(%Submission{} = submission, _params) do
     with path_to_code <- Path.join(submission.code_path, submission.code_file),
          :ok <- Logger.info("Attempting to read code file", code_file_path: path_to_code),
          {:code_read, {:ok, code_str}} <- {:code_read, File.read(path_to_code)},
@@ -213,12 +213,12 @@ defmodule ElixirAnalyzer do
 
   # Analyze
   # - Start the static analysis
-  defp analyze(submission = %Submission{halted: true}, _params) do
+  defp analyze(%Submission{halted: true} = submission, _params) do
     Logger.info("Analysis not performed, halted previously")
     submission
   end
 
-  defp analyze(submission = %Submission{}, _params) do
+  defp analyze(%Submission{} = submission, _params) do
     Logger.info("Analyzing code started")
 
     submission =
@@ -230,7 +230,7 @@ defmodule ElixirAnalyzer do
     submission
   end
 
-  defp write_results(submission = %Submission{}, params) do
+  defp write_results(%Submission{} = submission, params) do
     if params.write_results do
       output_file_path = Path.join(params.output_path, params.output_file)
       Logger.info("Writing final results.json to file", path: output_file_path)

lib/elixir_analyzer/cli.ex

@@ -1,4 +1,8 @@
 defmodule ElixirAnalyzer.CLI do
+  @moduledoc """
+  A CLI for running analysis on a single solution.
+  """
+
   @usage """
   Usage:
 

lib/elixir_analyzer/constants.ex

@@ -1,4 +1,14 @@
 defmodule ElixirAnalyzer.Constants do
+  @moduledoc """
+  A list of Elixir analyzer comments, in the format:
+  ```
+  elixir.[directory].[filename]
+  ```
+
+  `[directory]` must correspond to a directory in https://github.com/exercism/website-copy/tree/main/analyzer-comments/elixir
+  and `[filename].md` must be a file in that directory.
+  """
+
   @constants [
     # Status Comments
     # status_approve: "elixir.status.approve",

lib/elixir_analyzer/exercise_test.ex

@@ -23,6 +23,7 @@ defmodule ElixirAnalyzer.ExerciseTest do
   #
 
   defmacro __before_compile__(env) do
+    # credo:disable-for-previous-line Credo.Check.Refactor.CyclomaticComplexity
     feature_test_data = Macro.escape(Module.get_attribute(env.module, :feature_tests))
     assert_call_data = Module.get_attribute(env.module, :assert_call_tests)
 
@@ -37,7 +38,7 @@ defmodule ElixirAnalyzer.ExerciseTest do
 
     quote do
       @spec analyze(Submission.t(), String.t()) :: Submission.t()
-      def analyze(submission = %Submission{}, code_as_string) do
+      def analyze(%Submission{} = submission, code_as_string) do
         case Code.string_to_quoted(code_as_string) do
           {:ok, code_ast} ->
             feature_results = unquote(feature_tests) |> filter_suppressed_results()
@@ -56,21 +57,25 @@ defmodule ElixirAnalyzer.ExerciseTest do
         feature_results
         |> Enum.reject(fn
           {_test_result, %{suppress_if: condition}} when condition !== false ->
-            [suppress_on_test_name, suppress_on_result] = condition
-
-            Enum.any?(feature_results, fn {result, test} ->
-              case {result, test.name} do
-                {^suppress_on_result, ^suppress_on_test_name} -> true
-                _ -> false
-              end
-            end)
+            any_result_matches_suppress_condition?(feature_results, condition)
 
           _result ->
             false
         end)
       end
 
-      defp append_test_comments(submission = %Submission{}, results) do
+      defp any_result_matches_suppress_condition?(feature_results, condition) do
+        [suppress_on_test_name, suppress_on_result] = condition
+
+        Enum.any?(feature_results, fn {result, test} ->
+          case {result, test.name} do
+            {^suppress_on_result, ^suppress_on_test_name} -> true
+            _ -> false
+          end
+        end)
+      end
+
+      defp append_test_comments(%Submission{} = submission, results) do
         Enum.reduce(results, submission, fn
           {:skip, _description}, submission ->
             submission
@@ -94,7 +99,7 @@ defmodule ElixirAnalyzer.ExerciseTest do
         end)
       end
 
-      defp append_analysis_failure(submission = %Submission{}, {location, error, token}) do
+      defp append_analysis_failure(%Submission{} = submission, {location, error, token}) do
         line =
           case location do
             location when is_integer(location) -> location

lib/elixir_analyzer/exercise_test/feature.ex

@@ -1,4 +1,9 @@
 defmodule ElixirAnalyzer.ExerciseTest.Feature do
+  @moduledoc """
+  Defines a `feature` macro that allows looking for specific snippets
+  whose AST matches part of the AST of the solution.
+  """
+
   @doc false
   defmacro __using__(_opts) do
     quote do
@@ -54,13 +59,13 @@ defmodule ElixirAnalyzer.ExerciseTest.Feature do
 
   defp gather_feature_data({:form, _, [[do: form]]} = node, acc) do
     ast =
-      unless acc.meta.keep_meta do
+      if acc.meta.keep_meta do
+        form
+      else
         Macro.prewalk(form, fn
           {name, _, param} -> {name, :_ignore, param}
           node -> node
         end)
-      else
-        form
       end
 
     {ast, block_params} =