Fork me on GitHub
#clj-kondo
<
2020-03-12
>
serioga13:03:56

@borkdude The :missing-docstring linter warns on deftype declarations like (deftype Foo [x]) . Is it intended behavior?

borkdude13:03:14

That's probably a false positive. Issue welcome.

borkdude13:03:57

Same for deftest I see.

(ns foo
  {:clj-kondo/config {:linters {:missing-docstring {:level :warning}}}}
  (:require [clojure.test :as t]))

(deftype Foo [x])

(t/deftest foo)

serioga13:03:42

yes, deftest too single issue for both?

borkdude13:03:54

just paste that code into the issue

borkdude13:03:52

probably the same applies to this linter

serioga13:03:19

but it's possible to add docstring in both cases!

serioga13:03:29

(deftest ^{:doc "Docstring"} my-test)

serioga13:03:59

but for defrecord it useless.

borkdude13:03:20

it's not very common I think to write docstrings for tests

borkdude13:03:27

people usually use testing

serioga13:03:30

but anyway deftest has no syntax for docstring

borkdude13:03:50

yeah, I think we just are going to ignore deftest and deftype for missing docstrings

serioga13:03:57

«it's not very common I think to write docstrings for tests» I guess very uncommon

serioga14:03:05

@borkdude hm, I cannot disable :unresolved-namespace linter for specific namespaces?

borkdude14:03:40

why would you want to do that?

serioga14:03:12

I use criterium-core in comment criterium dependency is in profiles.clj I don't want import it in the file

serioga14:03:50

well, the (require '[criterium.core]) before “fixes” this issue...

borkdude14:03:57

yeah, that's the way to do it.

serioga15:03:13

Should not we lint for missing docstring for namespaces as well?

borkdude15:03:32

can do. I would say as an option to the existing linter.

borkdude15:03:48

or would you say, just enable it always for that linter?

serioga15:03:14

the option is more flexible I guess.

borkdude15:03:09

or maybe enable it by default but people can turn it off?

borkdude15:03:45

{:missing-docstring {:level :warning :namespace-declaration false}?

serioga15:03:19

I don't have enough experience to say what is better Probably library and application developers have different expectations for default settings

borkdude15:03:00

I'll think about it some more. Welcome to post an issue about this.

serioga15:03:16

Maybe it would be useful to have an option like {:profile :lib}, and defaults change accordingly

serioga15:03:36

ah, another topic was introduced later, OK

serioga15:03:51

I'll create issue

borkdude15:03:21

if people need different settings for a lib, they can just change their config accordingly

borkdude15:03:45

there isn't going to be one default that satisfies everyone

serioga15:03:38

people don't care but idea is to provide a guide for best practices for corresponding context

serioga15:03:11

at least on documentation level, as examples

serioga15:03:14

for example "missing docstring" is disabled by default. but it is no-go for libraries 🙂

serioga19:03:11

again about rum macros.

(defcs name doc-string? (< mixins+)? [ state-arg params* ] render-body+)
when I define component like
(rum/defcs hello-world
  "Example react component."
  [state name]
  "foo")
then :missing-docstring linter still complains about missing docstring. @borkdude should I file an issue?

borkdude19:03:46

what are you linting rum as?

borkdude19:03:48

what is your config?

serioga20:03:55

:lint-as {app.database.hugsql/declare-fn clojure.core/declare
           mount.core/defstate clojure.core/def
           mount.tools.macrovich/deftime clojure.core/do
           rum.core/defc clj-kondo.lint-as/def-catch-all
           rum.core/defcs clj-kondo.lint-as/def-catch-all
           rum.core/defcc clj-kondo.lint-as/def-catch-all}

serioga20:03:35

ah, without lint-as there is no message about docstring

borkdude20:03:54

that seems like a false positive then. you can append it to the existing issue.

borkdude20:03:01

including your config

serioga20:03:54

hm, without lint-as it does not detect missing docstring...

borkdude20:03:51

without lint-as it doesn't know what this macro is, so it also doesn't know it should have a docstring