Run common checks for every single solution; Add common check for sna…

…ke_case (#102)

Co-authored-by: Tim Austin <[email protected]>

README.md

@@ -57,7 +57,7 @@ As a demonstration, once iex loads with `iex -S mix` you can type `ElixirAnalyze
 ### `ElixirAnalyzer`
 
 - The is the main application module. A function call to `start_analyze/3` begins the analysis (either through IEX or the CLI escript [if generated]).
-- A configuation in `config/config.exs` holds data for each exercise supported
+- A configuration in `config/config.exs` holds data for each exercise supported
 
 ```elixir
 config :elixir_analyzer,
@@ -110,7 +110,7 @@ Contains macro to generate a function returning the comment path on the `exercis
 
 ### `ElixirAnalyzer.CLI`
 
-This module is a module for the CLI escript to parse the command line arguements and start the processing
+This module is a module for the CLI escript to parse the command line arguments and start the processing
 
 ### `ElixirAnalyzer.ExerciseTest.________`
 

lib/elixir_analyzer.ex

@@ -134,15 +134,6 @@ defmodule ElixirAnalyzer do
           analysis_module: analysis_module
       }
     rescue
-      e in ArgumentError ->
-        Logger.warning("TestSuite halted, ArgumentError", error_message: e.message)
-
-        submission
-        |> Submission.halt()
-        |> Submission.set_halt_reason(
-          "Analysis skipped, no analysis suite exists for this exercise"
-        )
-
       e in File.Error ->
         Logger.warning("Unable to decode 'config.json'", error_message: e.message)
 
@@ -156,6 +147,13 @@ defmodule ElixirAnalyzer do
         submission
         |> Submission.halt()
         |> Submission.set_halt_reason("Analysis skipped, not able to decode solution config.")
+
+      e ->
+        Logger.warning("TestSuite halted, #{e.__struct__}", error_message: e.message)
+
+        submission
+        |> Submission.halt()
+        |> Submission.set_halt_reason("Analysis skipped, unexpected error #{e.__struct__}")
     end
   end
 
@@ -168,7 +166,7 @@ defmodule ElixirAnalyzer do
     code_path = Path.dirname(full_code_path)
     code_file = Path.basename(full_code_path)
 
-    {code_path, code_file, exercise_config[:analyzer_module]}
+    {code_path, code_file, exercise_config[:analyzer_module] || ElixirAnalyzer.TestSuite.Default}
   end
 
   # Else, use passed in params to analyze

lib/elixir_analyzer/comment.ex

@@ -11,6 +11,7 @@ defmodule ElixirAnalyzer.Comment do
           comment: String.t(),
           type: :essential | :actionable | :informative | :celebratory,
           status: :skip | :test,
-          suppress_if: false | {String.t(), :pass | :fail}
+          suppress_if: false | {String.t(), :pass | :fail},
+          params: map()
         }
 end

lib/elixir_analyzer/constants.ex

@@ -18,6 +18,8 @@ defmodule ElixirAnalyzer.Constants do
     solution_use_moduledoc: "elixir.solution.use_module_doc",
     solution_use_specification: "elixir.solution.use_specification",
     solution_raise_fn_clause_error: "elixir.solution.raise_fn_clause_error",
+    solution_module_attribute_name_snake_case: "elixir.solution.module_attribute_name_snake_case",
+    solution_function_name_snake_case: "elixir.solution.function_name_snake_case",
 
     # Concept exercises
 

lib/elixir_analyzer/exercise_test.ex

@@ -3,6 +3,7 @@ defmodule ElixirAnalyzer.ExerciseTest do
 
   alias ElixirAnalyzer.ExerciseTest.Feature.Compiler, as: FeatureCompiler
   alias ElixirAnalyzer.ExerciseTest.AssertCall.Compiler, as: AssertCallCompiler
+  alias ElixirAnalyzer.ExerciseTest.CommonChecks
 
   alias ElixirAnalyzer.Submission
   alias ElixirAnalyzer.Constants
@@ -45,10 +46,12 @@ defmodule ElixirAnalyzer.ExerciseTest do
           {:ok, code_ast} ->
             feature_results = unquote(feature_tests) |> filter_suppressed_results()
             assert_call_results = unquote(assert_call_tests) |> filter_suppressed_results()
+            common_checks_results = CommonChecks.run(code_ast, code_as_string)
 
             submission
             |> append_test_comments(feature_results)
             |> append_test_comments(assert_call_results)
+            |> append_test_comments(common_checks_results)
 
           {:error, e} ->
             append_analysis_failure(submission, e)

lib/elixir_analyzer/exercise_test/common_checks.ex

@@ -0,0 +1,18 @@
+defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do
+  @moduledoc """
+  This module aggregates all common checks that should be run on every single solution.
+  """
+
+  alias ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames
+  alias ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames
+  alias ElixirAnalyzer.Comment
+
+  @spec run(Macro.t(), String.t()) :: [{:pass | :fail | :skip, %Comment{}}]
+  def run(code_ast, code_as_string) when is_binary(code_as_string) do
+    [
+      FunctionNames.run(code_ast),
+      ModuleAttributeNames.run(code_ast)
+    ]
+    |> List.flatten()
+  end
+end

lib/elixir_analyzer/exercise_test/common_checks/function_names.ex

@@ -0,0 +1,63 @@
+defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames do
+  @moduledoc """
+  Reports the first function/macro/guard with a name that's not snake_case.
+
+  Doesn't report more if there are more.
+  A single comment should be enough for the student to know what to fix.
+
+  Common check to be run on every single solution.
+  """
+
+  alias ElixirAnalyzer.Constants
+  alias ElixirAnalyzer.Comment
+
+  @def_ops [:def, :defp, :defmacro, :defmacrop, :defguard, :defguardp]
+
+  @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}]
+  def run(ast) do
+    {_, names} = Macro.prewalk(ast, [], &traverse/2)
+    wrong_name = List.last(names)
+
+    if wrong_name do
+      wrong_name = to_string(wrong_name)
+      correct_name = to_snake_case(wrong_name)
+
+      [
+        {:fail,
+         %Comment{
+           type: :actionable,
+           comment: Constants.solution_function_name_snake_case(),
+           params: %{
+             expected: correct_name,
+             actual: wrong_name
+           }
+         }}
+      ]
+    else
+      []
+    end
+  end
+
+  defp traverse({op, _meta, [{name, _meta2, _arguments} | _]} = ast, names) when op in @def_ops do
+    if snake_case?(name) do
+      {ast, names}
+    else
+      {ast, [name | names]}
+    end
+  end
+
+  defp traverse(ast, names) do
+    {ast, names}
+  end
+
+  defp snake_case?(name) do
+    # the code had to compile and pass all the tests to get to the analyzer
+    # so we can assume the name is otherwise valid
+    to_snake_case(name) == to_string(name)
+  end
+
+  defp to_snake_case(name) do
+    # Macro.underscore is good enough because a module attribute name must be a valid Elixir identifier anyway
+    Macro.underscore(to_string(name))
+  end
+end

lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex

@@ -0,0 +1,61 @@
+defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames do
+  @moduledoc """
+  Reports the first module attribute with a name that's not snake_case.
+
+  Doesn't report more if there are more.
+  A single comment should be enough for the student to know what to fix.
+
+  Common check to be run on every single solution.
+  """
+
+  alias ElixirAnalyzer.Constants
+  alias ElixirAnalyzer.Comment
+
+  @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}]
+  def run(ast) do
+    {_, names} = Macro.prewalk(ast, [], &traverse/2)
+    wrong_name = List.last(names)
+
+    if wrong_name do
+      wrong_name = to_string(wrong_name)
+      correct_name = to_snake_case(wrong_name)
+
+      [
+        {:fail,
+         %Comment{
+           type: :actionable,
+           comment: Constants.solution_module_attribute_name_snake_case(),
+           params: %{
+             expected: correct_name,
+             actual: wrong_name
+           }
+         }}
+      ]
+    else
+      []
+    end
+  end
+
+  defp traverse({:@, _meta, [{name, _meta2, _arguments}]} = ast, names) do
+    if snake_case?(name) do
+      {ast, names}
+    else
+      {ast, [name | names]}
+    end
+  end
+
+  defp traverse(ast, names) do
+    {ast, names}
+  end
+
+  defp snake_case?(name) do
+    # the code had to compile and pass all the tests to get to the analyzer
+    # so we can assume the name is otherwise valid
+    to_snake_case(name) == to_string(name)
+  end
+
+  defp to_snake_case(name) do
+    # Macro.underscore is good enough because a module attribute name must be a valid Elixir identifier anyway
+    Macro.underscore(to_string(name))
+  end
+end

lib/elixir_analyzer/test_suite/default.ex

@@ -0,0 +1,10 @@
+defmodule ElixirAnalyzer.TestSuite.Default do
+  @moduledoc """
+  This is the default exercise analyzer extension module.
+
+  It will be run for any exercise submission that doesn't have its own extension module.
+  It's intentionally empty, which means it will only run the common checks.
+  """
+
+  use ElixirAnalyzer.ExerciseTest
+end