Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide all metadata from vars analysis #1280

Closed
latacora-gabriel opened this issue May 11, 2021 · 17 comments
Closed

Provide all metadata from vars analysis #1280

latacora-gabriel opened this issue May 11, 2021 · 17 comments

Comments

@latacora-gabriel
Copy link

Hi @borkdude. Once again thanks for your awesome tool!

Is your feature request related to a problem? Please describe.
I'd like to be able to mark vars with metadata and the filter vars with that metadata using analysis' :var-definitions in a bb script. With this feature, I could write bb scripts that generate configuration for tools that have var configuration. For example, I could generate a carve ignore list by marking fns with ^:repl

Describe the solution you'd like
It'd be helpful for a var in :var-definitions to include all the metadata it contains. This metadata could be at the top level or under a :meta key if that seems cleaner. It seems analysis obtains a var's metadata and then only provides some of that metadata as analysis data.

Describe alternatives you've considered
I think :var-definitions is the most appropriate place to contains this analysis data. I don't know of other tools that could provide this data as effectively in a bb script

@borkdude
Copy link
Member

@latacora-gabriel Interesting. An alternative approach could be to just iterate over all vars using (all-ns) and ns-publics and then filter all the ones that have :repl metadata. Is there a reason you prefer using clj-kondo for this? I'm not suggesting you shouldn't, just trying to learn more about your use case.

@latacora-gabriel
Copy link
Author

Unfortunately that won't work b/c I'm looking to do this analysis with bb and most of the namespaces I want to analyze aren't bb compatible

@borkdude
Copy link
Member

borkdude commented May 14, 2021

One of the problems here is that not all metadata on vars is valid EDN since you can put arbitrary objects on there. Perhaps we can make a configurable filter of a list of keys that people can provide:

{:config {:output {:analysis {:var-meta {:select-keys [:repl]}}}}}

?

@latacora-gabriel
Copy link
Author

latacora-gabriel commented May 14, 2021 via email

@borkdude
Copy link
Member

borkdude commented Sep 2, 2021

As discussed with @IGJoshua:

Add option {:analysis {:var-definitions {:metadata true}}} to collect all metadata of constant keys and constant vals (dynamic values are ignored to guarantee correct EDN output).

We need one extra :meta key or so here which will be processed when the option is provided.

(let [attrs (select-keys attrs [:private :macro :fixed-arities :varargs-min-arity

We could also implement {:var-usages {:metadata #{:style/indent}}} to only collect certain keys.

@borkdude
Copy link
Member

borkdude commented Oct 12, 2021

OK, I thought more about it. I think we should let the user do the selection and validation they want in a function and support:

{:analysis {:var-definitions {:meta (fn [m] ...)}}

So you can just select the extra metadata you want and the burden of validation is on the consumer. We can support this option on the command line too by evaluation the function in SCI. Passing a boolean true means pass all the metadata.

The selected metadata will end up in {:var-definitions [{:name ... :meta ...}].

@borkdude
Copy link
Member

/cc @lread

@lread
Copy link
Contributor

lread commented Oct 12, 2021

This looks good to me. Would this also grab metadata from ns defs?

@borkdude
Copy link
Member

Yes, defs are also :var-definitions.

@lread
Copy link
Contributor

lread commented Oct 12, 2021

Sorry, a horrible choice of wording on my part!

I mean metadata on namespaces like so:

(ns ^:no-doc my-ns.here)

@IGJoshua
Copy link
Contributor

Going that direction sounds great, but I think unless I missed something is beyond what I know of how clj-kondo works, so it might be best (especially with my time constraints recently) if someone else were to take over making a PR for this.

@borkdude
Copy link
Member

@IGJoshua That is fine, thank you for your work so far!

@borkdude
Copy link
Member

borkdude commented Oct 12, 2021

@lread We can support a similar :meta fn for :namespace-definitions.

@lread
Copy link
Contributor

lread commented Oct 14, 2021

@borkdude and I decided to do a Clojure core team style spreadsheet to go over options, we think it went pretty well.
Here's a PDF of what we came up with:
clj-kondo meta export - Sheet1.pdf

@borkdude
Copy link
Member

@latacora-gabriel This should now be available on master.

See https://github.com/clj-kondo/clj-kondo/blob/master/analysis/README.md#extra-analysis

Please test it out if you have time. Should be available as pod after next clj-kondo release.

@borkdude
Copy link
Member

@latacora-gabriel OK, released and pod was also updated. Let me know how it goes.

@latacora-gabriel
Copy link
Author

@borkdude I've been away for awhile. Confirmed this works great with cldwalker/bb-clis@3ad1b4a. Thanks! Example use on the babashka repo:

bbg var-meta src | bb-table
+------------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| var                                                        | meta                                                                                                                                                                                                            |
+------------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| babashka.impl.clojure.test/load-tests                      | {:doc "True by default.  If set to false, no test functions will\n   be created by deftest, set-test, or with-test.  Use this to omit\n   tests when compiling or loading production code."}                    |
| babashka.impl.clojure.test/stack-trace-depth               | {:doc "The maximum depth of stack traces to print when an Exception\n  is thrown during a test.  Defaults to nil, which means print the\n  complete stack trace."}                                              |
| babashka.impl.clojure.test/with-test-out-internal          | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/testing-vars-str                | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/testing-contexts-str            | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/inc-report-counter              | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/report-impl                     | {:added "1.1", :dynamic true, :doc "Generic reporting function, may be overridden to plug in\n   different report formats (e.g., TAP, JUnit).  Assertions such as\n   'is' call 'report' to indicate results... |
| babashka.impl.clojure.test/do-report                       | {:added "1.2"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/get-possibly-unbound-var        | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/function?                       | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/assert-predicate                | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/assert-any                      | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/try-expr                        | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/is                              | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/are                             | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/testing                         | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/with-test                       | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/deftest                         | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/deftest-                        | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/set-test                        | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/ns->fixtures                    | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.test/add-ns-meta                     | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/use-fixtures                    | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/default-fixture                 | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/compose-fixtures                | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/join-fixtures                   | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/test-var-impl                   | {:added "1.1", :dynamic true}                                                                                                                                                                                   |
| babashka.impl.clojure.test/test-vars                       | {:added "1.6"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/test-all-vars                   | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/test-ns                         | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/run-tests                       | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/run-all-tests                   | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.test/successful?                     | {:added "1.1"}                                                                                                                                                                                                  |
| babashka.impl.clojure.data/diff                            | {:added "1.3"}                                                                                                                                                                                                  |
| babashka.impl.clojure.core.server/*session*                | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/lock                     | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/servers                  | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/with-lock                | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/thread                   | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/prepl                    | {:added "1.10"}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.server/io-prepl                 | {:added "1.10"}                                                                                                                                                                                                 |
| babashka.impl.clojure.core.async/executor                  | {:tag java.util.concurrent.Executor}                                                                                                                                                                            |
| babashka.impl.clojure.spec.test.alpha/*instrument-enabled* | {:private true, :dynamic true}                                                                                                                                                                                  |
| babashka.impl.clojure.spec.test.alpha/instrumented-vars    | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.gen.alpha/quick-check-ref       | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.gen.alpha/for-all*-ref          | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.gen.alpha/delay-impl            | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.gen.alpha/lazy-combinator       | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.gen.alpha/lazy-combinators      | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.gen.alpha/lazy-prim             | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.gen.alpha/lazy-prims            | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.gen.alpha/gen-builtins          | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/*recursion-limit*         | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/*fspec-iterations*        | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/*coll-check-limit*        | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/*coll-error-limit*        | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/registry-ref              | {:private true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/*explain-out*             | {:dynamic true}                                                                                                                                                                                                 |
| babashka.impl.clojure.spec.alpha/def-impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/map-spec-impl             | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/spec-impl                 | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/multi-spec-impl           | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/tuple-impl                | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/or-spec-impl              | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/and-spec-impl             | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/merge-spec-impl           | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/every-impl                | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/cat-impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/rep-impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/rep+impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/amp-impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/alt-impl                  | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/maybe-impl                | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/regex-spec-impl           | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/fspec-impl                | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/nonconforming             | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/nilable-impl              | {:skip-wiki true}                                                                                                                                                                                               |
| babashka.impl.clojure.spec.alpha/*compile-asserts*         | {:dynamic true, :doc "If true, compiler will enable spec asserts, which are then\nsubject to runtime control via check-asserts? If false, compiler\nwill eliminate all spec assert overhead. See 'assert'.\n... |
| babashka.impl.clojure.core/sync                            | {:added "1.0"}                                                                                                                                                                                                  |
| babashka.impl.clojure.java.io/as-relative-path             | {:tag String, :added "1.2"}                                                                                                                                                                                     |
| babashka.impl.clojure.java.io/file                         | {:tag java.io.File, :added "1.2"}                                                                                                                                                                               |
| babashka.impl.clojure.main/repl-requires                   | {:doc "A sequence of lib specs that are applied to `require`\nby default when a new command-line REPL is started."}                                                                                             |
| babashka.impl.pprint/formatter-out                         | {:added "1.2"}                                                                                                                                                                                                  |
+------------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants