encore icon indicating copy to clipboard operation
encore copied to clipboard

Add clj-kondo hooks

Open ericdallo opened this issue 2 years ago • 1 comments

This should make clj-kondo understand the defalias custom macro and correctly lint it as an alias, this PR also add the hook config to resources classpath following export feature, this make clj-kondo understand this macro on any lib that uses encore for example taoensso.timbre/spit-appender, the user only needs to add to its clj-kondo config a {:config-paths ["taoensso/encore"]}.

Some examples of other libs that did the same: midje, state-flow

ericdallo avatar Sep 04 '21 15:09 ericdallo

LMK if any questions @ptaoussanis :)

ericdallo avatar Sep 04 '21 15:09 ericdallo

@ericdallo Hi Eric, apologies for the long delay getting back to you on this! I've never used kondo, so not too familiar with the details on this.

Just double checking if the PR is still up-to-date / relevant? If so, will get it merged as part of the next Encore release.

Thanks!

ptaoussanis avatar Nov 29 '22 17:11 ptaoussanis

No problem @ptaoussanis! Yes, it's still relevant as it should help users of encore and clj-kondo (via clojure-lsp as well) have that config automatically and make linter and other features happy :)

ericdallo avatar Nov 29 '22 20:11 ericdallo

Merging now, thanks Eric!

ptaoussanis avatar Nov 30 '22 08:11 ptaoussanis

@ericdallo I don't see where the dependency on clj-kondo is defined in the project. In fact, I'm getting these errors in one of my projects (when loading taoensso.encore)

1. Caused by java.io.FileNotFoundException
   Could not locate clj_kondo/hooks_api__init.class, clj_kondo/hooks_api.clj or
   clj_kondo/hooks_api.cljc on classpath. Please check that namespaces with dashes use underscores
   in the Clojure file name.

jumarko avatar Dec 15 '22 13:12 jumarko

@jumarko Hi Juraj, while waiting for a reply from Eric - I'd like to ask in the meantime:

In fact, I'm getting these errors in one of my projects (when loading taoensso.encore)

What do you mean by "loading" here? Can you be more specific about exactly what command you're running? What build tool are you using?

Thanks!

ptaoussanis avatar Dec 15 '22 13:12 ptaoussanis

This works following clj-kondo exports feature, basically the encore lib how has under resources folder the clj-kondo hooks configuration, when clj-kondo is lining a project that has encore as the lib dependency, clj-kondo will scan the encore jar and find this clj-kondo config as it's available on the classpath, all what the user needs to do is include this on it's project:

.clj-kondo/config.edn

{:config-paths ["taoensso/encore"]}

ericdallo avatar Dec 15 '22 16:12 ericdallo

@ptaoussanis I'm trying to use datalevin that requires encore. I had encore set in my deps.edn file explicitly but it was too old so I decided to upgrade to the latest version of encore. However, when I required dataleving.core it failed (transitively) because of encore:

(ns clojure-experiments.datomic.datalevin
  "https://github.com/juji-io/datalevin/blob/master/README.md"
  (:require
   [datalevin.core :as d]))

trying to load the above ns definition fails:

Syntax error macroexpanding taoensso.encore/defalias at (taoensso/encore.cljc:541:6).
[encore/defalias] Failed to resolve source var

It works using encore version 3.30.0 (chosen more or less randomly looking at the releases that were made before this PR was merged).

I was wondering why it couldn't find the var and thus the namespace. Thus I opened the encore jar and tried to load encore.clj - I didn't realize it was actually the one in clj-kondo.exports/...

That's the reason why I thought it's related to this change. But it seems that problem is different - maybe it has something to do with the truss version in my project?

jumarko avatar Dec 15 '22 18:12 jumarko

Syntax error macroexpanding taoensso.encore/defalias at (taoensso/encore.cljc:541:6).

Just to confirm: this error should also mention what the exact var is couldn't be resolved. It's taoensso.truss/with-data? If so, it sounds like you may have a dependency conflict involving Truss.

Are you manually including Truss in your own project? If so, please make sure that you're either on the latest version - or just let Encore bring in an appropriate version automatically.

Some info to help you debug here.

ptaoussanis avatar Dec 15 '22 18:12 ptaoussanis

Upgrading to truss 1.8.0 indeed fixed the problem. Thanks for the help!

jumarko avatar Dec 15 '22 19:12 jumarko

No problem Juraj, thanks for the confirmation. And thanks @ericdallo!

ptaoussanis avatar Dec 15 '22 20:12 ptaoussanis