akoutmos / doctor

Ensure that your Elixir project documentation is healthy
MIT License
178 stars 15 forks source link

Incorrect data gathering for __using__ macros #34

Closed jrogov closed 3 years ago

jrogov commented 3 years ago

Hi!

First of all, thanks for such a great library: although not without sharp edges, it's a perfect replacement for inch_ex for automated checks! Definitely going to keep using it.

Here's an issue I've encountered: it seems doctor can't parse @spec in quoted code inside __using__ macro (not sure if it even must to).

Module:

defmodule SampleModule do
  defmacro __using__(opts) do
    quote do
      @spec answer() :: :integer
      def answer, do: 42

      @spec same_answer() :: :integer
      def same_answer, do: 42
    end
  end
end

Please note that 2 functions here are only to be sure that exactly funs inside __using__'s quoted code is the source: not some Elixir-generated MACRO-__using__.

Result:

~/c/d/s/using_bug (master) $ mix doctor --short
Doctor file not found. Using defaults.
Generated using_bug app
----------------------------------------------------------------------------------------------
Doc Cov  Spec Cov  Functions  Module                                   Module Doc  Struct Spec
100%     0%        2          SampleModule                             No          N/A
----------------------------------------------------------------------------------------------
Summary:

Passed Modules: 0
Failed Modules: 1
Total Doc Coverage: 100.0%
Total Spec Coverage: 0.0%
akoutmos commented 3 years ago

Thanks for submitting this issue and for using Doctor! I have had some issues with this as well and have been working on a solution. I think my only option will be to look at the AST for a module and extract out the function definitions within defmacro. Whereas currently I am mostly relying on the Code.fetch_docs/1 and Code.Typespec.fetch_specs/1 which are used at runtime once the module has already been compiled.

For now I have been adding macro modules to my ignore list. Hoping to have a better solution soon though!

jrogov commented 3 years ago

Sorry for late response!

Didn't know it was using those functions! Workaround with ignore list is in fact the exact solution I've used!

But yeah, it seems that working directly with AST is required. Yet, this direction raises a question: what's the future of the project through the lens of existing Credo? I can clearly see that they're somewhat different and solve completely different problems, yet the underlying solution is very similar 🤔.

Regardless, I think that Credo's traverse function primitive might be very useful here.