Skip to content

Commit

Permalink
Add a check for snake_case function names
Browse files Browse the repository at this point in the history
  • Loading branch information
angelikatyborska committed Jul 9, 2021
1 parent f4562e2 commit 2b5b3a7
Show file tree
Hide file tree
Showing 5 changed files with 334 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/elixir_analyzer/constants.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ defmodule ElixirAnalyzer.Constants do
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

Expand Down
2 changes: 2 additions & 0 deletions lib/elixir_analyzer/exercise_test/common_checks.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do
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()
Expand Down
67 changes: 67 additions & 0 deletions lib/elixir_analyzer/exercise_test/common_checks/function_names.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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)

if names == [] do
[]
else
wrong_name =
names
|> Enum.reverse()
|> hd()
|> to_string()

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
}
}}
]
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
# credo:disable-for-this-file Credo.Check.Readability.FunctionNames

defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNamesTest do
use ExUnit.Case
alias ElixirAnalyzer.Comment
alias ElixirAnalyzer.Constants
alias ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames

test "returns empty list if there are functions, macros, or guards" do
code =
quote do
defmodule User do
defstruct(:name, :email)
end
end

assert FunctionNames.run(code) == []
end

test "doesn't crash when a variable is named def" do
code =
quote do
defmodule User do
def name(user) do
def = user.first_name
defmacro = user.last_name
def <> " " <> defmacro
end
end
end

assert FunctionNames.run(code) == []
end

test "returns empty list if all names are correct" do
code =
quote do
defmodule User do
def first_name(user), do: user.first_name
defp registered_in_last_quarter?(user), do: true
defmacro validate_user!, do: true
defmacrop do_validate_user, do: true
defguard(is_user, do: true)
defguardp(is_registered_user, do: true)
end
end

assert FunctionNames.run(code) == []
end

test "returns an actionable comment with params" do
code =
quote do
defmodule User do
def firstName(user), do: user.first_name
end
end

assert FunctionNames.run(code) == [
{:fail,
%Comment{
type: :actionable,
comment: Constants.solution_function_name_snake_case(),
params: %{
expected: "first_name",
actual: "firstName"
}
}}
]
end

test "works for def, defp, defmacro, defmacrop, defguard, and defguardp" do
get_result = fn expected, actual ->
[
{:fail,
%Comment{
type: :actionable,
comment: Constants.solution_function_name_snake_case(),
params: %{
expected: expected,
actual: actual
}
}}
]
end

code =
quote do
defmodule User do
defp registeredInLastQuarter?(user), do: true
end
end

assert FunctionNames.run(code) ==
get_result.("registered_in_last_quarter?", "registeredInLastQuarter?")

code =
quote do
defmodule User do
defmacro validateUser!, do: true
end
end

assert FunctionNames.run(code) == get_result.("validate_user!", "validateUser!")

code =
quote do
defmodule User do
defmacrop doValidateUser, do: true
end
end

assert FunctionNames.run(code) == get_result.("do_validate_user", "doValidateUser")

code =
quote do
defmodule User do
defguard(isUser, do: true)
end
end

assert FunctionNames.run(code) == get_result.("is_user", "isUser")

code =
quote do
defmodule User do
defguardp(isRegisteredUser, do: true)
end
end

assert FunctionNames.run(code) == get_result.("is_registered_user", "isRegisteredUser")
end

test "only reports the first wrong name" do
code =
quote do
defmodule User do
def first_name(user), do: getFirstName(user)
defguard(isRegisteredUser(user), do: true)
defp getFirstName(user), do: user.first_name
end
end

assert FunctionNames.run(code) == [
{:fail,
%Comment{
type: :actionable,
comment: Constants.solution_function_name_snake_case(),
params: %{
expected: "is_registered_user",
actual: "isRegisteredUser"
}
}}
]
end

test "can handle function heads" do
code =
quote do
defmodule User do
def getName(user, fallback \\ "Doe")
end
end

assert FunctionNames.run(code) == [
{:fail,
%Comment{
type: :actionable,
comment: Constants.solution_function_name_snake_case(),
params: %{
expected: "get_name",
actual: "getName"
}
}}
]
end
end
87 changes: 87 additions & 0 deletions test/elixir_analyzer/exercise_test/common_checks_test.exs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# credo:disable-for-this-file Credo.Check.Readability.ModuleAttributeNames
# credo:disable-for-this-file Credo.Check.Readability.FunctionNames

defmodule ElixirAnalyzer.ExerciseTestTest.Empty do
use ElixirAnalyzer.ExerciseTest
Expand All @@ -10,6 +11,92 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecksTest do
use ElixirAnalyzer.ExerciseTestCase,
exercise_test_module: ElixirAnalyzer.ExerciseTestTest.Empty

describe "function, guard, macro names" do
test_exercise_analysis "doesn't report correct names",
comments_exclude: [Constants.solution_function_name_snake_case()] do
[
defmodule SomeModule do
def foo_bar, do: 1

def foo_bar_baz do
foo_bar + 8
end
end,
defmodule SomeModule do
defp foo_bar, do: 1

defp foo_bar_baz do
foo_bar + 8
end
end,
defmodule SomeModule do
defmacro foo_bar, do: 1

defmacro foo_bar_baz do
foo_bar + 8
end
end,
defmodule SomeModule do
defmacrop foo_bar, do: 1

defmacrop foo_bar_baz do
foo_bar + 8
end
end,
defmodule SomeModule do
defguard(foo_bar, do: 1)

defguard foo_bar_baz do
foo_bar + 8
end
end,
defmodule SomeModule do
defguardp(foo_bar, do: 1)

defguardp foo_bar_baz do
foo_bar + 8
end
end
]
end

test_exercise_analysis "reports incorrect names",
comments_include: [Constants.solution_function_name_snake_case()] do
[
defmodule SomeModule do
def fooBarBaz do
foo_bar + 8
end
end,
defmodule SomeModule do
defp fooBarBaz do
foo_bar + 8
end
end,
defmodule SomeModule do
defmacro fooBarBaz do
foo_bar + 8
end
end,
defmodule SomeModule do
defmacrop fooBarBaz do
foo_bar + 8
end
end,
defmodule SomeModule do
defguard fooBarBaz do
foo_bar + 8
end
end,
defmodule SomeModule do
defguardp fooBarBaz do
foo_bar + 8
end
end
]
end
end

describe "module attribute names" do
test_exercise_analysis "doesn't report correct module attribute names",
comments_exclude: [Constants.solution_module_attribute_name_snake_case()] do
Expand Down

0 comments on commit 2b5b3a7

Please sign in to comment.