Consider adding hook support for edn files, re #1392

[lread]

A place to discuss, flesh out, then scope #1392
All ideas presented are just that, critique, corrections and brainstorming are most welcome!

[lread]

Topic: Edn hook registration

An edn hook will be registered in clj-kondo config.
Same idea as existing hooks.
We can flesh out any specifics here.

[lread]

Topic: Edn hook activation

How will clj-kondo know if an edn hook should lint an edn file?
I think only the edn hook can decide that.

Initial ideas:

1. clj-kondo calls a single edn hook function, the hook function can look at the filename and rewrite-clj nodes and decide to process or not.
2. clj-kondo calls a matcher edn hook function, if this returns true, the clj-kondo calls a linter edn hook function. More complicated for little gain? Probably, but including it perhaps in order to reject it.
3. ?

Other thoughts:

1. Any number of edn hooks can participate in linting, just because edn hook a is interested in linting file x, doesn't mean edn hook b won't also be interested.
2. I don't think clj-kondo will actively go out looking for edn files to lint, if the edn file is not specifically included in the --lint list, it won't be linted.

[lread]

Does the hook receive the full path? That might be exposing too much to the hook.
Then again a hook might find the file path, or at least parts of the path, useful.

[lread]

If you tell clj-kondo to lint an edn file, it will do some base checks on that edn.
For example, if there is a map that has a key with no value, ex: {:a}, it will report that.
It might be interesting for a hook to know if clj-kondo has already found issues in its base checks.
For example, a hook might not want to deal with what is effectively invalid edn.

[lread]

Small detail: the current built-in edn linters receive only the first form in an edn file.
I think that an edn hook should receive all forms. That way it can decide if an extra form is an issue or not.

[lread]

Topic: use case: cljdoc.edn

Exploring specific real-world edn files and what linting they might benefit from is interesting.

The cljdoc.edn file characteristics of interest:

• cljdoc looks for this file under doc and docs directory off the git repo root.
• contains a doc tree which references git repo root relative markdown and asciidoc files.
• can optionally specify a list of group-id/artifact-id projects to be included as part of imported docs.

[lread]

By design, clj-kondo hooks are constrained so that they cannot do harmful things.

The cljdoc edn hook will probably want to check if files exists.
And maybe (but maybe not, we'll see) want to check if group-id/artificat-id projects exist.

[lread]

Let's look at the doc file existence check.
What is safe for clj-kondo to allow?

• Allowing a hook to check the existence of any file seems a bit on the loose side and might legitimately raise eyebrows.
• If clj-kondo can limit file existence checks to the files in a project, I expect that would be acceptable. But I'm not sure what concepts of projects clj-kondo currently has.

[lread]

Let's say I want to check that cljdoc.edn is under either a doc or docs directory.
Can I check that?
Does the filename the edn hook is delivered also include its path, or a portion of the path?

[lread]

Expected usages of cljdoc.edn linting:

1. by library authors via editor integrations
2. by library authors clj-kondo ad-hoc on command line
3. by library authors in CI scripts to fail build and/or release if cljdoc.edn has issues
4. by cljdoc server to validate cljdoc.edn on library import, some findings will fail the import, others will be informational

[lread]

Topic: use case: kaocha tests.edn

Characteristics of interest:

• filename is on the generic side, but maybe that's ok? Just like deps.edn has a special meaning maybe tests.edn does too? Do we know of other folks using tests.edn for other things?
• kaocha supports and recommends use of shorthand syntax, example:

  #kaocha/v1
  {:tests [{:id          :unit
            :test-paths  ["test" "src"]
            :ns-patterns [".*"]}]}

• this implies that a v2 of kaocha, will likely continue to use tests.edn
• long-hand syntax is, of course, still supported
• if we want to use content in addition to filename to be relatively confident that we are looking at a kaocha config, the content is quite distinct in both shorthand and longhand variants.
• it includes support for aero reader conditionals such as #meta-merge, #profile, #include etc.
• supports plugins, which could have their own linting requirements.
• can select tests by namespace or namespace pattern
• can specify source and test directories
• ?

[lread]

Topic: Export/Import edn hook support

Current clj-kondo hooks can be exported by library authors to the benefit of folks using those libraries.

1. An edn hook might not have an associated library.
   For example, cljdoc-edn. But I suppose a library would be created, if only for the purpose of delivering the hook, yeah?

2. Current clj-kondo hooks are automatically activated.
   Should an edn hook be also activated automatically?
   For example, if I am using the kaocha lib, should its edn hook be automatically activated for clj-kondo?

[lread]

Topic: The hooks API

The current hooks API offers support for working with rewrite-clj nodes, looking up analysis data, and registering findings.

As we explore and experiment, we'll likely find some new API functions that are needed, and others that are not absolutely needed, but helpful when working to lint edn files.

We'll have to add what is needed, but should probably be conservative, at least initially, in adding in nice-to-haves.

[lread]

Which makes me wonder: can a hook require a minimum version of clj-kondo to run? Perhaps the hook API evolves in clj-kondo version z but a user is still using clj-kondo w.

Perhaps it could check the clj-kondo version and emit a finding (of a type that is not ignored by default) if the version does not meet its needs.

[lread]

Tangential but related: reporting

In some cases, like cljdoc.edn validation on CI, it might be nice to show a shadow-cljs style error report instead of the existing brief clj-kondo terminal output. Given the following hypothetical findings:

cljdoc.edn:10:58:cljdoc.doc/tree doc entry attributes :file > file not found
cljdoc.edn:14:54:cljdoc.doc/tree doc entry attributes map > must only use :file
cljdoc.edn:17:55:cljdoc/include-namespaces-from-dependencies entry > must a group-id/artifact-id symbol

Here's the same in a format for custom stand-alone cljdoc.edn validator I was experimenting with recently:

-[ERROR #1]-----------------------------------------------------------------------------------------
   7 |                    ;;  ["Record Architecture Decisions" {:file "doc/adr/0001-record-architecture-decisions.md"}]
   8 |                    ;;  ]
   9 |                    ["Userguide" {}
  10 |                     ["For Library Authors" {:file "doc/userguide/for-library-author.adoc"}]
---------------------------------------------------------^------------------------------------------
 :cljdoc.doc/tree doc entry attributes :file > file not found
----------------------------------------------------------------------------------------------------

-[ERROR #2]-----------------------------------------------------------------------------------------
  11 |                     ["For Library Users" {:file "doc/userguide/for-users.md"}]
  12 |                     ["Articles" {:file "doc/userguide/articles.md"}]
  13 |                     ["FAQ" {:file "doc/userguide/faq.md"}]]
  14 |                    ["Running cljdoc Locally" {:fie "doc/running-cljdoc-locally.adoc"}]
-----------------------------------------------------^----------------------------------------------
 :cljdoc.doc/tree doc entry attributes map > must only use :file
----------------------------------------------------------------------------------------------------

-[ERROR #3]-----------------------------------------------------------------------------------------
  14 |                    ["Running cljdoc Locally" {:fie "doc/running-cljdoc-locally.adoc"}]
  15 |                    ["Developer Technical Guide" {:file "doc/cljdoc-developer-technical-guide.adoc"}]
  16 |                    ["Custom Articles" {:file "doc/custom-articles.adoc"}]]
  17 |  :cljdoc/include-namespaces-from-dependencies ["foobar"]
------------------------------------------------------^---------------------------------------------
 :cljdoc/include-namespaces-from-dependencies entry > must a group-id/artifact-id symbol
----------------------------------------------------------------------------------------------------

One could argue,

• when the user sees the CI lint warning/error, they could bring up the file in their editor and get a pretty clear picture of what went wrong.
• that my hypothetical errors could include the errant value, but I prefer not to do that as the errant value might be anything (something very large for example).

[borkdude]

Now having read the above, I think a config like:

{:hooks {:edn-file [{:name :cljdoc.edn :file-regex "..." :fn org.cljdoc/config-hook}]}}

would work. We'll have to figure out if regexes work well with Windows file paths too, but regexes are already used with {:output {:exclude-files ...} as well.

An .edn file will be matched against any of these regexes, and then the corresponding :fns will be invoked.

The hook will see a map with {:filename ... :node ...} and get the default hooks api to begin with. We can add more stuff by need.

Additionally, for custom rules, a library has to export for default settings of their custom rules.

{:linters {:cljdoc/cljdoc.edn-missing-foobar {:level :warning}
           :cljdoc/cljdoc.edn-consistent-quux {:level :warning}}

This is already the case with the other hooks as well. Hooks do not have access to the file system and I don't see a need for that to change for this specific feature.

[lread]

We'll have to figure out if regexes work well with Windows file paths too

The file separators can be awkward to deal with.
One solution is to always have users specify their paths with forward slashes.
Here's an example of doing that.

[lread]

Hooks do not have access to the file system and I don't see a need for that to change for this specific feature

As discussed in chat, also checking for file existence is interesting to explore but we'll have to be careful about the security aspect of doing so.

If edn hooks don't offer anything above checking the rewrite-clj nodes that represent the source edn, I suppose they could be considered functionally equivalent to a malli or clojure specs. (Except that the edn hook will pinpoint issues down to the row/col in the source edn). This is not necessarily a bad thing, just an observation.

[borkdude]

Clj-kondo provides feedback directly in the editor using static analysis, which is (often) not the case with malli or spec, so far.

[lread]

Yes, good distinction, thanks.

Was not trying to suggest that malli or spec be used instead, just trying to feel out the potential functional scope of the edn hook.