Fork me on GitHub
#clj-kondo
<
2024-03-18
>
Noah Bogart15:03:30

ayyyyy that looks great

cjohansen15:03:36

And one for the other way around 😄 https://github.com/magnars/kaocha-noyoda

borkdude15:03:11

@U9MKYDN4Q it will be configurable with {:position :last} :)

cjohansen15:03:33

Interesting. Would probably be cool if clj-kondo could expand threading macros, as those could mess with several linters I guess?

borkdude15:03:49

it already expands those

Noah Bogart15:03:04

is-eq-order in splint only checks is calls

borkdude15:03:09

but that is the problem: the value you write here is the last one, but cosmetically it is the first one

cjohansen15:03:00

I guess the point of the lint rule is to help you get meaningful output from the test runner. As such, this is actually a mistake, and should probably be rewritten (e.g. the linter is correct)

Noah Bogart15:03:07

i don't think this matters much outside of is assertions

cjohansen15:03:23

ah, this isn't inside a test

cjohansen15:03:32

then I take my opinions back 😅

borkdude15:03:46

well, I do like to read the expected value first in all cases, but yeah it only technically matters inside of clojure.test/is etc

borkdude15:03:26

btw, isn't clojure.test/is more general than just =

borkdude15:03:32

e.g. (is (< 1 x))

borkdude15:03:06

user=> (let [x -1] (is (< 1 x)))

FAIL in () (NO_SOURCE_FILE:1)
expected: (< 1 x)
  actual: (not (< 1 -1))
false

Noah Bogart15:03:15

absolutely. i just wrote the splint rule for = because that's been my experience with eftest/kaocha/etc

borkdude15:03:42

well, even if you reverse it, you will get a good message right?

FAIL in () (NO_SOURCE_FILE:1)
expected: (< x 1)
  actual: (not (< 1 1))

borkdude15:03:56

so what is the problem with clojure.test/is + the position + = specifically?

Noah Bogart15:03:42

given:

(deftest example
  (let [expected {:a 1}]
    (is (= expected {:a 6}))))
with kaocha:
Expected:
  {:a 1}
Actual:
  {:a -1 +6}
1 tests, 1 assertions, 1 failures.

Noah Bogart15:03:53

vs

(deftest example
  (let [expected {:a 1}]
    (is (= {:a 6} expected))))
and
Expected:
  {:a 6}
Actual:
  {:a -6 +1}
1 tests, 1 assertions, 1 failures.

Noah Bogart15:03:35

it's just about what's printed in the "expected" vs "actual" logs

borkdude15:03:46

ok, so the problem isn't the position, but those downstream third-party tools ;)

Noah Bogart15:03:47

matcher-combinators cares about this

Noah Bogart15:03:36

sure, but the community has standardized around putting the expected value first. this is purely a style lint for test output prettiness

borkdude15:03:04

I think so too. btw I don't get the problematic difference between kaocha example 1 and 2, can you explain it to me

borkdude15:03:50

ok, I see, it displays the "expected" as the actual value, yeah this makes sense

👍 1
borkdude15:03:01

how does it behave with < etc then?

cjohansen15:03:35

I'd say it's not so much about "prettiness" as it is about clear communication of what failed

Noah Bogart15:03:56

(deftest example
  (let [expected {:a 1}]
    (is (< 2 1))))

expected: (< 2 1)
  actual: (not (< 2 1))

Noah Bogart15:03:26

kaocha only cares about =, i don't know about others

borkdude15:03:34

btw when I did my first clojure project in a company once, I also wrote (= x 10) and my then colleague told me to write (= 10 x) (2013, long before those other test runners). So I think this preference of style may also have come from other programming languages/cultures, somewhere. Since then I just stuck with it

borkdude15:03:51

2013 I mean

cjohansen15:03:52

I think it stems from junit

cjohansen15:03:06

I've been resisting it since at least 2013 😂

😂 1
borkdude15:03:24

@U9MKYDN4Q resistance is futile, join the borkiverse

😂 1
cjohansen15:03:40

I have joined the borkiverse, I just don't get on all the rides 😄

😅 2
borkdude15:03:24

that's why config exists, to not piss off everyone ;)

🎯 1
cjohansen15:03:09

And I do think the semantic significance is important. Which is why I use a plugin, as opposed to just flipping the order and not caring about the output.

cjohansen15:03:22

So I think the linter is a good idea, especially when it comes with config 😄

borkdude15:03:28

I guess I could add one more config so it only applies to clojure.test

Noah Bogart15:03:12

splint has a performance rule about using .equals when comparing against a string literal lol, i think that's off by default

borkdude15:03:36

I hate it ;)

😂 1
Noah Bogart15:03:59

yeah the performance rules are weird, they're all off by default because they lead to strange code

borkdude15:03:13

they will only lead to faster code on the JVM, not in bb btw ;)

borkdude15:03:35

this goes for most perf things

borkdude15:03:40

JVM:

user=> (time (dotimes [i 1000000000] (.equals "foo" "foo")))
"Elapsed time: 16.448459 msecs"
bb:
user=> (time (dotimes [i 1000000000] (.equals "foo" "foo")))
loading see you tomorrow...

😂 4
cjohansen15:03:38

Why is that?

borkdude15:03:12

because bb uses SCI , which is a clojure interpreter and interop is based on reflection.

Ben Sless17:03:55

This is some deep jvm lore, but will having a static value as the first argument eventually get optimized better by the JIT compiler?

borkdude17:03:47

I guess it depends which JVM you ask :)

Ben Sless17:03:22

11, 17, 21 x GraalVM

borkdude16:03:10

ok, merged this linter to master, feel free to try it out

🎉 1