Skip to content

info and eldoc ops: accept a Compliment-style context parameter #815

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Sep 21, 2023
Merged

info and eldoc ops: accept a Compliment-style context parameter #815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
65c34d9
`info` and `eldoc` ops: accept a Compliment-style `context` parameter
vemv Sep 20, 2023
2010733
Revert "Use mranderson more cleanly"
vemv Sep 21, 2023
635ea13
Improve `dump-version` Make task
vemv Sep 21, 2023
e9ceb76
Add a docstring
vemv Sep 21, 2023
1313661
Bump Compliment
vemv Sep 21, 2023
94cd87a
Solve mranderson <-> kondo conflict
vemv Sep 21, 2023
614df43
Add extra cautions around mranderson
vemv Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ out
*.class
.cljs_nashorn_repl
.inline-deps
.no-pedantic
.lein-deps-sum
.lein-failures
.lein-plugins
.lein-repl-history
.nrepl-port
.source-deps
.clj-kondo/.cache
/.no-mranderson
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
* `info` and `eldoc` ops: also return `:doc-fragments`, `:doc-first-sentence-fragments`, `:doc-first-sentence-fragments` attributes when available.
* These are explained in https://github.com/clojure-emacs/orchard/blob/v0.15.0/src-newer-jdks/orchard/java/parser_next.clj#L2-L20
* These typically are only available when running a modern JDK through enrich-classpath.
* `info` and `eldoc` ops: accept a Compliment-style `context` parameter that helps infering the Java class of the object being queried.
* Bump `orchard` to [1.5.0](https://github.com/clojure-emacs/orchard/blob/v0.15.0/CHANGELOG.md#0150-2023-09-20).
* Bump `compliment` to [0.4.2](https://github.com/alexander-yakushev/compliment/blob/f7d872d/CHANGELOG.md#042-2023-09-17).
* Bump `compliment` to [0.4.3](https://github.com/alexander-yakushev/compliment/blob/0.4.3/CHANGELOG.md#043-2023-09-21).

## 0.37.1 (2023-09-08)

Expand Down
35 changes: 22 additions & 13 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ test/resources/cider/nrepl/clojuredocs/export.edn:
dump-version:
echo '"$(PROJECT_VERSION)"' > resources/cider/nrepl/version.edn

.inline-deps: dump-version clean
lein with-profile -user,-dev,+mranderson inline-deps
touch .inline-deps

inline-deps: .inline-deps
.inline-deps: project.clj clean
rm -f .no-mranderson
lein with-profile -user,-dev inline-deps
touch $@

test: clean .inline-deps test/resources/cider/nrepl/clojuredocs/export.edn
lein with-profile -user,-dev,+$(CLOJURE_VERSION),+test,+mranderson,+plugin.mranderson/config test
rm -f .no-mranderson
lein with-profile -user,-dev,+$(CLOJURE_VERSION),+test,+plugin.mranderson/config test

quick-test: clean
lein with-profile -user,-dev,+$(CLOJURE_VERSION),+test test
Expand All @@ -44,33 +44,42 @@ cljfmt:

.make_kondo_prep: project.clj .clj-kondo/config.edn
touch .no-pedantic
touch .no-mranderson
lein with-profile -user,-dev,+test,+clj-kondo,+deploy,+$(CLOJURE_VERSION) clj-kondo --copy-configs --dependencies --lint '$$classpath' > $@
rm .no-pedantic
rm -f .no-pedantic
rm -f .no-mranderson

kondo: .make_kondo_prep clean
touch .no-pedantic
touch .no-mranderson
lein with-profile -user,-dev,+test,+clj-kondo,+deploy,+$(CLOJURE_VERSION) clj-kondo
rm .no-pedantic
rm -f .no-pedantic
rm -f .no-mranderson

# A variation that does not analyze the classpath, as it OOMs otherwise on CircleCI.
light-kondo: clean
touch .no-pedantic
touch .no-mranderson
lein with-profile -user,-dev,+test,+clj-kondo,+deploy,+$(CLOJURE_VERSION) clj-kondo
rm .no-pedantic
rm -f .no-pedantic
rm -f .no-mranderson

lint: kondo cljfmt eastwood

# PROJECT_VERSION=0.37.1 make install
install: check-install-env .inline-deps
install: dump-version check-install-env .inline-deps
rm -f .no-mranderson
touch .no-pedantic
lein with-profile -user,-dev,+$(CLOJURE_VERSION),+mranderson,+plugin.mranderson/config install
lein with-profile -user,-dev,+$(CLOJURE_VERSION),+plugin.mranderson/config install
touch .no-pedantic
make clean
git checkout resources/cider/nrepl/version.edn

# PROJECT_VERSION=0.37.1 make fast-install
fast-install: dump-version check-install-env
lein with-profile -user,-dev,+$(CLOJURE_VERSION) install
make clean
git checkout resources/cider/nrepl/version.edn

smoketest: install
cd test/smoketest && \
Expand All @@ -87,7 +96,8 @@ detect_timeout:
# Deployment is performed via CI by creating a git tag prefixed with "v".
# Please do not deploy locally as it skips various measures (particularly around mranderson).
deploy: check-env .inline-deps
lein with-profile -user,+$(CLOJURE_VERSION),+mranderson,+plugin.mranderson/config deploy clojars
rm -f .no-mranderson
lein with-profile -user,+$(CLOJURE_VERSION),+plugin.mranderson/config deploy clojars

check-env:
ifndef CLOJARS_USERNAME
Expand All @@ -109,7 +119,6 @@ clean:
lein with-profile -user clean
cd test/smoketest && lein with-profile -user clean
rm -f .inline-deps
git checkout resources/cider/nrepl/version.edn

# Create and cache a `java` command. project.clj is mandatory; the others are optional but are taken into account for cache recomputation.
# It's important not to silence with step with @ syntax, so that Enrich progress can be seen as it resolves dependencies.
Expand Down
26 changes: 20 additions & 6 deletions doc/modules/ROOT/pages/nrepl-api/ops.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,20 @@ Returns::
Return a map of information about the specified symbol.

Required parameters::
{blank}

Optional parameters::
* `:class` A Java class. If ``:ns`` is passed, it will be used for fully-qualifiying the class, if necessary.
* `:context` A Compliment completion context, just like the ones already passed for the "complete" op,
with the difference that the symbol at point should be entirely replaced by "\__prefix__".

For Java interop queries, it helps inferring the precise type of the object the ``:sym`` or ``:member`` refers to,
making the results more accurate (and less numerous).
* `:member` A Java class member.
* `:ns` The current namespace
* `:sym` The symbol to lookup


Optional parameters::
{blank}

Returns::
* `:doc-block-tags-fragments` May be absent. Represent the 'param', 'returns' and 'throws' sections a Java doc comment. It's a vector of fragments, where fragment is a map with ``:type`` ('text' or 'html') and ``:content`` plain text or html markup, respectively
* `:doc-first-sentence-fragments` May be absent. Represents the first sentence of a Java doc comment. It's a vector of fragments, where fragment is a map with ``:type`` ('text' or 'html') and ``:content`` plain text or html markup, respectively
Expand Down Expand Up @@ -434,13 +441,20 @@ Returns::
Return a map of information about the specified symbol.

Required parameters::
{blank}

Optional parameters::
* `:class` A Java class. If ``:ns`` is passed, it will be used for fully-qualifiying the class, if necessary.
* `:context` A Compliment completion context, just like the ones already passed for the "complete" op,
with the difference that the symbol at point should be entirely replaced by "\__prefix__".

For Java interop queries, it helps inferring the precise type of the object the ``:sym`` or ``:member`` refers to,
making the results more accurate (and less numerous).
* `:member` A Java class member.
* `:ns` The current namespace
* `:sym` The symbol to lookup


Optional parameters::
{blank}

Returns::
* `:doc-block-tags-fragments` May be absent. Represent the 'param', 'returns' and 'throws' sections a Java doc comment. It's a vector of fragments, where fragment is a map with ``:type`` ('text' or 'html') and ``:content`` plain text or html markup, respectively
* `:doc-first-sentence-fragments` May be absent. Represents the first sentence of a Java doc comment. It's a vector of fragments, where fragment is a map with ``:type`` ('text' or 'html') and ``:content`` plain text or html markup, respectively
Expand Down
23 changes: 15 additions & 8 deletions project.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
^:inline-dep [thunknyc/profile "0.5.2"]
^:inline-dep [mvxcvi/puget "1.3.4"]
^:inline-dep [fipp ~fipp-version] ; can be removed in unresolved-tree mode
^:inline-dep [compliment "0.4.2"]
^:inline-dep [compliment "0.4.3"]
^:inline-dep [org.rksm/suitable "0.5.0" :exclusions [org.clojure/clojurescript]]
^:inline-dep [cljfmt "0.9.2" :exclusions [org.clojure/clojurescript]]
^:inline-dep [org.clojure/tools.namespace "1.3.0"]
Expand All @@ -28,6 +28,17 @@
;; :pedantic? can be problematic for certain local dev workflows:
false)

;; mranderson cannot be put in a profile (as the other plugins),
;; so we conditionally disable it, because otherwise clj-kondo cannot run.
:plugins ~(if (-> ".no-mranderson" java.io.File. .exists)
[]
'[[thomasa/mranderson "0.5.4-SNAPSHOT"]])

:mranderson {:project-prefix "cider.nrepl.inlined.deps"
:overrides {[mvxcvi/puget fipp] [fipp ~fipp-version]} ; only takes effect in unresolved-tree mode
:expositions [[mvxcvi/puget fipp]] ; only takes effect unresolved-tree mode
:unresolved-tree false}

:filespecs [{:type :bytes :path "cider/cider-nrepl/project.clj" :bytes ~(slurp "project.clj")}]

:source-paths ["src"]
Expand All @@ -45,7 +56,9 @@
true))))
:debugger :debugger}

:aliases {"docs" ["with-profile" "+maint" "run" "-m" "cider.nrepl.impl.docs" "--file"
:aliases {"bump-version" ["change" "version" "leiningen.release/bump-version"]
"mranderson" ["with-profile" "+plugin.mranderson/config"]
"docs" ["with-profile" "+maint" "run" "-m" "cider.nrepl.impl.docs" "--file"
~(clojure.java.io/as-relative-path
(clojure.java.io/file "doc" "modules" "ROOT" "pages" "nrepl-api" "ops.adoc"))]}

Expand All @@ -71,12 +84,6 @@
[com.google.code.findbugs/jsr305 "3.0.2"]]
:test-paths ["test/spec"]}

:mranderson {:plugins [[thomasa/mranderson "0.5.4-SNAPSHOT"]]
:mranderson {:project-prefix "cider.nrepl.inlined.deps"
:overrides {[mvxcvi/puget fipp] [fipp ~fipp-version]} ;; only takes effect in unresolved-tree mode
:expositions [[mvxcvi/puget fipp]] ;; only takes effect unresolved-tree mode
:unresolved-tree false}}

:1.8 {:dependencies [[org.clojure/clojure "1.8.0"]
[org.clojure/clojurescript "1.10.520" :scope "provided"]
[javax.xml.bind/jaxb-api "2.3.1" :scope "provided"]]}
Expand Down
17 changes: 13 additions & 4 deletions src/cider/nrepl.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,18 +213,27 @@ Depending on the type of the return value of the evaluation this middleware may
"doc-first-sentence-fragments" (str "May be absent. Represents the first sentence of a Java doc comment. " fragments-desc)
"doc-block-tags-fragments" (str "May be absent. Represent the 'param', 'returns' and 'throws' sections a Java doc comment. " fragments-desc)})

(def info-params
{"sym" "The symbol to lookup"
"ns" "The current namespace"
"context" "A Compliment completion context, just like the ones already passed for the \"complete\" op,
with the difference that the symbol at point should be entirely replaced by \"__prefix__\".

For Java interop queries, it helps inferring the precise type of the object the `:sym` or `:member` refers to,
making the results more accurate (and less numerous)."
"class" "A Java class. If `:ns` is passed, it will be used for fully-qualifiying the class, if necessary."
"member" "A Java class member."})

(def-wrapper wrap-info cider.nrepl.middleware.info/handle-info
(cljs/requires-piggieback
{:requires #{#'session}
:handles {"info"
{:doc "Return a map of information about the specified symbol."
:requires {"sym" "The symbol to lookup"
"ns" "The current namespace"}
:optional info-params
:returns (merge {"status" "done"} fragments-doc)}
"eldoc"
{:doc "Return a map of information about the specified symbol."
:requires {"sym" "The symbol to lookup"
"ns" "The current namespace"}
:optional info-params
:returns (merge {"status" "done"} fragments-doc)}
"eldoc-datomic-query"
{:doc "Return a map containing the inputs of the datomic query."
Expand Down
49 changes: 44 additions & 5 deletions src/cider/nrepl/middleware/info.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
(ns cider.nrepl.middleware.info
(:require
[compliment.context]
[compliment.sources.class-members]
[cider.nrepl.middleware.util :as util]
[cider.nrepl.middleware.util.cljs :as cljs]
[cider.nrepl.middleware.util.error-handling :refer [with-safe-transport]]
Expand Down Expand Up @@ -57,9 +59,47 @@
blacklist
util/transform-value))))

(defn- extract-class-from-compliment
"Given a Compliment-style `context`, returns the inferred class name
of the object placed at __prefix__."
[ns-str context]
(when (and (seq ns-str)
(seq context))
(try
(when-let [ns-obj (find-ns (symbol ns-str))]
(let [c (compliment.context/cache-context context)
^Class c (compliment.sources.class-members/try-get-object-class ns-obj c)]
(some-> c .getName)))
(catch Exception _
;; We can always be analyzing a broken context.
nil))))

(defn info
[{:keys [ns sym symbol class member] :as msg}]
(let [[ns sym class member] (map misc/as-sym [ns (or sym symbol) class member])
[{:keys [ns sym class member context]
legacy-sym :symbol
:as msg}]
(let [sym (or (not-empty legacy-sym)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need not-empty here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For discarding empty strings

I don't necessarily expect them, but I've found it's a good habit to handle them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I figured as much although clients are generally expected to omit params instead of sending empty strings. I don't think we've got any such checks in cider-nrepl, that's why I've mostly done "truthy" check instead. Anyways, not a big deal right now, but something we can discuss later for the sake of consistency.

Copy link
Member Author

@vemv vemv Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤝

In an ideal world we'd have something like Spec checking/coercion at the edges

(not-empty sym))
class (try
(or (when (and (seq class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this nil-punning here? I still think it doesn't read very well. :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure of what the alternative would be

(I had written a suggestion, but it wasn't accurate)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine either way - I just don't like much mixing the use of seq and not-empty. Rich liked seq, but not-empty was other later on for those who felt seq wasn't very intention revealing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(my main point was the one I made earlier - that probably checking for empty strings is redundant, although I understand your reasoning as well)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice convention is to exclusively use seq as a condition, and not-empty as an object one intends to further use

(seq ns)
(find-ns (symbol ns)))
(some-> ^Class (ns-resolve (find-ns (symbol ns))
(symbol class))
.getName))
(not-empty class)
(when (some-> sym (str/starts-with? "."))
(extract-class-from-compliment ns context)))
(catch Exception e
nil))
java? (seq class)
[ns sym class member] (mapv misc/as-sym [ns
(cond-> sym
(and (seq sym)
java?)
(str/replace #"^\." ""))
class
member])
env (cljs/grab-cljs-env msg)
info-params (merge {:dialect :clj
:ns ns
Expand All @@ -68,10 +108,9 @@
{:env env
:dialect :cljs}))]
(cond
java? (info/info-java class (or member sym))
(and ns sym) (info/info* info-params)
(and class member) (info/info-java class member)
:else (throw (Exception.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The throw wasn't compatible with the new logic. nil is more flexible and will result in a graceful op-specific reply (e.g. status no-info or no-eldoc)

"Either \"symbol\", or (\"class\", \"member\") must be supplied")))))
:else nil)))

(defn info-reply
[msg]
Expand Down
Loading