elixir-analyzer
elixir-analyzer copied to clipboard
exercism
fixed issue #318, order of @doc and @spec in private functions or macros
By just adding three lines in the check_wrong_order() function, one is able to allow for all combinations with :defp and :defmacrop at once. Since the nodes corresponding to different functions are chunked and merged accordingly, there is no harm done by adding these to the allowed combinations.
The three examples from Angelika that produced comments before now run smoothly.
A wrong order of @spec - @doc - def(p)/defmacro(p) still doesn't pass.
mix test also passes completely.
I think one may leave out the @def_ops completely, such that the check_wrong_order() function just reads something like
defp check_wrong_order(%{operations: operations}) do
Enum.uniq(operations) not in [
[],
[:spec],
[:doc],
[:doc, :spec],
]
end
end
but I didn't try it.
I'm not sure if I should write corresponding tests for the new behaviour and if so, in which file I should do that. What's your opinion?
Yes, please add tests so we can see the behavior difference. Ideally those would be tests that initially fail before making changes to the code.
Ok, I wrote some tests that would't pass without the fix. Please review the changes.
Yes, I see. It was just the shortest (and probably dirtiest) fix for the (in my experience with the track exercises) most common bug, when defining a private function with a @spec always triggers a misleading (and after a while a little annoying) analyzer comment.
I will go back to the start (if I'm able to roll back git to the initial point) and try to apply your suggested changes (if I can't find a way, I'll admit it within two weeks or so, I'm still learning, so it's still a bit hard for me to manipulate the AST).
By the way: Should I put any new tests in a separate pull request or does it suffice to put them in a separate commit, such that it is possible for you to run them first on the old and then on some new version from a later commit?
You don't need to roll back anything, you can take it from this point on. The tests are great and should stay, the changes in check_wrong_order can stay as well (if they are still necessary).
You should keep going in this PR and commit whatever changes, with git it's always possible to jump around in the timeline, even for specific files if necessary.
Take your time, and do ask for help if you need some. I think the changes I'm talking about won't require manipulating the AST too much, just tweaking what we already have.
@Teilzeitdenker do you mind if I take this PR over? If you don't, I'll jump in in a couple o days.