Allow reading multiple file solutions and exemploids (#298)

* General updates

* Read multiple files

* Removed suppressed compilation warning

* Adapt tests to match lib changes

* Add smoke tests with multiple files

* Remove extra space

* Add newline

* Apply suggestions from code review

Co-authored-by: Angelika Tyborska <[email protected]>

* Replace ls_r by Path.wildcard

* Fix error message

* Renaming (_path to _files)

* Start iterator with empty string

* Revert "Start iterator with empty string" not to mess with line numbers

This reverts commit b0b04ce.

Co-authored-by: Angelika Tyborska <[email protected]>

Author: jiegillet

elixir

@@ -79,7 +79,7 @@ defmodule ElixirAnalyzer do
 
     defaults = [
       {:exercise, exercise},
-      {:path, input_path},
+      {:path, String.trim_leading(input_path, "./")},
       {:output_path, output_path},
       {:output_file, @output_file},
       {:exercise_config, default_exercise_config()},
@@ -107,14 +107,14 @@ defmodule ElixirAnalyzer do
       Logger.debug("Getting the exercise config")
       exercise_config = params.exercise_config[params.exercise]
 
-      {code_path, exercise_type, exemploid_path, analysis_module} =
+      {submitted_files, exercise_type, exemploid_files, analysis_module} =
         do_init(params, exercise_config)
 
       Logger.debug("Initialization successful",
         path: params.path,
-        code_path: code_path,
+        submitted_files: submitted_files,
         exercise_type: exercise_type,
-        exemploid_path: exemploid_path,
+        exemploid_files: exemploid_files,
         analysis_module: analysis_module
       )
 
@@ -129,9 +129,9 @@ defmodule ElixirAnalyzer do
 
       source = %{
         source
-        | code_path: code_path,
+        | submitted_files: submitted_files,
           exercise_type: exercise_type,
-          exemploid_path: exemploid_path
+          exemploid_files: exemploid_files
       }
 
       %{
@@ -165,16 +165,28 @@ defmodule ElixirAnalyzer do
 
   defp do_init(params, exercise_config) do
     meta_config = Path.join(params.path, @meta_config) |> File.read!() |> Jason.decode!()
-    relative_code_path = meta_config["files"]["solution"] |> hd()
-    code_path = Path.join(params.path, relative_code_path)
+    solution_files = meta_config["files"]["solution"] |> Enum.map(&Path.join(params.path, &1))
+    if Enum.empty?(solution_files), do: raise("No solution files specified")
 
-    {exercise_type, exemploid_path} =
+    submitted_files =
+      Path.join([params.path, "lib", "**", "*.ex"])
+      |> Path.wildcard()
+      |> Enum.concat(solution_files)
+      |> Enum.uniq()
+      |> Enum.sort()
+
+    editor_files = Map.get(meta_config["files"], "editor", [])
+
+    {exercise_type, exemploid_files} =
       case meta_config["files"] do
-        %{"exemplar" => [path | _]} -> {:concept, Path.join(params.path, path)}
-        %{"example" => [path | _]} -> {:practice, Path.join(params.path, path)}
+        %{"exemplar" => path} -> {:concept, path}
+        %{"example" => path} -> {:practice, path}
       end
 
-    {code_path, exercise_type, exemploid_path,
+    exemploid_files =
+      (editor_files ++ exemploid_files) |> Enum.sort() |> Enum.map(&Path.join(params.path, &1))
+
+    {submitted_files, exercise_type, exemploid_files,
      exercise_config[:analyzer_module] || ElixirAnalyzer.TestSuite.Default}
   end
 
@@ -190,15 +202,15 @@ defmodule ElixirAnalyzer do
   end
 
   defp check(%Submission{source: source} = submission, _params) do
-    Logger.info("Attempting to read code file", code_file_path: source.code_path)
+    Logger.info("Attempting to read code files", code_file_path: source.submitted_files)
 
-    with {:code_read, {:ok, code_string}} <- {:code_read, File.read(source.code_path)},
+    with {:code_read, {:ok, code_string}} <- {:code_read, read_files(source.submitted_files)},
          source <- %{source | code_string: code_string},
-         Logger.info("Code file read successfully"),
-         Logger.info("Attempting to read exemploid", exemploid_path: source.exemploid_path),
+         Logger.info("Code files read successfully"),
+         Logger.info("Attempting to read exemploid", exemploid_files: source.exemploid_files),
          {:exemploid_read, _, {:ok, exemploid_string}} <-
-           {:exemploid_read, source, File.read(source.exemploid_path)},
-         Logger.info("Exemploid file read successfully, attempting to parse"),
+           {:exemploid_read, source, read_files(source.exemploid_files)},
+         Logger.info("Exemploid files read successfully, attempting to parse"),
          {:exemploid_ast, _, {:ok, exemploid_ast}} <-
            {:exemploid_ast, source, Code.string_to_quoted(exemploid_string)} do
       Logger.info("Exemploid file parsed successfully")
@@ -208,36 +220,50 @@ defmodule ElixirAnalyzer do
       {:code_read, {:error, reason}} ->
         Logger.warning("TestSuite halted: Code file not found. Reason: #{reason}",
           path: source.path,
-          code_path: source.code_path
+          submitted_files: source.submitted_files
         )
 
         submission
         |> Submission.halt()
         |> Submission.append_comment(%Comment{
           comment: Constants.general_file_not_found(),
           params: %{
-            "file_name" => Path.basename(source.code_path),
+            "file_name" => Path.basename(source.submitted_files),
             "path" => source.path
           },
           type: :essential
         })
 
       {:exemploid_read, source, {:error, reason}} ->
         Logger.warning("Exemploid file not found. Reason: #{reason}",
-          exemploid_path: source.exemploid_path
+          exemploid_files: source.exemploid_files
         )
 
         %{submission | source: source}
 
       {:exemploid_ast, source, {:error, reason}} ->
         Logger.warning("Exemploid file could not be parsed. Reason: #{inspect(reason)}",
-          exemploid_path: source.exemploid_path
+          exemploid_files: source.exemploid_files
         )
 
         %{submission | source: source}
     end
   end
 
+  defp read_files(paths) do
+    Enum.reduce_while(
+      paths,
+      {:ok, nil},
+      fn path, {:ok, code} ->
+        case File.read(path) do
+          {:ok, file} when is_nil(code) -> {:cont, {:ok, file}}
+          {:ok, file} -> {:cont, {:ok, code <> "\n" <> file}}
+          {:error, err} -> {:halt, {:error, err}}
+        end
+      end
+    )
+  end
+
   # Analyze
   # - Start the static analysis
   defp analyze(%Submission{halted: true} = submission, _params) do

lib/elixir_analyzer.ex

@@ -35,7 +35,7 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do
 
   @spec run(Source.t()) :: [{:pass | :fail, Comment.t()}]
   def run(%Source{
-        code_path: code_path,
+        submitted_files: submitted_files,
         code_ast: code_ast,
         code_string: code_string,
         exercise_type: type,
@@ -46,7 +46,7 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do
       VariableNames.run(code_ast),
       ModuleAttributeNames.run(code_ast),
       ModulePascalCase.run(code_ast),
-      CompilerWarnings.run(code_path, code_ast),
+      CompilerWarnings.run(submitted_files),
       BooleanFunctions.run(code_ast),
       FunctionAnnotationOrder.run(code_ast),
       ExemplarComparison.run(code_ast, type, exemploid_ast),

lib/elixir_analyzer/exercise_test/common_checks.ex

@@ -5,28 +5,30 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.CompilerWarnings do
   alias ElixirAnalyzer.Constants
   alias ElixirAnalyzer.Comment
 
-  def run(code_path, _code_ast) do
-    import ExUnit.CaptureIO
-    Application.put_env(:elixir, :ansi_enabled, false)
+  def run(code_path) do
+    Logger.configure(level: :critical)
 
     warnings =
-      capture_io(:stderr, fn ->
-        try do
-          Code.compile_file(code_path)
-          |> Enum.each(fn {module, _binary} ->
+      case Kernel.ParallelCompiler.compile(code_path) do
+        {:ok, modules, warnings} ->
+          Enum.each(modules, fn module ->
             :code.delete(module)
             :code.purge(module)
           end)
-        rescue
-          # There are too many compile errors for tests, so we filter them out
-          # We assume that real code passed the tests and therefore compiles
-          _ -> nil
-        end
-      end)
+
+          warnings
+
+        {:error, _errors, _warnings} ->
+          # This should not happen, as real code is assumed to have compiled and
+          # passed the tests
+          []
+      end
+
+    Logger.configure(level: :warn)
 
     Application.put_env(:elixir, :ansi_enabled, true)
 
-    if warnings == "" do
+    if Enum.empty?(warnings) do
       []
     else
       [
@@ -35,9 +37,20 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.CompilerWarnings do
            type: :actionable,
            name: Constants.solution_compiler_warnings(),
            comment: Constants.solution_compiler_warnings(),
-           params: %{warnings: warnings}
+           params: %{warnings: Enum.map_join(warnings, &format_warning/1)}
          }}
       ]
     end
   end
+
+  defp format_warning({filepath, line, warning}) do
+    [_ | after_lib] = String.split(filepath, "/lib/")
+    filepath = "lib/" <> Enum.join(after_lib)
+
+    """
+    warning: #{warning}
+      #{filepath}:#{line}
+
+    """
+  end
 end

lib/elixir_analyzer/exercise_test/common_checks/compiler_warnings.ex

@@ -5,23 +5,23 @@ defmodule ElixirAnalyzer.Source do
   defstruct [
     :slug,
     :path,
-    :code_path,
+    :submitted_files,
     :code_string,
     :code_ast,
     :exercise_type,
-    :exemploid_path,
+    :exemploid_files,
     :exemploid_string,
     :exemploid_ast
   ]
 
   @type t() :: %__MODULE__{
           slug: String.t(),
           path: String.t(),
-          code_path: String.t(),
+          submitted_files: [String.t()],
           code_string: String.t(),
           code_ast: Macro.t(),
           exercise_type: :concept | :practice,
-          exemploid_path: String.t(),
+          exemploid_files: [String.t()],
           exemploid_string: String.t(),
           exemploid_ast: Macro.t()
         }

lib/elixir_analyzer/source.ex

@@ -6,9 +6,7 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberDeluxe do
   alias ElixirAnalyzer.Constants
   alias ElixirAnalyzer.Source
 
-  use ElixirAnalyzer.ExerciseTest,
-    # this is temporary until we include editor files in compilation
-    suppress_tests: [Constants.solution_compiler_warnings()]
+  use ElixirAnalyzer.ExerciseTest
 
   feature "uses GenServer" do
     type :actionable

lib/elixir_analyzer/test_suite/take_a_number_deluxe.ex

@@ -29,10 +29,10 @@ defmodule ElixirAnalyzer.CLITest do
          source: %Source{
            code_ast: {:defmodule, _, _},
            code_string: "defmodule Lasagna" <> _,
-           code_path: @lasagna_path <> "/lib/lasagna.ex",
+           submitted_files: [@lasagna_path <> "/lib/lasagna.ex"],
            exemploid_ast: {:defmodule, _, _},
            exemploid_string: "defmodule Lasagna" <> _,
-           exemploid_path: @lasagna_path <> "/.meta/exemplar.ex",
+           exemploid_files: [@lasagna_path <> "/.meta/exemplar.ex"],
            exercise_type: :concept,
            path: @lasagna_path,
            slug: "lasagna"

test/elixir_analyzer/cli_test.exs

@@ -3,7 +3,7 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.CompilerWarningsTest do
   alias ElixirAnalyzer.ExerciseTest.CommonChecks.CompilerWarnings
 
   test "Implementing a protocol doesn't trigger a compiler warning" do
-    filepath = "test_data/clock/lib/clock.ex"
-    assert CompilerWarnings.run(filepath, nil) == []
+    filepath = "test_data/clock/perfect_solution/lib/clock.ex"
+    assert CompilerWarnings.run([filepath]) == []
   end
 end