Ignores functions used inside 'defmethod'

[thisdotrob]

In the following code try-parse-num is flagged as unused:

(defn- try-parse-num          
  "Attempt to parse a num-string. Returns nil if fails, or num otherwise. Takes parse-fn and num-str"                                                                                                 
  [parse-fn num-str]
  (if (string? num-str)                      
    (try (parse-fn num-str)                  
      (catch Exception e (str "Num-string parse fail. Input: " (.getMessage e))
        nil))))

(defmethod coerce-num-str
  :int [num-str _]
  (if (number? num-str)
    (int num-str)
    (try-parse-num #(Integer/parseInt %) num-str)))

[venantius]

This is a harder feature to support than you might think.

The way Yagni works is to look at all the interned vars returned by (ns-interns) for each namespace it can find. Fundamentally, this method only lets us know about the defmulti definition, which isn't particularly helpful.

Normally the way Yagni works is once it knows about a definition it can extract metadata from that definition to look up its actual source code. The problem with defmulti/defmethod is that because there's only the "one" record in ns-interns we don't know where to look up the various defmethods. We can find out what methods exist on a defmulti by calling methods, but that just returns a map of pure functions without the metadata we care about like what line number those functions are defined on for the purposes of looking up the actual form and walking it.

Getting around this would require a major re-write of the walker from the top down - instead of enumerating all of the forms with ns-interns we'd start instead by just feeding the whole namespace source code itself directly into the walker and parsing things that way. Not impossible, but a big change from how things are done today.

Once that was done we'd also want to change the way Yagni actually reports on unused code - it'd no longer be enough to say coerce-num-str wasn't used; we'd want to at least provide the file and line number, and potentially call out explicitly that it was an implementation of a multimethod with a particular dispatch.

Anyways all of that seems good and worth working on. Just not a small feature ask :). Same applies to #41

[venantius]

To add one more note to my prior comment - even if Yagni did have the capability to track the various multimethod implementations and to parse their contents, it would still have its core limitation of not being able to tell which dispatch functions were actually used by the program. This means that you could have a function called in one method implementation that is in turn never dispatched to - Yagni would never know and would just have to track whether the multimethod was called or not.