Closed petros closed 1 year ago
Thank you for the bug report! There's definitely something wrong. The check for the order of @doc
and @spec
seems not to handle private functions very well.
This code produces a comment about a wrong order when it shouldn't:
defmodule Test do
@spec x()
def x()
@spec y()
defp y()
end
This one as well:
defmodule Test do
@spec x()
def x()
@spec y()
defmacrop y()
end
and also this one:
defmodule Test do
@spec x()
def x()
@spec y()
end
Well, I think one only has to change the last function in /elixir-analyzer/tree/main/lib/elixir_analyzer/exercise_test/common_checks/function_annotation_order.ex to
defp check_wrong_order(%{operations: operations}) do
Enum.uniq(operations) not in [
[],
[:def],
[:defp],
[:defmacro],
[:defmacrop],
[:spec, :def],
[:spec, :defp],
[:spec, :defmacro],
[:spec, :defmacrop],
[:doc, :def],
[:doc, :defp],
[:doc, :defmacro],
[:doc, :defmacrop],
[:doc, :spec, :def],
[:doc, :spec, :defp],
[:doc, :spec, :defmacro]
[:doc, :spec, :defmacrop]
]
end
end
to also include all the cases where there are private functions or macros.
I'd be able to modify the corresponding files and write corresponding tests etc., but since I haven't been contributing here before, I'm not sure how I should go about it.
If I fork this repo and create a feature branch, how can I push this branch and create a pull request. Don't I have to be added as a collaborator first?
If I fork this repo and create a feature branch, how can I push this branch and create a pull request. Don't I have to be added as a collaborator first?
You can create a pull request from your fork, that targets the main branch of the parent repository.
@Teilzeitdenker
Well, I think one only has to change the last function in (...) to also include all the cases where there are private functions or macros.
I think you would need to start here instead: https://github.com/exercism/elixir-analyzer/blob/5765dc1213c430c81cda8b1c008d659d05a170c4/lib/elixir_analyzer/exercise_test/common_checks/function_annotation_order.ex#L12 I think the main problem with the current code is that defp
, defmacrop
, defguard
and defguardp
don't trigger a cut off in the chain of found annotations. After that most likely check_wrong_order
needs to be edited too.
If I fork this repo and create a feature branch, how can I push this branch and create a pull request. Don't I have to be added as a collaborator first?
You don't need to be added as a collaborator. First you need to create your own fork of this repository. You have all the necessary permissions to your own fork, and as @petros said, you can always open a pull request from your own fork into this repository. That doesn't require extra permissions.
Ok, I see. Thanks for the advice.
I'll give it a try and issue a pull request from my account if I'm able to fix it.
I issued a pull request, but I can't link it in here...
I was working on the Name Badge exercise, where I decided to have a
@spec
for two private functions. In my editor, locally, I initially had a@doc
and a@spec
for those functions. ElixirLS informed me that the@doc
strings will be stripped out because those are private functions. I decided to remove the@doc
strings and only keep the@spec
s.The code (SPOILER)
```elixir defmodule NameBadge do @separator " - " @doc """ Take an id, a name, and a department and format a string that can be printed on a badge. It handles not having an ID which is the case for new employees. And it recognizes and onwer when no department is passed. """ @spec print(integer() | nil, String.t(), String.t() | nil) :: String.t() def print(id, name, department) do format_id(id) <> name <> format_department(department) end @spec format_id(integer() | nil) :: String.t() defp format_id(id) do if is_nil(id), do: "", else: "[#{id}]" <> @separator end @spec format_department(String.t() | nil) :: String.t() defp format_department(department) do if is_nil(department), do: @separator <> "OWNER", else: @separator <> String.upcase(department) end end ```When I submitted my iteration, I got the following from the Elixir Analyzer:
I do understand that, but why would I get it for a function that only has a
@spec
, and not a@doc
string?I would understand it if the recommendation was to also have a
@doc
string. But referring to the order of something that does not exist is confusing.