diff --git a/CHANGELOG.md b/CHANGELOG.md index e14ca12..93a14e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,3 +30,5 @@ Initial release. CI of a server. - [#11](https://github.com/nrepl/proof/pull/11): Checks for `interrupt`, and scenarios for nREPL's interrupt bugs in `proof serve`. +- [#12](https://github.com/nrepl/proof/pull/12): Checks for `load-file`, and + `load-file` in `proof serve`. diff --git a/PLAN.md b/PLAN.md index 2c79761..75b60ff 100644 --- a/PLAN.md +++ b/PLAN.md @@ -92,9 +92,12 @@ lookups and completions, plus requests that go wrong. ### load-file -- Checks for `load-file` with the fields CIDER, Calva and Conjure send. - The spec and the servers don't agree on which field has the contents - of the file ([spec#3](https://github.com/nrepl/spec.nrepl.org/issues/3)). +- Done: checks for the value of `load-file` and for a load that fails, + where CIDER needs `eval-error`, Calva `ex` and Conjure `err`, plus what + CIDER, Calva and Conjure send to load a file. The spec and the servers + don't agree on which field has the contents of the file + ([spec#3](https://github.com/nrepl/spec.nrepl.org/issues/3)), but every + server in the matrix takes `file` and passes them all. ### Lookups and Completions diff --git a/README.md b/README.md index 62a069c..e15da34 100644 --- a/README.md +++ b/README.md @@ -131,14 +131,14 @@ the replies). `-like jank` turns on all of jank's scenarios at once, and ## Status proof is still in its early days. Right now it covers the core of the -protocol (`describe`, unknown ops, `eval`, sessions, `stdin`, `interrupt` and the wire format), +protocol (`describe`, unknown ops, `eval`, sessions, `stdin`, `interrupt`, `load-file` and the wire format), what CIDER, Calva, Conjure, vim-fireplace, REPLy and neorepl send while -connecting and evaluating code, the requests +connecting, evaluating code and loading files, the requests clients send and the server differences clients have to deal with, and `proof list` will show you all the checks. -Checks for `load-file`, lookups, completions and malformed requests are -coming next, along with the first release. +Checks for lookups, completions and malformed requests are coming next, +along with the first release. [PLAN.md](PLAN.md) has all the details. ## Why "proof"? diff --git a/doc/design.md b/doc/design.md index 148bd7e..9c76d5c 100644 --- a/doc/design.md +++ b/doc/design.md @@ -306,6 +306,6 @@ to have a single test suite that every server can be checked against. ## Future Plans At this point proof covers the core of the protocol (`describe`, unknown -ops, sessions, `eval`, `stdin`, `interrupt` and the wire format), the requests of +ops, sessions, `eval`, `stdin`, `interrupt`, `load-file` and the wire format), the requests of clients and the server differences clients have to deal with. [PLAN.md](../PLAN.md) has what's planned next. diff --git a/doc/hacking.md b/doc/hacking.md index 2b4dc6d..7eafce9 100644 --- a/doc/hacking.md +++ b/doc/hacking.md @@ -256,7 +256,7 @@ besides `done`: | Option | What the reply needs | |---|---| | `new-session` | A `new-session`, which later steps can use as `$` and this name (e.g. `session = "$repl"`). | -| `snippet` | The value of this snippet of the server's profile, with all the parts it came in joined together. Its code goes in the request, as it stands for the user's code. | +| `snippet` | The value of this snippet of the server's profile, with all the parts it came in joined together. Its code goes in the request (in `file` for `load-file`), as it stands for the user's code. | | `values` | At least this many values, told apart by the `ns` that comes with each one (or after it), which is how some clients tell them apart. | | `dicts` | These fields have to be dicts (or empty lists), if the reply has them. Each one is a list of keys, e.g. `["versions", "clojure"]`. | diff --git a/doc/profiles.md b/doc/profiles.md index 2ec58f2..c41458c 100644 --- a/doc/profiles.md +++ b/doc/profiles.md @@ -130,10 +130,10 @@ snippet is missing, all the checks that need it are skipped. | Snippet | Options | What it should do | Checks that use it | |---|---|---|---| -| `value` | `code`, `value` | evaluate to `value` | `eval.value`, `eval.survives-error`, `eval.ns`, `eval.unknown-ns`, `session.ephemeral`, `session.unknown`, `session.closed`, `cider.eval`, `cider.repl`, `calva.eval`, `conjure.eval`, `fireplace.eval`, `neorepl.eval` | +| `value` | `code`, `value` | evaluate to `value` | `eval.value`, `eval.survives-error`, `eval.ns`, `eval.unknown-ns`, `session.ephemeral`, `session.unknown`, `session.closed`, `load-file.value`, `cider.eval`, `cider.repl`, `cider.load-file`, `calva.eval`, `calva.load-file`, `conjure.eval`, `conjure.load-file`, `fireplace.eval`, `neorepl.eval` | | `stdout` | `code`, `out` | print `out` to the standard output | `eval.stdout`, `eval.stdout-order` | | `stderr` | `code`, `err` | print `err` to the standard error | `eval.stderr` | -| `throw` | `code` | raise an error | `eval.error-status`, `eval.error-report`, `eval.survives-error` | +| `throw` | `code` | raise an error | `eval.error-status`, `eval.error-report`, `eval.survives-error`, `load-file.error` | | `define` | `code`, `use`, `value` | define something that `use` reads back as `value` | `session.persistent` | | `session-state` | `code`, `use`, `value` | leave some state in the session that `use` reads back as `value` | `session.across-connections`, `session.isolated` | | `multiple` | `code`, `values` | consist of several forms that evaluate to `values` (in order) | `eval.multiple-forms` | diff --git a/doc/spec-changes.md b/doc/spec-changes.md index e308e63..35b3282 100644 --- a/doc/spec-changes.md +++ b/doc/spec-changes.md @@ -50,6 +50,11 @@ uses `file-path`, but all the servers we've looked at expect `file` (the contents of the file), and some of them accept `file-path` and `file-name` as well. +The spec doesn't say what a failed load looks like either, and clients +tell it by different things: CIDER by `eval-error`, Calva only by `ex` +(it reports a load without `ex` as a success) and Conjure only shows +`err`. A failed load should have all three, just like a failed eval. + ### Where `interrupted` Goes The spec's example reuses the `id` of the eval request for the interrupt diff --git a/doc/usage.md b/doc/usage.md index 47b8baf..ed89ec2 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -100,12 +100,14 @@ eval ``` The checks in the `cider`, `calva`, `conjure`, `fireplace`, `reply` and -`neorepl` groups send what those clients send while connecting and -evaluating code, with all the extra fields they put in their requests. +`neorepl` groups send what those clients send while connecting, +evaluating code and loading files, with all the extra fields they put in +their requests. If one of them fails, the client won't work properly with your server, and the report says what the client does with the reply it got (e.g. "CIDER gives up connecting"). The code they evaluate for the user is the -`value` snippet of your profile. (REPLy sends nothing but the code and +`value` snippet of your profile, which goes in a file for `load-file`. +(REPLy sends nothing but the code and the session, so `eval.value` covers it.) Before evaluating code from a source file, CIDER, Calva and Conjure evaluate an `ns` form for the file's namespace, so those checks also need the `clojure` capability. @@ -424,7 +426,9 @@ Other functions get the error Clojure gives for a symbol it can't resolve, and syntax proof doesn't read (e.g. sets or anonymous functions) gets a read error. That's enough for CIDER, Calva, Conjure and vim-fireplace to connect and work, and it's all you need for checking -output, values, errors, input and interrupts. +output, values, errors, input and interrupts. `load-file` evaluates +what's in the file the same way, and like nREPL it sends only the value +of the last form and stops at the first error. When you stop it, `proof serve` checks the requests your client sent, just like `proof proxy` does, with the same report, exit codes and diff --git a/internal/checks/all.go b/internal/checks/all.go index bdd4512..ade0a2f 100644 --- a/internal/checks/all.go +++ b/internal/checks/all.go @@ -11,6 +11,7 @@ func All() []*check.Check { all = append(all, evalChecks()...) all = append(all, stdinChecks()...) all = append(all, interruptChecks()...) + all = append(all, loadFileChecks()...) all = append(all, clientProfileChecks()...) return all } diff --git a/internal/checks/checks_test.go b/internal/checks/checks_test.go index a1b53a2..5b2ed3d 100644 --- a/internal/checks/checks_test.go +++ b/internal/checks/checks_test.go @@ -57,17 +57,23 @@ func TestChecksCatchMisbehaviour(t *testing.T) { for _, c := range interruptChecks() { noInterrupts[c.ID] = S } + noLoadFile := map[string]check.Verdict{} + for _, c := range loadFileChecks() { + noLoadFile[c.ID] = S + } + // What CIDER, Calva and Conjure send to load a file. + clientsLoad := map[string]check.Verdict{"cider.load-file": F, "calva.load-file": F, "conjure.load-file": F} cases := []struct { name string q quirks want map[string]check.Verdict }{ // Conjure does without the features that need ops. - {"ops as a list", quirks{opsList: true}, checktest.Merged(connects, noInterrupts, map[string]check.Verdict{"describe.ops-dict": F, + {"ops as a list", quirks{opsList: true}, checktest.Merged(connects, noInterrupts, noLoadFile, map[string]check.Verdict{"describe.ops-dict": F, "describe.required-ops": S, "stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "conjure.connect": check.Pass})}, {"clone not advertised", quirks{noClone: true}, map[string]check.Verdict{"describe.required-ops": F}}, {"no versions", quirks{noVersions: true}, map[string]check.Verdict{"describe.versions": W}}, - {"describe kills the connection", quirks{crashOnDescribe: true}, checktest.Merged(connects, noInterrupts, map[string]check.Verdict{ + {"describe kills the connection", quirks{crashOnDescribe: true}, checktest.Merged(connects, noInterrupts, noLoadFile, map[string]check.Verdict{ "describe.reply": F, "describe.ops-dict": S, "describe.required-ops": S, "describe.versions": S, "stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S})}, {"no unknown-op", quirks{noUnknownOp: true}, map[string]check.Verdict{"op.unknown": F, "op.unknown-echo": W}}, @@ -77,9 +83,9 @@ func TestChecksCatchMisbehaviour(t *testing.T) { "op.unknown": F, "op.unknown-echo": F, "wire.status-type": F, "conjure.connect": F}}, {"no session-closed", quirks{noSessionClosed: true}, map[string]check.Verdict{"session.close": F}}, // Only clients put dicts and lists in their requests. - {"flat fields only", quirks{flatFields: true}, map[string]check.Verdict{ + {"flat fields only", quirks{flatFields: true}, checktest.Merged(clientsLoad, map[string]check.Verdict{ "cider.connect": F, "cider.eval": F, "cider.repl": F, "calva.connect": F, "calva.eval": F, "conjure.eval": F, - "neorepl.eval": F}}, + "neorepl.eval": F})}, {"any session accepted", quirks{acceptAnySession: true}, map[string]check.Verdict{"session.unknown": F, "session.closed": F}}, {"shared session state", quirks{sharedState: true}, map[string]check.Verdict{"session.isolated": F}}, {"sessions tied to sockets", quirks{socketSessions: true}, map[string]check.Verdict{"session.across-connections": W, @@ -88,8 +94,9 @@ func TestChecksCatchMisbehaviour(t *testing.T) { {"last value only", quirks{lastValueOnly: true}, map[string]check.Verdict{"eval.multiple-forms": F, "fireplace.connect": F}}, {"stderr dropped", quirks{dropErr: true}, map[string]check.Verdict{"eval.stderr": F}}, {"output after value", quirks{outAfterValue: true}, map[string]check.Verdict{"eval.stdout-order": W}}, - {"no eval-error", quirks{noEvalError: true}, map[string]check.Verdict{"eval.error-status": F}}, - {"no ex", quirks{noEx: true}, map[string]check.Verdict{"eval.error-report": W}}, + {"no eval-error", quirks{noEvalError: true}, map[string]check.Verdict{"eval.error-status": F, "load-file.error": F}}, + // Calva tells a failed load by its ex alone. + {"no ex", quirks{noEx: true}, map[string]check.Verdict{"eval.error-report": W, "load-file.error": F}}, {"no no-code", quirks{noNoCode: true}, map[string]check.Verdict{"eval.no-code": W}}, // REPLy evaluates nothing before its first prompt. {"empty code never answered", quirks{emptyCodeHangs: true}, map[string]check.Verdict{"reply.connect": F}}, @@ -105,11 +112,11 @@ func TestChecksCatchMisbehaviour(t *testing.T) { {"state after eval-error", quirks{errorThenState: true}, map[string]check.Verdict{}}, {"session not echoed", quirks{noSessionEcho: true}, map[string]check.Verdict{"wire.session-echo": F}}, {"output without id", quirks{noID: true}, map[string]check.Verdict{ - "wire.id": W, "eval.stdout": F, "eval.stderr": F, "eval.error-report": W}}, - {"integer value", quirks{intValue: true}, map[string]check.Verdict{ + "wire.id": W, "eval.stdout": F, "eval.stderr": F, "eval.error-report": W, "load-file.error": F}}, + {"integer value", quirks{intValue: true}, checktest.Merged(clientsLoad, map[string]check.Verdict{ "wire.field-types": F, "eval.value": F, "eval.survives-error": F, "eval.multiple-forms": F, "session.ephemeral": F, "session.persistent": F, "cider.eval": F, "cider.repl": F, "calva.eval": F, "conjure.eval": F, "fireplace.eval": F, - "neorepl.eval": F, "interrupt.survives": F, "interrupt.queued": F}}, + "neorepl.eval": F, "interrupt.survives": F, "interrupt.queued": F, "load-file.value": F})}, {"unsorted keys", quirks{unsortedKeys: true}, map[string]check.Verdict{"wire.canonical": W}}, {"invalid UTF-8", quirks{badUTF8: true}, map[string]check.Verdict{"wire.utf8": W, "eval.stdout": F}}, {"no stdin op", quirks{noStdinOp: true}, map[string]check.Verdict{ @@ -133,6 +140,11 @@ func TestChecksCatchMisbehaviour(t *testing.T) { {"interrupted on the wrong connection", quirks{interruptHere: true}, map[string]check.Verdict{ "interrupt.other-connection": F, "wire.id": W}}, {"session dies on interrupt", quirks{diesOnInterrupt: true}, map[string]check.Verdict{"interrupt.survives": F, "interrupt.queued": F}}, + // CIDER and Conjure send load-file without asking, and so does + // Calva's profile, as it can't load files either way. + {"no load-file op", quirks{noLoadFileOp: true}, checktest.Merged(noLoadFile, clientsLoad)}, + {"load-file without a value", quirks{loadFileNoValue: true}, checktest.Merged(clientsLoad, + map[string]check.Verdict{"load-file.value": F})}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/internal/checks/client_profiles.go b/internal/checks/client_profiles.go index 04df4a9..ca19374 100644 --- a/internal/checks/client_profiles.go +++ b/internal/checks/client_profiles.go @@ -46,8 +46,8 @@ type clientStep struct { // an earlier step got. Send request `toml:"send"` // Snippet names a snippet of the server's profile whose code goes in - // the request, and whose value the reply has to have. It stands for - // the user's code. + // the request (in file for load-file, and code otherwise), and whose + // value the reply has to have. It stands for the user's code. Snippet string `toml:"snippet"` // NewSession names the session the reply has to hand back. NewSession string `toml:"new-session"` @@ -147,8 +147,13 @@ func (c clientCheck) replay(t *check.T) { } var want string if s.Snippet != "" { + // The user's code is in a file for load-file. + field := "code" + if req.Str("op") == "load-file" { + field = "file" + } sn := t.Snippet(s.Snippet) - req["code"], want = sn.Code, sn.Value + req[field], want = sn.Code, sn.Value } step := fmt.Sprintf("step %d (%s)", i+1, req.Str("op")) resp := t.Request(conn, req) diff --git a/internal/checks/client_profiles_test.go b/internal/checks/client_profiles_test.go index 07c4969..6172a1b 100644 --- a/internal/checks/client_profiles_test.go +++ b/internal/checks/client_profiles_test.go @@ -21,8 +21,7 @@ func TestClientProfiles(t *testing.T) { } pinned := regexp.MustCompile(`^https://github\.com/[^/]+/[^/]+/blob/[0-9a-f]{40}/`) // The links in refs.go to the same clients. - bases := map[string]string{"CIDER": ciderBase, "Calva": strings.TrimSuffix(calvaBase, "nrepl/index.ts"), - "Conjure": strings.TrimSuffix(conjureBase, "client/clojure/nrepl/server.fnl"), "vim-fireplace": fireplaceBase, + bases := map[string]string{"CIDER": ciderBase, "Calva": calvaSrc, "Conjure": conjureSrc, "vim-fireplace": fireplaceBase, "REPLy": strings.TrimSuffix(replyBase, "eval_modes/nrepl.clj")} for _, p := range clientProfiles() { if p.Name == "" || !pinned.MatchString(p.Code) { diff --git a/internal/checks/clients/calva.toml b/internal/checks/clients/calva.toml index 479bd17..5ff6ca9 100644 --- a/internal/checks/clients/calva.toml +++ b/internal/checks/clients/calva.toml @@ -79,3 +79,30 @@ column = 1 pprint = 1 "nrepl.middleware.print/print" = "cider.nrepl.pprint/pprint" "nrepl.middleware.print/options" = { right-margin = 120, length = 50 } + +[[checks]] +id = "calva.load-file" +title = "Calva can load a file" +why = "Loading a file sends what's in it with load-file, along with the file's path, copies of its name and path in fields of Calva's own and options for printing the value, so a server that can't handle them breaks loading files in Calva." + +[[checks.steps]] +send = { op = "clone", client-name = "Calva", client-version = "2.0.599" } +new-session = "calva" +why = "Calva gives up connecting after 30 seconds" +refs = ["nrepl/index.ts#L277-L286", "nrepl/index.ts#L243-L253"] + +[[checks.steps]] +snippet = "value" +why = "Calva says there were no results from the file" +refs = ["nrepl/index.ts#L783-L817", "evaluate.ts#L786-L793"] + +[checks.steps.send] +op = "load-file" +session = "$calva" +file-name = "calva.clj" +file-path = "/home/user/project/src/proof/calva.clj" +fileName = "calva.clj" +filePath = "/home/user/project/src/proof/calva.clj" +pprint = 1 +"nrepl.middleware.print/print" = "cider.nrepl.pprint/pprint" +"nrepl.middleware.print/options" = { right-margin = 120, length = 50 } diff --git a/internal/checks/clients/cider.toml b/internal/checks/clients/cider.toml index 12fe83e..76b39a7 100644 --- a/internal/checks/clients/cider.toml +++ b/internal/checks/clients/cider.toml @@ -116,3 +116,28 @@ column = 7 "nrepl.middleware.print/buffer-size" = 4096 "nrepl.middleware.print/options" = { right-margin = 70 } content-type = "true" + +[[checks]] +id = "cider.load-file" +title = "CIDER can load a buffer" +why = "Loading a buffer (C-c C-k) sends what's in it with load-file, along with the buffer's path and options for printing the value, so a server that can't handle them breaks loading buffers in CIDER." + +[[checks.steps]] +send = { op = "clone", client-name = "CIDER", client-version = "2.1.0-snapshot" } +new-session = "repl" +why = "CIDER gives up connecting" +refs = ["nrepl-client.el#L749-L760"] + +[[checks.steps]] +snippet = "value" +why = "CIDER doesn't show the result and never marks the buffer as loaded" +refs = ["cider-client.el#L799-L817", "cider-client.el#L572-L586", "cider-eval.el#L614-L635"] + +[checks.steps.send] +op = "load-file" +session = "$repl" +file-path = "/home/user/project/src/proof/cider.clj" +file-name = "cider.clj" +"nrepl.middleware.print/print" = "cider.nrepl.pprint/pr" +"nrepl.middleware.print/stream?" = [] +"nrepl.middleware.print/quota" = 1048576 diff --git a/internal/checks/clients/conjure.toml b/internal/checks/clients/conjure.toml index 5133c1b..b41ebd9 100644 --- a/internal/checks/clients/conjure.toml +++ b/internal/checks/clients/conjure.toml @@ -149,3 +149,27 @@ line = 3 column = 1 "nrepl.middleware.print/print" = "cider.nrepl.pprint/pprint" "nrepl.middleware.print/options" = { associative = 1, length = 500, level = 50, right-margin = 72 } + +[[checks]] +id = "conjure.load-file" +title = "Conjure can evaluate a file" +why = "Evaluating a file sends what's in it with load-file, along with the file's path and options for printing the value, so a server that can't handle them breaks evaluating files in Conjure." + +[[checks.steps]] +send = { op = "clone", client-name = "Conjure" } +new-session = "conjure" +why = "Conjure gives up waiting after 10 seconds and evaluates code without a session, so nothing (e.g. *1 or *e) carries over from one evaluation to the next" +refs = ["client/clojure/nrepl/server.fnl#L259-L272", "client/clojure/nrepl/server.fnl#L379-L392", "remote/nrepl.fnl#L48-L57"] + +[[checks.steps]] +snippet = "value" +why = "users don't see the result of the file" +refs = ["client/clojure/nrepl/server.fnl#L154-L165", "client/clojure/nrepl/ui.fnl#L24-L50"] + +[checks.steps.send] +op = "load-file" +session = "$conjure" +file-name = "conjure.clj" +file-path = "/home/user/project/src/proof/conjure.clj" +"nrepl.middleware.print/print" = "cider.nrepl.pprint/pprint" +"nrepl.middleware.print/options" = { associative = 1, length = 500, level = 50, right-margin = 72 } diff --git a/internal/checks/fake_test.go b/internal/checks/fake_test.go index bd7a7ad..0bd88e2 100644 --- a/internal/checks/fake_test.go +++ b/internal/checks/fake_test.go @@ -63,6 +63,8 @@ type quirks struct { dropQueued bool // evals queued behind an interrupted one never run interruptHere bool // the interrupted eval's done goes to the interrupt's connection diesOnInterrupt bool // an interrupted session never answers again + noLoadFileOp bool // there's no load-file op + loadFileNoValue bool // load-file sends no value } var unknownSession = map[string]any{"status": []any{"error", "unknown-session", "done"}} @@ -214,6 +216,9 @@ func (s *fakeServer) handle(c net.Conn, req nrepl.Message, local map[string]bool if !q.noInterruptOp { names = append(names, "interrupt") } + if !q.noLoadFileOp { + names = append(names, "load-file") + } var ops any if q.opsList { l := []any{} @@ -239,7 +244,7 @@ func (s *fakeServer) handle(c net.Conn, req nrepl.Message, local map[string]bool session = "7:session" + strconv.Itoa(len(sess)) + ":" + sess } c.Write([]byte("d6:statusl4:donee2:id" + strconv.Itoa(len(req.Str("id"))) + ":" + req.Str("id") + - "3:opsd8:describede4:evalde5:clonede5:closede5:stdinde9:interruptdee" + session + + "3:opsd8:describede4:evalde5:clonede5:closede5:stdinde9:interruptde9:load-filedee" + session + "8:versionsd4:faked14:version-string3:1.0eee")) return true } @@ -270,23 +275,34 @@ func (s *fakeServer) handle(c net.Conn, req nrepl.Message, local map[string]bool s.stdin(c, req, local) case "interrupt": s.interrupt(c, req, local) - default: - switch { - case q.statusString: - s.send(c, req, map[string]any{"status": "done"}) - case q.noUnknownOp: - s.send(c, req, map[string]any{"status": done}) - default: - fields := map[string]any{"status": []any{"error", "unknown-op", "done"}} - if !q.noOpEcho { - fields["op"] = op - } - s.send(c, req, fields) + case "load-file": + if q.noLoadFileOp { + s.unknownOp(c, req) + break } + // Like nREPL, an eval of what's in the file. + s.eval(c, req.With(nrepl.Message{"code": req["file"]}), local) + default: + s.unknownOp(c, req) } return true } +func (s *fakeServer) unknownOp(c net.Conn, req nrepl.Message) { + switch { + case s.q.statusString: + s.send(c, req, map[string]any{"status": "done"}) + case s.q.noUnknownOp: + s.send(c, req, map[string]any{"status": []any{"done"}}) + default: + fields := map[string]any{"status": []any{"error", "unknown-op", "done"}} + if !s.q.noOpEcho { + fields["op"] = req.Str("op") + } + s.send(c, req, fields) + } +} + // stdin finishes an eval waiting for input, answering like Clojure's // read-line: the line without its newline, or nil at end of input. func (s *fakeServer) stdin(c net.Conn, req nrepl.Message, local map[string]bool) { @@ -487,7 +503,7 @@ func (s *fakeServer) eval(c net.Conn, req nrepl.Message, local map[string]bool) valueMsg["value"] = n } } - if q.lastValueOnly { + if q.lastValueOnly || q.loadFileNoValue && req.Str("op") == "load-file" { if out != "" { s.send(c, req, map[string]any{"out": out}) } diff --git a/internal/checks/loadfile.go b/internal/checks/loadfile.go new file mode 100644 index 0000000..66e89e5 --- /dev/null +++ b/internal/checks/loadfile.go @@ -0,0 +1,60 @@ +package checks + +import ( + "github.com/nrepl/proof/internal/check" + "github.com/nrepl/proof/internal/profile" + "github.com/nrepl/proof/nrepl" +) + +func loadFileChecks() []*check.Check { + return []*check.Check{ + { + ID: "load-file.value", + Title: "load-file returns the value of the last form", + Severity: check.Fail, + Why: "CIDER marks a buffer as loaded and shows the result only when it gets a value, and Calva says there were no results without one.", + Refs: []check.Ref{ciderLoadFile, calvaLoadFile, nreplLoadFile}, + Snippets: []string{"value"}, + Requires: []string{"describe.reply", "describe.ops-dict", "session.clone"}, + Run: func(t *check.T) { + s, resp := loadSnippet(t, "value") + if vs := resp.Values(); len(vs) == 0 || vs[len(vs)-1] != s.Value { + t.Violatef("loading a file with %q gave values %q, want %q last", s.Code, vs, s.Value) + } + }, + }, + { + ID: "load-file.error", + Title: "A load-file that throws says so in err, ex and eval-error", + Severity: check.Fail, + Why: "Clients tell a failed load apart in different ways: CIDER by eval-error, Calva only by ex (and it reports the load as a success without it) and Conjure only shows err.", + Refs: []check.Ref{ciderLoadFileError, calvaLoadFileError, conjureLoadFile, nreplLoadFile}, + Snippets: []string{"throw"}, + Requires: []string{"describe.reply", "describe.ops-dict", "session.clone"}, + Run: func(t *check.T) { + s, resp := loadSnippet(t, "throw") + if !resp.HasStatus("eval-error") { + t.Violatef("loading a file with %q gave status %v, without eval-error", s.Code, resp.Status()) + } + if resp.Str("ex") == "" { + t.Violatef("loading a file with %q gave no ex", s.Code) + } + if resp.Err() == "" { + t.Violatef("loading a file with %q gave no err", s.Code) + } + }, + }, + } +} + +// loadSnippet loads a file with a snippet's code in it in a new session, +// with the fields CIDER, Calva and Conjure all send. It skips the check +// unless the server has load-file. +func loadSnippet(t *check.T, name string) (profile.Snippet, nrepl.Response) { + requireOp(t, "load-file") + s := t.Snippet(name) + c := t.Connect() + session := t.Session(c) + return s, t.Request(c, session.With(nrepl.Message{"op": "load-file", "file": s.Code, + "file-path": "/home/user/project/proof", "file-name": "proof"})) +} diff --git a/internal/checks/refs.go b/internal/checks/refs.go index 7f5f2c7..691f2a9 100644 --- a/internal/checks/refs.go +++ b/internal/checks/refs.go @@ -10,8 +10,10 @@ import ( // Links are pinned to commits so line numbers stay put. const ( ciderBase = "https://github.com/clojure-emacs/cider/blob/9e049baa1c2c136724d7538b1df6898ee77de6e7/lisp/" - calvaBase = "https://github.com/BetterThanTomorrow/calva/blob/ad00dd7e518e0d08262810673111eaeddc5c2fdd/src/nrepl/index.ts" - conjureBase = "https://github.com/Olical/conjure/blob/9842bf38464df071f72546f65fa6ec44942ca7b2/fnl/conjure/client/clojure/nrepl/server.fnl" + calvaSrc = "https://github.com/BetterThanTomorrow/calva/blob/ad00dd7e518e0d08262810673111eaeddc5c2fdd/src/" + calvaBase = calvaSrc + "nrepl/index.ts" + conjureSrc = "https://github.com/Olical/conjure/blob/9842bf38464df071f72546f65fa6ec44942ca7b2/fnl/conjure/" + conjureBase = conjureSrc + "client/clojure/nrepl/server.fnl" fireplaceBase = "https://github.com/tpope/vim-fireplace/blob/5e66509599de92550762cf2681338fc4cd1e71cf/autoload/" nreplBase = "https://github.com/nrepl/nrepl/blob/16cc5cb0f2adae44d389ba23c864a02eaa4e71a5/src/clojure/nrepl/" specBase = "https://github.com/nrepl/spec.nrepl.org/blob/67796e34ac34f2f28c3af685fc3ab432fe5eb03f/spec.md" @@ -72,9 +74,15 @@ var ( ciderReplPrompt = ref("CIDER shows the REPL's next prompt at done", ciderBase+"cider-repl.el#L1363-L1366") ciderSpinner = ref("CIDER stops its spinner at done", ciderBase+"cider-client.el#L105-L117") calvaInterrupted = ref("Calva settles an eval only at its done", calvaBase+"#L1504-L1527") - conjureInterrupt = ref("Conjure interrupts without interrupt-id", "https://github.com/Olical/conjure/blob/9842bf38464df071f72546f65fa6ec44942ca7b2/fnl/conjure/client/clojure/nrepl/action.fnl#L260-L297") + conjureInterrupt = ref("Conjure interrupts without interrupt-id", conjureSrc+"client/clojure/nrepl/action.fnl#L260-L297") fireplaceInterrupt = ref("vim-fireplace interrupts on Ctrl-C and goes on in the same session", fireplaceBase+"fireplace.vim#L1346-L1369") neoreplInterrupt = ref("neorepl interrupt stops an eval from another process", neoreplBase+"main.clj#L374-L385") + ciderLoadFile = ref("CIDER marks a buffer as loaded when it gets a value", ciderBase+"cider-eval.el#L614-L635") + ciderLoadFileError = ref("CIDER shows an error when it gets eval-error", ciderBase+"cider-eval.el#L265-L274") + calvaLoadFile = ref("Calva says there were no results without a value", calvaSrc+"evaluate.ts#L786-L793") + calvaLoadFileError = ref("Calva tells a failed load by its ex", calvaBase+"#L1461-L1463") + conjureLoadFile = ref("Conjure shows out, err and value and nothing else", conjureSrc+"client/clojure/nrepl/ui.fnl#L24-L50") + nreplLoadFile = ref("nREPL's load-file", nreplBase+"middleware/load_file.clj#L41-L70") ciderNeedInput = ref("CIDER need-input handling", ciderBase+"nrepl-client.el#L948-L950") nreplStdinEOF = ref("nREPL treats an empty stdin as EOF", nreplBase+"middleware/session.clj#L386-L396") diff --git a/internal/serve/serve.go b/internal/serve/serve.go index 9dc9d99..8096a8e 100644 --- a/internal/serve/serve.go +++ b/internal/serve/serve.go @@ -211,7 +211,7 @@ func (s *Server) encode(msg map[string]any) []byte { var ( done = []any{"done"} unknownSession = map[string]any{"status": []any{"error", "unknown-session", "done"}} - ops = []string{"clone", "close", "describe", "eval", "interrupt", "stdin"} + ops = []string{"clone", "close", "describe", "eval", "interrupt", "load-file", "stdin"} ) // offers reports whether the server supports an op. @@ -253,11 +253,15 @@ func (s *Server) handle(c *conn, req nrepl.Message) bool { s.clone(c, req, sess) case "close": s.close(c, req, sess) - case "eval": + case "eval", "load-file": if s.b.hangUp { c.hangUp() return false } + if op == "load-file" { + // An eval of what's in the file, as on nREPL. + req = req.With(nrepl.Message{"code": req["file"]}) + } s.eval(c, req, sess) case "stdin": sess.give(req.Str("stdin")) @@ -503,7 +507,11 @@ func (s *Server) runEval(sess *session, j *job) { }, } final := map[string]any{"status": done} - // held is the value last-value sends at the end. + // load-file sends only the value of the last form, without its ns, + // and stops at the first error, as nREPL does. + loading := req.Str("op") == "load-file" + lastOnly := s.b.lastValue || loading + // held is the value last-value and load-file send at the end. var held map[string]any rd := &reader{src: req.Str("code")} for !stopped() { @@ -528,14 +536,17 @@ func (s *Server) runEval(sess *session, j *job) { final, held = s.fail(reply, t, e.ns), nil // nREPL goes on with the next form, unless there's no telling // where it starts. Basilisp and jank give up on the rest. - if t.phase == reading || s.b.lastValue || s.b.errorWithDone { + if t.phase == reading || lastOnly || s.b.errorWithDone { break } continue } sess.b.push(v) - msg := map[string]any{"value": pr(v), "ns": e.ns} - if s.b.lastValue { + msg := map[string]any{"value": pr(v)} + if !loading { + msg["ns"] = e.ns + } + if lastOnly { held = msg } else { reply(msg) @@ -544,8 +555,9 @@ func (s *Server) runEval(sess *session, j *job) { if held != nil { reply(held) } - // A namespace given with the request is only for this eval. - if !req.Has("ns") { + // A namespace given with the request is only for this eval, and + // loading a file doesn't change the session's either. + if !req.Has("ns") && !loading { sess.b.setNS(e.ns) } if sess.finish(j) { diff --git a/internal/serve/serve_test.go b/internal/serve/serve_test.go index 7386cf6..62465c6 100644 --- a/internal/serve/serve_test.go +++ b/internal/serve/serve_test.go @@ -407,10 +407,10 @@ func TestDescribe(t *testing.T) { ops []string version any }{ - {nil, []string{"clone", "close", "describe", "eval", "interrupt", "stdin"}, + {nil, []string{"clone", "close", "describe", "eval", "interrupt", "load-file", "stdin"}, map[string]any{"major": int64(0), "minor": int64(1), "incremental": int64(0), "version-string": "0.1.0-dev"}}, - {[]string{"no-close-op", "no-interrupt", "no-stdin", "string-versions"}, []string{"clone", "describe", "eval"}, "0.1.0-dev"}, - {[]string{"read-line-throws"}, []string{"clone", "close", "describe", "eval", "interrupt"}, + {[]string{"no-close-op", "no-interrupt", "no-stdin", "string-versions"}, []string{"clone", "describe", "eval", "load-file"}, "0.1.0-dev"}, + {[]string{"read-line-throws"}, []string{"clone", "close", "describe", "eval", "interrupt", "load-file"}, map[string]any{"major": int64(0), "minor": int64(1), "incremental": int64(0), "version-string": "0.1.0-dev"}}, } for _, tc := range cases {