Fork me on GitHub
#clj-kondo
<
2019-10-02
>
Hannu Hartikainen18:10:49

I feel I’m starting to somewhat understand the type checking. What’s the reason to keep :seqable-out separate from :seqable, though?

mynomoto18:10:02

@borkdude could you please release a new version? I want to update to the latest but the false positives on types for partition and conj are stopping me.

borkdude18:10:09

@mynomoto There's one last bit I want to tweak about a new linter and then I'll release

borkdude18:10:53

The challenge with the new unused-private-var linter is that some private fns defined in .cljc/.cljs may be used in .cljs/.cljc, so it's not just a matter of checking all defs/calls in one file

borkdude19:10:09

I guess joker also just does that, since it lints one file at a time. So maybe it would be OK if you could exclude certain private fns from being reported

borkdude19:10:57

@hannu.hartikainen The reason for seqable-out is that I want to exclude nil and string that are normally seqable. So you should not rely on a seqable-out function to be able to produce nil or a string

borkdude19:10:33

this leads to better error messages

Hannu Hartikainen19:10:15

@borkdude good to know. There’s already a bug with next and the functions that use it, then 🙂

borkdude19:10:16

since you might have an argument type where nil could be a valid value and then seqable would match, while the non-nilable variant of that type could not be a seqable at all

borkdude19:10:48

can you show an example please

Hannu Hartikainen19:10:33

it’s just that next can return nil

borkdude19:10:37

I don't think that will lead to a false positive in practice. Excluding string is probably the most pressing reason, to still be able to output errors like:

$ clj -A:clj-kondo --lint - <<< '(defn foo [^String _x]) (foo (next [1 2 3]))'
<stdin>:1:30: warning: Expected: string or nil, received: seqable collection.

borkdude19:10:05

if next produced a seqable it COULD produce a string, so this would be valid

borkdude19:10:59

it sounds a bit confusing I know, but I think starting with a false positive is better than just changing it

borkdude19:10:36

a seqable-out is a seqable and when you call next you expect a seqable back, so that mostly works. there aren't any functions expecting explicitly a nil value.

borkdude19:10:51

if you can come up with an example where this doesn't play out, I'd be curious.

Hannu Hartikainen19:10:20

yes, that’s absolutely true

Hannu Hartikainen19:10:42

but when in the future someone extends the type system to give warnings of missing nil checks, that wrong type will cause false negatives

Hannu Hartikainen19:10:45

so it’s not very worrying but I’ll change to :nilable/seqable-out someday… not today though

borkdude19:10:09

I guess making seqable-out contain nil will probably work out everywhere btw, but not doing that has probably the same effect in terms of linting, since there are no functions requiring an explicit nil anywhere

borkdude19:10:02

but turning next into seqable -> seqable (instead of seqable-out) will stop this from warning:

clj -A:clj-kondo --lint - <<< '(defn foo [^String _x]) (foo (next [1 2 3]))'
linting took 82ms, errors: 0, warnings: 0
Just tried that. So let's forget about what I said about nil, seqable-out is about excluding strings.

borkdude19:10:14

There is more to this "type system" that's not always telling the truth for a 100%, but it's made with linting in mind. Like:

$ clj -A:clj-kondo --lint - <<< '(reduce 1 1 1)'
<stdin>:1:9: warning: Expected: function, received: positive integer.
<stdin>:1:13: warning: Expected: seqable collection, received: positive integer.
Actually reduce expects a reducible, not a seqable. But in practice this works well enough so far.

borkdude20:10:19

@mynomoto if you want to use the latest, you can do that and turn the type checking off like any other linter. or override the types for problematic functions in your config. like so: https://github.com/borkdude/clj-kondo/issues/470

borkdude20:10:38

but I expect a new release within a few days hopefully (no promises)

borkdude21:10:52

@mynomoto oh, and if I can make you happy with a binary from master, I think you were using linux right? https://6255-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.09.23-alpha-SNAPSHOT-linux-amd64.zip

mynomoto22:10:23

@borkdude I will try it thanks, but to run on ci I need a proper release. I'm happy to wait, if I do configuration to ignore something it tends to be forgotten 😕

borkdude22:10:35

are you running with a binary or from java in CI?

borkdude22:10:13

that's fine. in CI you could also use deps.edn and then use a commit SHA, if you don't want to wait for the next binary

mynomoto22:10:08

I can wait, but thanks for the options. Test worked, all false positives are gone. Thanks!