exercism / elixir-analyzer

GNU Affero General Public License v3.0
30 stars 32 forks source link

Warns about order of doc and spec attributes, but I can't find the error #284

Closed chriseyre2000 closed 2 years ago

chriseyre2000 commented 2 years ago

Here is the code:

defmodule School do
  @moduledoc """
  Simulate students in a school.

  Each student is in a grade.
  """
  @type school :: any()
  @doc """
  Create a new, empty school.
  """
  @spec new() :: school
  def new() do
    %{}
  end
  @doc """
  Add a student to a particular grade in school.
  """
  @spec add(school, String.t(), integer) :: {:ok | :error, school}
  def add(school, name, grade) do
    if school |> Map.get(name) != nil do
      {:error, school}
    else
      {:ok, school |> Map.put(name, grade)}
    end
  end
  @doc """
  Return the names of the students in a particular grade, sorted alphabetically.
  """
  @spec grade(school, integer) :: [String.t()]
  def grade(school, grade) do
    (for {k, v} <- school, v == grade, do: k) |> Enum.sort()
  end
  @doc """
  Return the names of all the students in the school sorted by grade and name.
  """
  @spec roster(school) :: [String.t()]
  def roster(school) do
    (for {k, v} <- school, do: {k, v} )
    |> Enum.sort(School.Sorting)
    |> Enum.map(&elem(&1, 0))
  end
  defmodule Sorting do
    @doc """
    Provides a compare function
    """
    @spec compare(first :: {String.t(), integer}, second :: {String.t(), integer}) :: :gt | :lt | :eq
    def compare({name, grade}, {name2, grade2}) do
      cond do
        grade > grade2 -> :gt
        grade < grade2 -> :lt
        name > name2 -> :gt
        name < name2 -> :lt
        true -> :eq
      end
    end
  end
end

Here is the warning:

Developers can choose the order of the @doc and @spec modules attributes, but the Elixir community convention is to use @doc first and @spec next to the function.

Example:

@doc """
This function counts the number elements in a list
"""
@spec count(list(any())) :: integer()
def count(list), do: length(list)
jiegillet commented 2 years ago

I bet it has something to do with the Sorting module. This check is a bit flacky and needs to be revisited.

chriseyre2000 commented 2 years ago

I only added the Sorting module to bypass another analytics warning, which has been raised as a distinct issue.

jiegillet commented 2 years ago

Oh sure, I didn't mean to criticize your code, I just remember thinking that the PR that added this check didn't handle switching modules in the best way possible.