reports "suspicious-expression" for valid uses of `core.async/alt!`

[darren-mk]

Currently, Eastwood seems to report error for valid cases of core.async/alt!.

For example, it reports suspicious-expression: or called with 1 args. (or x) always returns x. Perhaps there are misplaced parentheses?

for below code, which is excerpted from

https://clojuredocs.org/clojure.core.async/alt!

`(def trade-ch (chan))

(go-loop [] (<! (timeout 1000)) (print (<! trade-ch)) (recur))

(go (let [timeout-ch (timeout 1000) trade 100] (-> (alt! [[trade-ch trade]] :sent timeout-ch :timed-out) print))) ;;eval this at will`

Can anyone address this? If not, I will try to make a pull request.

Thanks.

[vemv]

Hi Darren,

thanks for the report!

Yes, I think I had seen this some years ago.

A silencing in the form of these files https://github.com/jonase/eastwood/tree/1b87b1f377052116f57c2b62b2f0e97669d91551/resource/eastwood/config should work.

When adding such a silencing, the :within-depth value should be as low as possible. Normally I try one value after the other, incrementing it until it works.

If contributing a PR, please make sure it's green in advance. Building on Circle from a public fork should be free. In particular we lint against core.async as a git submodule so something might need a tweak over there.

Remember you can always add {:config-files ["my_local_config.clj"] to important projects you work on, so that you don't have to craft a PR on a rush.

Cheers - V

[darren-mk]

Hi vemv,

thanks for sharing the tip.

disable-warning macro seems like a solution. if i wrote my own disable-warning, am i supposed to add it in my project? if so, where in the project? i am using lein as a build tool.

[vemv]

You can write a file named "eastwood.clj" anywhere in the project (including the project's root) and refer to it in config:

;; root level
{:config-files ["eastwood.clj"]}

;; Or, arbitrary relative path
{:config-files ["dev/tooling/eastwood.clj"]}

[darren-mk]

sorry, it doesn't come as success on my end.

in resources/eastwood.clj

(disable-warning
 {:linter :suspicious-expression
  :for-macro 'clojure.core.async/alt!
  :if-inside-macroexpansion-of #{'clojure.core.async/go}
  :within-depth 2
  :reason "other"})

and in project.clj,

(defproject eastwood-lein-clj "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "EPL-2.0 OR GPL-2.0-or-later WITH Classpath-exception-2.0"
            :url "https://www.eclipse.org/legal/epl-2.0/"}
  :dependencies [[org.clojure/clojure "1.10.1"]
                 [org.clojure/core.async "1.3.618"]]
  :repl-options {:init-ns eastwood-lein-clj.core}
  :profiles {:dev {:plugins [[jonase/eastwood "0.9.4"]]
                     }}
  :config-files ["eastwood.clj"]
  :main eastwood-lein-clj.core/main)

is here anything wrong?

thanks.

[vemv]

The :config-files should reside inside an :eastwood key, there are a few examples throughout the https://github.com/jonase/eastwood readme

Other than that the snippet looks OK

The following might also work:

(disable-warning
 {:linter :suspicious-expression
  ;; no :for-macro
  :if-inside-macroexpansion-of #{'clojure.core.async/alt!}
  :within-depth 2
  :reason "other"})

for all cases, remember to try different :within-depth values. If e.g. 1000 doesn't work then the whole thing won't work. But :within-depth should be as small as possible

[darren-mk]

it worked! thanks.

but interestingly, below is the only version that works for this purpose. specifying alt! anywhere didn't really help. also, i had to give clojure.core/or to both :for-macro and :if-inside-macroexpansion-of. if either one is commented out, the disabling doesn't work. i am not too sure about the exact difference between these two fields.

(disable-warning
 {:linter :suspicious-expression
  :for-macro 'clojure.core/or
  :if-inside-macroexpansion-of #{'clojure.core/or}
  :within-depth 2
  :reason "other"})

[vemv]

Mmm, that doesn't look correct, it likely will also disable legitimate warnings unrelated to core.async.

I'll try to provide an official fix. Normally bugfixes are shipped fast :)

[darren-mk]

yes, i agree that it will also disable legit warnings outside core.async. i tested it also.

just fyi, as included in the original post, below is my testing code. in the eastwood.clj config file, i gave go, alt!, and or in virtually all possible combinations to those two fields. it seems only or is working.

(go 
 (let [timeout-ch (timeout 1000)
       trade 100]
   (->
    (alt!
     [[trade-ch trade]] :sent
     timeout-ch :timed-out)
    print)))

[vemv]

Thanks! I'll make sure to add that snippet to the test suite.

[darren-mk]

that's awesome! thanks so much!

[vemv]

This snippet worked for the provided use case:

(disable-warning
 {:linter :suspicious-expression
  :for-macro 'clojure.core/or
  :if-inside-macroexpansion-of #{'clojure.core.async/go}
  :reason "https://github.com/jonase/eastwood/issues/411})

Could you try it and check these two...?

• the issue gets silenced
• no other core.async warnings pop up

I intend to ship this disable-warning as it passed the unit tests. I just want to know first if there's anything else to fix, LMK

Cheers - V

[darren-mk]

i just tried the same snippet with what you provided above.

(disable-warning
 {:linter :suspicious-expression
  :for-macro 'clojure.core/or
  :if-inside-macroexpansion-of #{'clojure.core.async/go}
  :reason "https://github.com/jonase/eastwood/issues/411})

but it isn't silenced. the linter says

src/eastwood_lein_clj/core.clj:19:1: suspicious-expression: or called with 1 args.  (or x) always returns x.  Perhaps there are misplaced parentheses?
src/eastwood_lein_clj/core.clj:19:1: suspicious-expression: or called with 1 args.  (or x) always returns x.  Perhaps there are misplaced parentheses?

would you want to see my repo? if so, please let me know.

[vemv]

Yes thanks! That's surprising.

[darren-mk]

oh sorry, i lied. with your disable-warning code snippet, which doesn't include :within-depth field, works fine and stays silent with the or issue within go block. but when :within-depth is added, it complains again. i got an impression that this field is mandatory, but probably i was wrong.

[vemv]

As mentioned one has to find out a specific numeric value that will work. In this case I opted for leaving it unspecified - go is a very peculiar macro.

Do any other warnings pop up?

[darren-mk]

understood. and no, there is no other warnings. i believe this is it. if you agree, i think we can close the issue.

i really appreciate your help and quick turnarounds. it was a pleasure to work with you. thank you!

[vemv]

Cheers. Tonight 0.9.5 will be released with the snippet built-in.

Don't hesitate to report further issues. Hope Eastwood serves you for a long time!

[vemv]

I released 0.9.6 just now with the mentioned fix, and also with a couple more fixes for the go macro