Use clj-kondo :seqable for :+, :*, and :? not {:op :rest}
#952
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update clj-kondo type generation. `{:op :rest}` should be used only for varargs.
```
This can be used to match remaining arguments in vararg signatures.
https://github.com/clj-kondo/clj-kondo/blob/master/doc/types.md
```
ikitommi
requested changes
Sep 10, 2023
There was a problem hiding this comment.
Thanks for the PR! I think there are two use cases here:
- when used in function variable arguments,
{:op :rest ...}should be used. - when used in any other place,
:seqableshould be used
e.g. the accept should know which of the two use cases it is and return accordingly.
src/malli/clj_kondo.cljc
Outdated
| (defmethod accept :? [_ _ [child] _] {:op :rest, :spec child}) | ||
| (defmethod accept :+ [_ _ _ _] :seqable) | ||
| (defmethod accept :* [_ _ _ _] :seqable) | ||
| (defmethod accept :? [_ _ _ _] :seqable) | ||
| (defmethod accept :repeat [_ _ [child] _] {:op :rest, :spec child}) |
There was a problem hiding this comment.
:repeat is also one of the sequence schemas that can present more than 1 values.
| @@ -63,11 +75,27 @@ | |||
| {'kikka | |||
| {:arities {1 {:args [:int], | |||
| :ret :int}, | |||
| :varargs {:args [:int :int {:op :rest, :spec :int}], | |||
There was a problem hiding this comment.
This is a real varargs case and should generate :rest definition instead :seqable. From clj-kondo:
{:op :rest, :spec :int}. This can be used to match remaining arguments in vararg signatures. | :ret :int, | ||
| :min-arity 2}}} | ||
| 'siren | ||
| {:arities {2 {:args [:ifn :coll], :ret :map}}}}}] |
tvaisanen
force-pushed
the
fix/820/clj-kondo-type-export
branch
from
cee4aeb
to
ed29740
Compare
- use arity to decide whether to use `{:op :rest}` or `:seqable`
tvaisanen
force-pushed
the
fix/820/clj-kondo-type-export
branch
from
ed29740
to
68d1a82
Compare
ikitommi
approved these changes
Sep 23, 2023
src/malli/clj_kondo.cljc
Outdated
| {:op :rest :spec child} | ||
| :seqable)) | ||
|
|
||
| (defmethod accept :+ [_ _ children options] (-seqable-or-rest nil nil children options)) |
There was a problem hiding this comment.
no need to pass the extra nil values here?
There was a problem hiding this comment.
Updated the required args.
no need to match accept argument signature use only children and opts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update clj-kondo type generation.
{:op :rest}should be used only for varargs.Setup for validating the changes with clj-kondo: https://github.com/tvaisanen/clj-kondo-malli-error-2022-12-20
closes #820