jonase / eastwood

Clojure lint tool
1.08k stars 65 forks source link

Useless :unused-ret-val warning perhaps related to :gen-class #39

Closed jafingerhut closed 10 years ago

jafingerhut commented 10 years ago

For an example, see the following namespace inside of contrib library tools.reader. This warning still appears if you comment everything after the ns statement, and the ns statement includes a :gen-class, which is not common in the libraries I have tested, thus leading me to suspect it as the cause:

== Linting clojure.tools.reader.impl.ExceptionInfo == {:linter :unused-ret-vals, :msg "Constant value is discarded inside null: null", :line nil}

Bigsy commented 10 years ago

+1 I get this and its 100% related to :gen-class.

{:linter :unused-ret-vals, :msg "Constant value is discarded inside null: nil (nil value is returned by comment and gen-class expressions)", :line nil, :column nil}

jafingerhut commented 10 years ago

Thanks for the comment. I would like to figure out a good way to prevent these warnings, but it is a bit difficult given the current contents of the abstract syntax trees that the source code is analyzed into, because gen-class and comment are both macros that expand to nil. The best way I can think to avoid these warnings is to somehow attach info to the AST to indicate what the original form was that became the nil, which would probably mean remembering the original pre-macroexpanded form of all forms. Should be possible, but we are not there yet.

vbauer commented 10 years ago

Hello Andy!

Just FYI: I have the same warning on this code:

(ns ziggurat.app
  (:use ziggurat.core)
  (:require [ziggurat.ui.handler :refer [app]]
            [ring.middleware.reload :as reload]
            [org.httpkit.server :as http-kit]
            [taoensso.timbre :as timbre])
  (:gen-class))

(defn -main [& args]
  (let [site (if (:dev-mode config) (reload/wrap-reload app) app)
        port (:http-port config)]
    (http-kit/run-server site {:port port})
    (timbre/info "Server started on port" port)))

As result:

== Linting ziggurat.app ==
{:linter :unused-ret-vals,
 :msg
 "Constant value is discarded inside null: nil (nil value is returned by comment and gen-class expressions)",
 :line nil,
 :column nil}
eigenhombre commented 10 years ago

+1 on this, same behavior as others see.

Otherwise, great tool.

jafingerhut commented 10 years ago

Thanks for the additional reports. Nicola Mometto has been making some enhancements to the tools.analyzer(.jvm) libraries recently that I believe will help make it easy for Eastwood to eliminate these false reports. No promises yet, but will update this ticket if I think I have an actual fix.

jafingerhut commented 10 years ago

Thanks to recent enhancements by Nicola Mometto in tools.analyzer and tools.analyzer.jvm libraries, fixing this problem became easy.

The fix should be included in the next release of Eastwood. No particular date is planned for this yet. I would like to fix a few other things that should also be easy now, before doing another release. Keep an eye on the Clojure Google group for an announcement, and I will also add another comment to this issue when it is released, asking folks to verify the fix for their code base.

jafingerhut commented 10 years ago

Eastwood version 0.1.4 was just released. Please try it out and let us know if it eliminates the incorrect warnings due to :gen-class

eigenhombre commented 10 years ago

Works for me now, thanks!