Fork me on GitHub
#clj-kondo
<
2020-04-09
>
helios08:04:38

Could this be a bug? https://nextjournal.com/helios/clj-kondo-md-repro There's something off with js interop and detecting whether we use or not a required ns

borkdude08:04:47

is this related to shadow-cljs somehow?

borkdude08:04:13

I mean, using the alias as some value?

borkdude08:04:08

$ plk
ClojureScript 1.10.520
cljs.user=> (require '[clojure.set :as s])
nil
cljs.user=> s
            ^
WARNING: Use of undeclared Var cljs.user/s at line 1
nil
^ @helios Not sure if I understand your repro. Could you post some real working code using CLJS only?

helios08:04:56

@borkdude we don't use shadow-cljs. But this is just a snippet from our codebase

borkdude08:04:33

without a repro I can understand, I can't say if this is a false positive or not.

borkdude08:04:10

pretty cool that you did it with nextjournal though 😉

helios08:04:18

(ns foo
  (:require [markdown-it-texmath :as md-texmath]))
           
(use md-texmath)
will say that markdown-it-texmath is required but never used

borkdude08:04:00

what is use? I've never seen that in CLJS

borkdude08:04:41

if this is about requiring namespaces using use I'd say this is pretty weird code, especially in CLJS?

borkdude08:04:42

if you could go through the trouble setting up a small CLJS project which has this code and I can run this using the clojure CLI, I'd be happy to take a further look

helios09:04:14

i don't think it should complain that md-textmath is not being used, technically

helios09:04:47

I don't know yet what it does in the context, i'll try to figure it out

borkdude09:04:04

@helios I still don't understand why this code should work at all. Using use is very rare in CLJS, especially in a threaded expression

helios09:04:21

For one second consider the second example. Why is that code not supposed to work?

borkdude09:04:06

because use requires namespaces and md-texmath is an alias? normally you use aliases like (md-texmath/foobar) to refer to a function of a namespace

borkdude09:04:29

I'm not saying it doesn't do something, but I don't understand this construct

borkdude09:04:24

What I expect to see is something like:

(ns foo
  (:require [markdown-it-texmath :as md-texmath]))

(md-texmath/dude)

borkdude09:04:06

I'll try with lumo

helios09:04:24

Okay, i'll investigate a bit further to understand what the snipped with the threading is supposed to do and why it's not blowing up in our codebase

Andrea09:04:42

in the example above use in this case is an instance method of the return value of md not the clojure use, it’s threaded cljs style with the dot-dot ..

borkdude09:04:52

@andrea712 Is it possible to create a small repro that I can run with e.g. lumo? I can npm install any lib and then run a script with lumo. Should be relatively easy to do

borkdude09:04:34

And then post this as an issue in the clj-kondo github repo

Andrea09:04:30

can discuss it with @helios, I’m not an expert in the field but maybe we’d need to make lumo aware of some piece of cljs compiler options about :global-exports ? cc @kommen

borkdude09:04:52

@andrea712 using pure CLJS is also fine, as long as I can run it to see what's happening

borkdude09:04:50

I just thought lumo would be less work, since you can just run a script, but a small repro repo using no downstream tooling might be better

borkdude09:04:54

Never mind, I think this is enough repro:

(require '[foo.lib :as foo])
(def x #js {:use (fn [x] x)})
(.. x (use foo))

borkdude09:04:41

I'm going to verify in #clojurescript if this is idiomatic: using aliases as values

borkdude10:04:56

@helios @andrea712 Now I remember. Use can or maybe even should use string namespace names for npm libs:

(require '["foo.lib" :as foo])
(def x #js {:use (fn [x] x)})
(.. x (use foo))
Then this won't give false positives anymore.

helios10:04:05

could this be something that we should propose to the clojure style guide?

borkdude10:04:50

possibly yes

borkdude10:04:58

so this works 100% free of errors:

(ns foo
  (:require
   ["markdown-it" :as md]
   ["markdown-it-texmath" :as md-texmath]
   [com.nextjournal.editor.markdown.todo-lists :as todo-lists]
   [com.nextjournal.editor.markdown.gh-preamble :as gh-preamble]))

(def mdit (.. (md #js{:linkify true :breaks false :html true})
              (use md-texmath)
              (use gh-preamble/Plugin)
              (use todo-lists/Plugin)))

parrot 4
Andrea10:04:28

thank you!

seancorfield16:04:29

@borkdude I notice that -main only returns a non-zero exit status for warnings and errors, which means that linter-kondo doesn't attempt to parse the response for any info level reports. What's your reasoning behind that, and how do you view info level linters overall?

borkdude17:04:22

@seancorfield hmm, flycheck does show info level, regardless of the exit code. exit code non zero usually means something is wrong. when you have no errors or warning, nothing is wrong

borkdude17:04:51

which is useful for CI environments and pre-commit hooks for example

borkdude17:04:58

I guess that's on linter-kondo

borkdude17:04:13

I'm sure gerred will take a PR to change it. ^ @seancorfield

seancorfield17:04:59

Yeah, I might go that route. Was just curious to hear your take on what you expected :info to be used for.

seancorfield17:04:54

@gerred How do you feel about linter-kondo always parsing the output and showing :info level responses?

seancorfield17:04:25

(I don't know how they would map into Atom's annotations on lines etc?)

gerred17:04:31

yeah I'm open to that!

gerred17:04:00

the last part I'm not sure either, I'd have to look at the core atom linter package

seancorfield18:04:31

(my use case is that I'd like to have missing docstring as :info level so they are still flagged but don't constitute a "failure")

borkdude18:04:54

@seancorfield I guess you can just write a shell script named clj-kondo, put that on your path, exit a non-zero exit code and print some clj-kondo like info lines before exiting, just to see how Atom responds to it.

seancorfield18:04:15

@borkdude linter-kondo doesn't pick up :info at all right now so that wouldn't work.

borkdude18:04:05

@seancorfield Another thought: I bet there is some generic linter package for Atom in which you can point to an executable or script that returns linting output. Maybe: https://atom.io/packages/linter The output that clj-kondo and joker produce is a generally accepted format

seancorfield18:04:00

I might take a look when I'm not working on other stuff 🙂