diff --git a/elixir b/elixir index fa09022a..4e0853da 160000 --- a/elixir +++ b/elixir @@ -1 +1 @@ -Subproject commit fa09022a7d65b291f03dec3ce32cb6a7d4def35c +Subproject commit 4e0853da74ecec55d07240acb78d53189f1c133b diff --git a/lib/elixir_analyzer/constants.ex b/lib/elixir_analyzer/constants.ex index 81691f93..cf16ca45 100644 --- a/lib/elixir_analyzer/constants.ex +++ b/lib/elixir_analyzer/constants.ex @@ -28,6 +28,7 @@ defmodule ElixirAnalyzer.Constants do solution_compiler_warnings: "elixir.solution.compiler_warnings", solution_same_as_exemplar: "elixir.solution.same_as_exemplar", solution_list_prepend_head: "elixir.solution.list_prepend_head", + solution_no_integer_literal: "elixir.solution.no_integer_literal", # Concept exercises diff --git a/lib/elixir_analyzer/exercise_test.ex b/lib/elixir_analyzer/exercise_test.ex index 933e21dd..8b3035d6 100644 --- a/lib/elixir_analyzer/exercise_test.ex +++ b/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.CheckSource.Compiler, as: CheckSourceCompiler alias ElixirAnalyzer.ExerciseTest.CommonChecks alias ElixirAnalyzer.Submission @@ -14,6 +15,7 @@ defmodule ElixirAnalyzer.ExerciseTest do quote do use ElixirAnalyzer.ExerciseTest.Feature use ElixirAnalyzer.ExerciseTest.AssertCall + use ElixirAnalyzer.ExerciseTest.CheckSource use ElixirAnalyzer.ExerciseTest.CommonChecks @suppress_tests unquote(opts)[:suppress_tests] @@ -31,10 +33,12 @@ defmodule ElixirAnalyzer.ExerciseTest do # credo:disable-for-previous-line Credo.Check.Refactor.CyclomaticComplexity feature_test_data = Module.get_attribute(env.module, :feature_tests) assert_call_data = Module.get_attribute(env.module, :assert_call_tests) + check_source_data = Module.get_attribute(env.module, :check_source_tests) suppress_tests = Module.get_attribute(env.module, :suppress_tests, []) - # ast placeholder for the submission code ast + # placeholders for submission code code_ast = quote do: code_ast + code_as_string = quote do: code_as_string # compile each feature to a test feature_tests = Enum.map(feature_test_data, &FeatureCompiler.compile(&1, code_ast)) @@ -42,6 +46,10 @@ defmodule ElixirAnalyzer.ExerciseTest do # compile each assert_call to a test assert_call_tests = Enum.map(assert_call_data, &AssertCallCompiler.compile(&1, code_ast)) + # compile each check_source to a test + check_source_tests = + Enum.map(check_source_data, &CheckSourceCompiler.compile(&1, code_as_string)) + quote do @spec analyze(Submission.t(), String.t(), nil | Macro.t()) :: Submission.t() def analyze(%Submission{} = submission, code_as_string, exemplar_ast) do @@ -60,6 +68,7 @@ defmodule ElixirAnalyzer.ExerciseTest do Enum.concat([ unquote(feature_tests), unquote(assert_call_tests), + unquote(check_source_tests), CommonChecks.run(code_ast, code_as_string, exemplar_ast) ]) |> filter_suppressed_results() diff --git a/lib/elixir_analyzer/exercise_test/check_source.ex b/lib/elixir_analyzer/exercise_test/check_source.ex new file mode 100644 index 00000000..34fba406 --- /dev/null +++ b/lib/elixir_analyzer/exercise_test/check_source.ex @@ -0,0 +1,101 @@ +defmodule ElixirAnalyzer.ExerciseTest.CheckSource do + @moduledoc """ + Defines a `check_source` macro that allows checking the source code + """ + + @doc false + defmacro __using__(_opts) do + quote do + import unquote(__MODULE__) + + @check_source_tests [] + end + end + + @doc """ + Defines a macro which runs a boolean function on the source code. + + This macro then collates the block into a map structure resembling: + test_data = %{ + description: description, + type: :actionable, + comment: "message", + suppress_if: {"name of other test", :fail} + } + and an AST for the function + """ + defmacro check_source(description, do: block) do + :ok = validate_check_block(block) + + test_data = + block + |> walk_check_source_block() + |> Map.put(:description, description) + |> Map.put_new(:type, :informative) + + check = test_data.check + + test_data = + test_data + |> Map.delete(:check) + # made into a key-val list for better quoting + |> Map.to_list() + + unless Keyword.has_key?(test_data, :comment) do + raise "Comment must be defined for each check_source test" + end + + quote do + @check_source_tests [ + {unquote(test_data), unquote(Macro.escape(check))} | @check_source_tests + ] + end + end + + @supported_expressions [:comment, :type, :suppress_if, :check] + defp validate_check_block({:__block__, _, args}) do + Enum.each(args, fn {name, _, _} -> + if name not in @supported_expressions do + raise """ + Unsupported expression `#{name}`. + The macro `check_source` supports expressions: #{Enum.join(@supported_expressions, ", ")}. + """ + end + end) + end + + defp walk_check_source_block(block, test_data \\ %{}) do + {_, test_data} = Macro.prewalk(block, test_data, &do_walk_check_source_block/2) + test_data + end + + defp do_walk_check_source_block({:comment, _, [comment]} = node, test_data) do + {node, Map.put(test_data, :comment, comment)} + end + + @supported_types ~w(essential actionable informative celebratory)a + defp do_walk_check_source_block({:type, _, [type]} = node, test_data) do + if type not in @supported_types do + raise """ + Unsupported type `#{type}`. + The macro `check_source` supports the following types: #{Enum.join(@supported_types, ", ")}. + """ + end + + {node, Map.put(test_data, :type, type)} + end + + defp do_walk_check_source_block({:suppress_if, _, [name, condition]} = node, test_data) do + {node, Map.put(test_data, :suppress_if, {name, condition})} + end + + defp do_walk_check_source_block({:check, _, [source, [do: function]]} = node, test_data) do + function = {:fn, [], [{:->, [], [[source], function]}]} + + {node, Map.put(test_data, :check, function)} + end + + defp do_walk_check_source_block(node, test_data) do + {node, test_data} + end +end diff --git a/lib/elixir_analyzer/exercise_test/check_source/compiler.ex b/lib/elixir_analyzer/exercise_test/check_source/compiler.ex new file mode 100644 index 00000000..e9e3d32e --- /dev/null +++ b/lib/elixir_analyzer/exercise_test/check_source/compiler.ex @@ -0,0 +1,30 @@ +defmodule ElixirAnalyzer.ExerciseTest.CheckSource.Compiler do + @moduledoc false + + alias ElixirAnalyzer.Comment + + def compile({check_source_data, check_function}, code_string) do + name = Keyword.fetch!(check_source_data, :description) + comment = Keyword.fetch!(check_source_data, :comment) + type = Keyword.get(check_source_data, :type, :informative) + suppress_if = Keyword.get(check_source_data, :suppress_if, false) + + test_description = + Macro.escape(%Comment{ + name: name, + comment: comment, + type: type, + suppress_if: suppress_if + }) + + quote do + (fn string -> + if unquote(check_function).(string) do + {:pass, unquote(test_description)} + else + {:fail, unquote(test_description)} + end + end).(unquote(code_string)) + end + end +end diff --git a/lib/elixir_analyzer/test_suite/german_sysadmin.ex b/lib/elixir_analyzer/test_suite/german_sysadmin.ex index 06f9cdca..49505b03 100644 --- a/lib/elixir_analyzer/test_suite/german_sysadmin.ex +++ b/lib/elixir_analyzer/test_suite/german_sysadmin.ex @@ -35,4 +35,14 @@ defmodule ElixirAnalyzer.TestSuite.GermanSysadmin do called_fn name: :case comment Constants.german_sysadmin_use_case() end + + check_source "does not use integer literals for code points" do + type :actionable + comment Constants.solution_no_integer_literal() + + check(source) do + integers = ["?ß", "?ä", "?ö", "?ü", "?_", "?a", "?z"] + Enum.all?(integers, &String.contains?(source, &1)) + end + end end diff --git a/test/elixir_analyzer/exercise_test/check_source_test.exs b/test/elixir_analyzer/exercise_test/check_source_test.exs new file mode 100644 index 00000000..1d34b711 --- /dev/null +++ b/test/elixir_analyzer/exercise_test/check_source_test.exs @@ -0,0 +1,124 @@ +defmodule ElixirAnalyzer.ExerciseTest.CheckSourceTest do + use ElixirAnalyzer.ExerciseTestCase, + exercise_test_module: ElixirAnalyzer.Support.AnalyzerVerification.CheckSource + + test_exercise_analysis "empty module", + comments: ["always return false", "didn't use multiline"] do + ~S""" + defmodule CheckSourceVerification do + end + """ + end + + test_exercise_analysis "contains integer literals", + comments: [ + "always return false", + "used integer literal from ?a to ?z", + "didn't use multiline" + ] do + ~S""" + defmodule CheckSourceVerification do + def foo(x) do + case x do + 97 -> "a" + 98 -> "b" + _ -> "z" + end + end + end + """ + end + + test_exercise_analysis "contains integer but false positive", + comments: [ + "always return false", + "used integer literal from ?a to ?z", + "didn't use multiline" + ] do + ~S""" + defmodule CheckSourceVerification do + def best_version(game) do + case game do + "fifa" -> "fifa 98" + "tomb raider" -> "tomb raider II (1997)" + _ -> "goat simulator" + end + end + end + """ + end + + test_exercise_analysis "uses multiline strings", + comments: ["always return false"] do + [ + ~S''' + defmodule CheckSourceVerification do + @moduledoc """ + this module doesn't do much + """ + end + ''', + ~S''' + defmodule CheckSourceVerification do + def foo do + """ + all + you + need + is + love + """ + end + end + ''', + ~S""" + defmodule CheckSourceVerification do + def foo do + ''' + love + is + all + you + need + ''' + end + end + """ + ] + end + + test_exercise_analysis "short module", + comments: ["always return false", "module is too short", "didn't use multiline"] do + ~S""" + defmodule C do + end + """ + end + + test_exercise_analysis "badly formatted modules", + comments: ["always return false", "didn't use multiline", "module is not formatted"] do + [ + ~S""" + defmodule CheckSourceVerification do + def foo(), do: :ok + end + """, + ~S""" + defmodule CheckSourceVerification do + + + + def foo, do: :ok + + end + """, + ~S""" + defmodule CheckSourceVerification do + end + """, + ~S""" + defmodule CheckSourceVerification do end + """ + ] + end +end diff --git a/test/elixir_analyzer/exercise_test/suppress_if_test.exs b/test/elixir_analyzer/exercise_test/suppress_if_test.exs index b6cd4107..05ce4bd6 100644 --- a/test/elixir_analyzer/exercise_test/suppress_if_test.exs +++ b/test/elixir_analyzer/exercise_test/suppress_if_test.exs @@ -6,7 +6,7 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do use ElixirAnalyzer.ExerciseTestCase, exercise_test_module: ElixirAnalyzer.Support.AnalyzerVerification.SuppressIf - test_exercise_analysis "common check is found and suppresses assert/feature 1", + test_exercise_analysis "common check is found and suppresses assert/feature/check 1", comments: [Constants.solution_debug_functions()] do defmodule MyModule do def my_function() do @@ -17,7 +17,11 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do end test_exercise_analysis "assert 1 and feature 1 find foo", - comments: ["feature 1: foo() was called", "assert 1: foo() was called"] do + comments: [ + "feature 1: foo() was called", + "assert 1: foo() was called", + "check source 1: foo() was called" + ] do defmodule MyModule do def my_function() do foo() @@ -25,8 +29,12 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do end end - test_exercise_analysis "assert/feature 1 find foo and suppress assert/feature 2", - comments: ["feature 1: foo() was called", "assert 1: foo() was called"] do + test_exercise_analysis "assert/feature/check 1 find foo and suppress assert/feature/check 2", + comments: [ + "feature 1: foo() was called", + "assert 1: foo() was called", + "check source 1: foo() was called" + ] do defmodule MyModule do def my_function() do foo() @@ -35,8 +43,12 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do end end - test_exercise_analysis "assert/feature 2 find bar", - comments: ["feature 2: bar() was called", "assert 2: bar() was called"] do + test_exercise_analysis "assert/feature/check 2 find bar", + comments: [ + "feature 2: bar() was called", + "assert 2: bar() was called", + "check source 2: bar() was called" + ] do defmodule MyModule do def my_function() do bar() @@ -44,8 +56,12 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do end end - test_exercise_analysis "assert/feature 1 find foo and suppress assert/feature 3", - comments: ["feature 1: foo() was called", "assert 1: foo() was called"] do + test_exercise_analysis "assert/feature/check 1 find foo and suppress assert/feature/check 3", + comments: [ + "feature 1: foo() was called", + "assert 1: foo() was called", + "check source 1: foo() was called" + ] do defmodule MyModule do def my_function() do foo() @@ -54,8 +70,12 @@ defmodule ElixirAnalyzer.ExerciseTest.SuppressIfTest do end end - test_exercise_analysis "assert/feature 3 find baz", - comments: ["feature 3: baz() was called", "assert 3: baz() was called"] do + test_exercise_analysis "assert/feature/check 3 find baz", + comments: [ + "feature 3: baz() was called", + "assert 3: baz() was called", + "check source 3: baz() was called" + ] do defmodule MyModule do def my_function() do baz() diff --git a/test/elixir_analyzer/test_suite/german_sysadmin_test.exs b/test/elixir_analyzer/test_suite/german_sysadmin_test.exs index 9d71d19a..671f39d3 100644 --- a/test/elixir_analyzer/test_suite/german_sysadmin_test.exs +++ b/test/elixir_analyzer/test_suite/german_sysadmin_test.exs @@ -4,6 +4,7 @@ defmodule ElixirAnalyzer.ExerciseTest.GermanSysadminTest do test_exercise_analysis "example solution", comments: [ElixirAnalyzer.Constants.solution_same_as_exemplar()] do + ~S""" defmodule Username do def sanitize('') do '' @@ -24,10 +25,12 @@ defmodule ElixirAnalyzer.ExerciseTest.GermanSysadminTest do sanitized ++ sanitize(tail) end end + """ end test_exercise_analysis "other valid solutions", comments: [] do + ~S""" defmodule Username do def sanitize(list) do List.foldr(list, [], fn code, acc -> @@ -46,70 +49,81 @@ defmodule ElixirAnalyzer.ExerciseTest.GermanSysadminTest do end) end end + """ end test_exercise_analysis "detects cheating with strings", - comments: [Constants.german_sysadmin_no_string()] do - [ - defmodule Username do - def sanitize(charlist) do - charlist - |> Enum.filter(&(&1 < 0xD800)) - |> to_string() - |> String.split("", trim: true) - |> Enum.map(fn letter -> - case letter do - "ß" -> - "ss" + comments: [ + Constants.german_sysadmin_no_string(), + Constants.solution_no_integer_literal() + ] do + ~S""" + defmodule Username do + def sanitize(charlist) do + charlist + |> Enum.filter(&(&1 < 0xD800)) + |> to_string() + |> String.split("", trim: true) + |> Enum.map(fn letter -> + case letter do + "ß" -> + "ss" - "ä" -> - "ae" + "ä" -> + "ae" - "ö" -> - "oe" + "ö" -> + "oe" - "ü" -> - "ue" + "ü" -> + "ue" - letter when letter in ~w(a b c d e f g h i j k l m n o p q r s t u v w x y z) -> - letter + letter when letter in ~w(a b c d e f g h i j k l m n o p q r s t u v w x y z) -> + letter - "_" -> - "_" + "_" -> + "_" - _ -> - "" + _ -> + "" + end + end) + |> Enum.join("") + |> to_charlist() + end + end + """ + end + + test_exercise_analysis "detects cheating with strings, with ?ß notation", + comments: [Constants.german_sysadmin_no_string()] do + ~S""" + defmodule Username do + def sanitize(list) do + List.foldr(list, "", fn code, acc -> + sanitized = + case code do + ?ß -> "ss" + ?ä -> "ae" + ?ö -> "oe" + ?ü -> "ue" + x when x >= ?a and x <= ?z -> <> + ?_ -> "_" + _ -> "" end - end) - |> Enum.join("") - |> to_charlist() - end - end, - defmodule Username do - def sanitize(list) do - List.foldr(list, "", fn code, acc -> - sanitized = - case code do - ?ß -> "ss" - ?ä -> "ae" - ?ö -> "oe" - ?ü -> "ue" - x when x >= ?a and x <= ?z -> <> - ?_ -> "_" - _ -> "" - end - - sanitized <> acc - end) - |> to_charlist() - end + + sanitized <> acc + end) + |> to_charlist() end - ] + end + """ end test_exercise_analysis "using case is required", comments: [Constants.german_sysadmin_use_case()] do [ + ~S""" defmodule Username do def sanitize('') do '' @@ -129,7 +143,9 @@ defmodule ElixirAnalyzer.ExerciseTest.GermanSysadminTest do sanitized ++ sanitize(tail) end - end, + end + """, + ~S""" defmodule Username do def sanitize('') do '' @@ -147,6 +163,56 @@ defmodule ElixirAnalyzer.ExerciseTest.GermanSysadminTest do defp do_sanitize(x) when x >= ?a and x <= ?z, do: [x] defp do_sanitize(x), do: [] end + """ ] end + + test_exercise_analysis "detects integer literals", + comments: [Constants.solution_no_integer_literal()] do + ~S""" + defmodule Username do + def sanitize('') do + '' + end + + def sanitize([head | tail]) do + sanitized = + case head do + 252 -> 'ue' + 246 -> 'oe' + 228 -> 'ae' + 223 -> 'ss' + x when x >= 97 and x <= 122 -> [x] + 95 -> '_' + _ -> '' + end + + sanitized ++ sanitize(tail) + end + end + """ + end + + test_exercise_analysis "valid solution without quoting triggers comment", + # This is because Elixir's ASTs don't differentiate between code points like ?ß and integers + comments: [Constants.solution_no_integer_literal()] do + defmodule Username do + def sanitize(list) do + List.foldr(list, [], fn code, acc -> + sanitized = + case code do + ?ß -> 'ss' + ?ä -> 'ae' + ?ö -> 'oe' + ?ü -> 'ue' + x when x >= ?a and x <= ?z -> [x] + ?_ -> '_' + _ -> '' + end + + sanitized ++ acc + end) + end + end + end end diff --git a/test/support/analyzer_verification/check_source.ex b/test/support/analyzer_verification/check_source.ex new file mode 100644 index 00000000..3ac8be8a --- /dev/null +++ b/test/support/analyzer_verification/check_source.ex @@ -0,0 +1,63 @@ +defmodule ElixirAnalyzer.Support.AnalyzerVerification.CheckSource do + @moduledoc """ + This is an exercise analyzer extension module to test the check_soucre macro + """ + + use ElixirAnalyzer.ExerciseTest + + check_source "always true" do + type :actionable + comment "always return true" + + check(_source) do + true + end + end + + check_source "always false" do + type :actionable + comment "always return false" + + check(_source) do + false + end + end + + check_source "finds integer literal" do + type :actionable + comment "used integer literal from ?a to ?z" + + check(source) do + integers = Enum.map(?a..?z, &to_string/1) + + not Enum.any?(integers, &String.contains?(source, &1)) + end + end + + check_source "finds use of multiline strings" do + type :actionable + comment "didn't use multiline" + + check(source) do + String.contains?(source, ["\"\"\"", "\'\'\'"]) + end + end + + check_source "module is suitably long" do + type :actionable + comment "module is too short" + + check(source) do + String.length(source) > 20 + end + end + + check_source "module is well formatted" do + type :actionable + comment "module is not formatted" + + check(source) do + String.trim(source) == Code.format_string!(source) |> Enum.join() + end + end +end diff --git a/test/support/analyzer_verification/suppress_if.ex b/test/support/analyzer_verification/suppress_if.ex index 4f18fa4d..9db1cd82 100644 --- a/test/support/analyzer_verification/suppress_if.ex +++ b/test/support/analyzer_verification/suppress_if.ex @@ -22,6 +22,12 @@ defmodule ElixirAnalyzer.Support.AnalyzerVerification.SuppressIf do called_fn name: :foo end + check_source "check source 1: no foo() unless common check was found" do + comment "check source 1: foo() was called" + suppress_if Constants.solution_debug_functions(), :fail + check(source, do: not String.contains?(source, "foo")) + end + feature "feature 2: no bar() unless assert 1 found a foo() first" do find :none comment "feature 2: bar() was called" @@ -38,6 +44,12 @@ defmodule ElixirAnalyzer.Support.AnalyzerVerification.SuppressIf do called_fn name: :bar end + check_source "check source 2: no bar() unless assert 1 found a foo() first" do + comment "check source 2: bar() was called" + suppress_if "check source 1: no foo() unless common check was found", :fail + check(source, do: not String.contains?(source, "bar")) + end + feature "feature 3: no baz() unless feature 1 found a foo() first" do find :none comment "feature 3: baz() was called" @@ -53,4 +65,10 @@ defmodule ElixirAnalyzer.Support.AnalyzerVerification.SuppressIf do suppress_if "assert 1: no foo() unless common check was found", :fail called_fn name: :baz end + + check_source "check source 3: no baz() unless feature 1 found a foo() first" do + comment "check source 3: baz() was called" + suppress_if "feature 1: no foo() unless common check was found", :fail + check(source, do: not String.contains?(source, "baz")) + end end