diff --git a/CLAUDE.md b/CLAUDE.md index e3b6e73..83eb479 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,8 +69,8 @@ row as a conformance claim. package format · 0004 compiling procedure bodies · 0005 mutable pairs · 0006 errors as abnormal passes · 0007 identity for derived combiners · 0008 caching compiled bodies (proposed, deliberately not built) · 0009 editor tooling from the runtime -outward (`ik check --json`, environment enumeration, and the `ik lsp` language -server are its phases 1–3). +outward (`ik check --json`, environment enumeration, the `ik lsp` language +server, and reader recovery are its phases 1–4). They record measurements and rejected options, not just decisions. When a prediction in one turns out wrong, correct it in place rather than leaving it — several carry diff --git a/IronKernel.Tests/CheckTests.fs b/IronKernel.Tests/CheckTests.fs index 15b05d8..0940ab0 100644 --- a/IronKernel.Tests/CheckTests.fs +++ b/IronKernel.Tests/CheckTests.fs @@ -98,3 +98,10 @@ let ``project check walks sources main and tests`` () = Assert.Equal(2, List.length findings) Assert.Contains(findings, fun finding -> finding.path = brokenSource) Assert.Contains(findings, fun finding -> finding.path = brokenTest)) + +[] +let ``check reports every broken region in one file`` () = + withSource ")\n(ok 1)\n)\n(ok2 2)\n" (fun path -> + let findings = checkFile Unrestricted path + Assert.Equal(2, List.length findings) + Assert.Equal(2, List.length (parseReport (toJson findings)))) diff --git a/IronKernel.Tests/LspTests.fs b/IronKernel.Tests/LspTests.fs index 811bb71..847d2ac 100644 --- a/IronKernel.Tests/LspTests.fs +++ b/IronKernel.Tests/LspTests.fs @@ -114,12 +114,9 @@ let ``completion merges buffer defines with environment symbols`` () = let frames = session [ request 1 "initialize" "{}" - didOpen "(define completion-probe 1)\n" - // Mid-edit the buffer no longer parses; defines from the last - // good parse must still complete. - notification - "textDocument/didChange" - """{"textDocument":{"uri":"file:///probe.ikr"},"contentChanges":[{"text":"(define completion-probe 1)\n(completion-pr"}]}""" + // The buffer does not parse mid-edit; the recovering reader + // still serves its defines -- no didChange, no cache. + didOpen "(define completion-probe 1)\n(completion-pr" request 2 "textDocument/completion" """{"textDocument":{"uri":"file:///probe.ikr"},"position":{"line":1,"character":14}}""" notification @@ -209,3 +206,28 @@ let ``unknown requests get a method-not-found error`` () = | true, value -> Some value | _ -> None) Assert.Equal(-32601, error.GetProperty("code").GetInt32()) + +[] +let ``a broken buffer still gets tokens defines and every error`` () = + let frames = + session + [ request 1 "initialize" "{}" + didOpen "(define twice (vau (x) e x))\n)\n(twice" + request 2 "textDocument/semanticTokens/full" + """{"textDocument":{"uri":"file:///probe.ikr"}}""" ] + // Every broken region publishes its own diagnostic. + match diagnosticsOf frames with + | [ published ] -> Assert.Equal(2, published.GetArrayLength()) + | published -> failwithf "expected one publication, got %d" published.Length + // The recovered tree still classifies the trailing `(twice` use as an + // operative -- the buffer's own vau define survives the breakage. + let data = + (resultOf 2 frames).GetProperty("data").EnumerateArray() + |> Seq.map (fun value -> value.GetInt32()) + |> List.ofSeq + Assert.Equal( + [ 0; 1; 6; 0; 0 // define + 0; 7; 5; 0; 0 // twice (definition) + 0; 7; 3; 0; 0 // vau + 2; 1; 5; 0; 0 ], // twice (use, on the completed trailing form) + data) diff --git a/IronKernel.Tests/ParserTests.fs b/IronKernel.Tests/ParserTests.fs index edeab43..295de6d 100644 --- a/IronKernel.Tests/ParserTests.fs +++ b/IronKernel.Tests/ParserTests.fs @@ -317,3 +317,90 @@ let ``integer literals are exact at any width`` () = Assert.IsType(parsed "123456789012345678901234567890") |> ignore // The L suffix still forces at least 64-bit. Assert.IsType(parsed "42L") |> ignore + +// ---- Recovering reader (ADR 0009 phase 4) ---- + +let private recoveringErrorLines errors = + errors + |> List.map (fun error -> + match error with + | LocatedError(span, _, Parser _) -> span.startPosition.line + | other -> failwithf "unexpected error shape: %A" other) + +[] +let ``recovering reader matches the strict reader on clean input`` () = + let input = "(define a 1)\n(define b (lambda (x) x))\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Empty errors + match readLocatedExprList "recover.ikr" input with + | Choice2Of2 strict -> Assert.Equal(List.length strict, List.length forms) + | Choice1Of2 error -> failwithf "strict reader failed: %A" error + +[] +let ``recovery resumes at the next top-level form`` () = + // The unclosed first form is lost; the good form after it is not, which + // is what the strict reader cannot do. + let input = "(broken\n(define after 1)\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Equal(1, List.length errors) + match forms with + | [ { kind = LList ({ kind = LAtom "define" } :: { kind = LAtom "after" } :: _) } ] -> () + | other -> failwithf "unexpected recovery: %A" other + +[] +let ``a stray closer costs one error and keeps both neighbours`` () = + let input = "(a 1))\n(b 2)\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Equal(1, List.length errors) + Assert.Equal(2, List.length forms) + +[] +let ``each broken region reports its own error`` () = + let input = ")\n(ok 1)\n)\n(ok2 2)\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Equal([1L; 3L], recoveringErrorLines errors) + let formLines = forms |> List.map (fun form -> form.span.startPosition.line) + Assert.Equal([2L; 4L], formLines) + +[] +let ``the trailing form is completed so mid-edit buffers keep their tree`` () = + let input = "(define done 1)\n(define partial (lambda (x)\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Equal(1, List.length errors) + match forms with + | [ first; second ] -> + (match first.kind with + | LList ({ kind = LAtom "define" } :: { kind = LAtom "done" } :: _) -> () + | other -> failwithf "unexpected first form: %A" other) + (match second.kind with + | LList ({ kind = LAtom "define" } :: { kind = LAtom "partial" } :: _) -> () + | other -> failwithf "unexpected completed form: %A" other) + Assert.Equal(2L, second.span.startPosition.line) + | other -> failwithf "expected two forms, got %A" other + +[] +let ``recovered spans survive the window remap exactly`` () = + let input = ")\n(marker 1)\n" + let forms, _ = readLocatedExprListRecovering "recover.ikr" input + match forms with + | [ { kind = LList ({ kind = LAtom "marker"; span = atomSpan } :: _); span = formSpan } ] -> + Assert.Equal(2L, formSpan.startPosition.line) + Assert.Equal(1L, formSpan.startPosition.column) + Assert.Equal(2L, atomSpan.startPosition.line) + Assert.Equal(2L, atomSpan.startPosition.column) + Assert.Equal(6L, atomSpan.endPosition.offset - atomSpan.startPosition.offset) + | other -> failwithf "unexpected forms: %A" other + +[] +let ``an unclosed string disables trailing completion`` () = + let input = "(define ok 1)\n(define s \"unterminated\n" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Equal(1, List.length errors) + Assert.Equal(1, List.length forms) + +[] +let ``the nesting bomb stays a single error with no forms`` () = + let input = String.replicate 300 "(" + let forms, errors = readLocatedExprListRecovering "recover.ikr" input + Assert.Empty forms + Assert.Equal(1, List.length errors) diff --git a/IronKernel/Check.fs b/IronKernel/Check.fs index 0e74b36..672fde9 100644 --- a/IronKernel/Check.fs +++ b/IronKernel/Check.fs @@ -20,9 +20,8 @@ module Check = } let checkFile profile (path: string) : Finding list = - match Emit.checkSourceFileForProfile profile path with - | Choice1Of2 error -> [ { path = path; error = error } ] - | Choice2Of2 () -> [] + Emit.checkFileDiagnostics profile path + |> List.map (fun error -> { path = path; error = error }) /// The files the project's author edits: sources, main, tests. Dependency /// sources are published artifacts and are not this project's to fix. diff --git a/IronKernel/Emit.fs b/IronKernel/Emit.fs index b696123..c237f01 100644 --- a/IronKernel/Emit.fs +++ b/IronKernel/Emit.fs @@ -165,18 +165,22 @@ module Emit = | Choice1Of2 e -> throwError e | Choice2Of2 expressions -> writeIkcPackage outputPath expressions - /// Everything `compile` checks -- parse, analyze -- on in-memory source. - let checkSourceInEnv env sourceName source : ThrowsError = - match analyzePackage env sourceName source with - | Choice1Of2 e -> throwError e - | Choice2Of2 _ -> returnM () - - /// Everything `compile` checks -- read, parse, analyze -- with nothing written. - let checkSourceFileForProfile profile (inputPath: string) : ThrowsError = + /// Everything `compile` checks, with recovery: every parse error in the + /// source (one per broken region), and the same analysis `compile` runs + /// over every form that did parse -- analysis cannot fail today, but the + /// semantic checks that land there will surface here for free. + let checkSourceDiagnostics env sourceName (source: string) : LispError list = + let forms, errors = Parser.readLocatedExprListRecovering sourceName source + for form in forms do + Analyze.analyzeLocatedGuarded env source form |> ignore + errors + + /// The same on a file; an unreadable file is itself the one diagnostic. + let checkFileDiagnostics profile (inputPath: string) : LispError list = match readSource inputPath with - | Choice1Of2 e -> throwError e + | Choice1Of2 e -> [e] | Choice2Of2 source -> - checkSourceInEnv (makePrimitiveBindingsForProfile profile) inputPath source + checkSourceDiagnostics (makePrimitiveBindingsForProfile profile) inputPath source [] let compileFileToAssembly inputPath outputPath = diff --git a/IronKernel/LanguageServer.fs b/IronKernel/LanguageServer.fs index 80acda0..1ba11b8 100644 --- a/IronKernel/LanguageServer.fs +++ b/IronKernel/LanguageServer.fs @@ -181,9 +181,9 @@ module LanguageServer = | _ -> acc let private parsedForms sourceName text = - match Parser.readLocatedExprList sourceName text with - | Choice2Of2 forms -> forms - | Choice1Of2 _ -> [] + // Recovering: a broken buffer still yields the forms that do parse, + // including the trailing form with its brackets closed. + Parser.readLocatedExprListRecovering sourceName text |> fst // ---- Contracts and hover text ----------------------------------------- @@ -234,12 +234,6 @@ module LanguageServer = type private Session = { documents : Dictionary - /// Buffer defines from each document's last *successful* parse. A - /// half-typed buffer does not parse (the reader has no error - /// recovery, ADR 0009 phase 4), and completion mid-edit is exactly - /// when the buffer is broken -- so the last good parse serves until - /// the next one. - defines : Dictionary> /// Bootstrapped kernel environment: symbol source for completion, /// hover, and semantic classification. Never evaluated into by the /// server -- buffers are parsed and analyzed, not run. @@ -255,16 +249,11 @@ module LanguageServer = if parsed.IsFile then parsed.LocalPath else uri with _ -> uri - let private refreshDefines session (uri: string) text = - match Parser.readLocatedExprList (uriToPath uri) text with - | Choice2Of2 forms -> - session.defines.[uri] <- List.fold collectDefines Map.empty forms - | Choice1Of2 _ -> () - - let private definesFor session (uri: string) = - match session.defines.TryGetValue uri with - | true, value -> value - | _ -> Map.empty + /// Defines straight from the current buffer: the recovering reader + /// keeps them available mid-edit, which retired the last-good-parse + /// cache phase 3 carried (ADR 0009 phase 4). + let private definesIn (uri: string) text = + parsedForms (uriToPath uri) text |> List.fold collectDefines Map.empty let private classifySymbol session defines name = match Map.tryFind name defines with @@ -292,9 +281,7 @@ module LanguageServer = writer.WriteString("uri", uri) writer.WritePropertyName "diagnostics" writer.WriteStartArray() - match Emit.checkSourceInEnv session.checkEnv sourceName text with - | Choice2Of2 () -> () - | Choice1Of2 error -> + for error in Emit.checkSourceDiagnostics session.checkEnv sourceName text do let locations, core = errorLocations error writer.WriteStartObject() writer.WritePropertyName "range" @@ -340,7 +327,7 @@ module LanguageServer = | _ -> "" let offset = offsetAt text line character let prefix, envMatches = Repl.completionCandidates session.env text offset - let defines = definesFor session uri + let defines = definesIn uri text let fromBuffer = if prefix = "" then [] else @@ -374,7 +361,7 @@ module LanguageServer = match symbolAt text offset with | None -> respond output id (fun writer -> writer.WriteNullValue()) | Some name -> - let defines = definesFor session uri + let defines = definesIn uri text let lines = match SymbolTable.getVar' session.env name with | Some value -> @@ -440,7 +427,7 @@ module LanguageServer = | true, value -> value | _ -> "" let sourceName = uriToPath uri - let data = semanticTokenData (classifySymbol session (definesFor session uri)) sourceName text + let data = semanticTokenData (classifySymbol session (definesIn uri text)) sourceName text respond output id (fun writer -> writer.WriteStartObject() writer.WritePropertyName "data" @@ -468,7 +455,6 @@ module LanguageServer = | Choice2Of2 env -> let session = { documents = Dictionary() - defines = Dictionary() env = env checkEnv = Runtime.makePrimitiveBindingsForProfile profile } @@ -530,7 +516,6 @@ module LanguageServer = let textDocument = parameters.GetProperty "textDocument" let uri = textDocument.GetProperty("uri").GetString() session.documents.[uri] <- textDocument.GetProperty("text").GetString() - refreshDefines session uri session.documents.[uri] publishDiagnostics output session uri | "textDocument/didChange" -> let uri = textDocumentUri parameters @@ -542,13 +527,11 @@ module LanguageServer = match text with | Some value -> session.documents.[uri] <- value - refreshDefines session uri value publishDiagnostics output session uri | None -> () | "textDocument/didClose" -> let uri = textDocumentUri parameters session.documents.Remove uri |> ignore - session.defines.Remove uri |> ignore notify output "textDocument/publishDiagnostics" (fun writer -> writer.WriteStartObject() writer.WriteString("uri", uri) diff --git a/IronKernel/Parser.fs b/IronKernel/Parser.fs index e91a6fe..f2d2b98 100644 --- a/IronKernel/Parser.fs +++ b/IronKernel/Parser.fs @@ -333,6 +333,173 @@ module Parser = let readLocatedExprList sourceName input = readLocatedOrThrow (ws >>. many (parseLocatedExpr .>> ws) .>> eof) sourceName input + // ---- Recovering reader (ADR 0009 phase 4) ----------------------------- + // + // Recovery lives outside the grammar: broken regions are re-windowed and + // re-parsed with the same strict parsers, and positions are remapped + // exactly, instead of weaving error-recovery alternatives through the + // grammar that `compile` depends on. + + /// A parse run on `input.Substring(offset)` reports positions relative to + /// the window; this is where the window sits in the whole input. + type private WindowBase = { + offset : int64 + line : int64 + column : int64 + } + + /// Exact for any cut point: a window position on line 1 shifts by the + /// base column, every later line already has real columns. + let private remapPosition (window: WindowBase) (position: SourcePosition) : SourcePosition = + { offset = window.offset + position.offset + line = window.line + position.line - 1L + column = + if position.line = 1L then window.column + position.column - 1L + else position.column } + + let private remapSpan window span = + { span with + startPosition = remapPosition window span.startPosition + endPosition = remapPosition window span.endPosition } + + let rec private remapLocated window (value: LocatedValue) = + { kind = + match value.kind with + | LList items -> LList(List.map (remapLocated window) items) + | LDottedList(items, tail) -> + LDottedList(List.map (remapLocated window) items, remapLocated window tail) + | LVector items -> LVector(Array.map (remapLocated window) items) + | LQuote inner -> LQuote(remapLocated window inner) + | leaf -> leaf + span = remapSpan window value.span } + + /// The next offset strictly inside a later line whose first character + /// opens a form -- top-level forms conventionally start at column 1, and + /// that convention is what makes them recovery points. + let private nextTopLevelStart (input: string) (after: int) = + let mutable index = after + let mutable result = None + while result.IsNone && index < input.Length - 1 do + if input.[index] = '\n' && (input.[index + 1] = '(' || input.[index + 1] = '[') then + result <- Some(index + 1) + index <- index + 1 + result + + let private lineAt (input: string) (offset: int) = + let mutable line = 1L + let mutable index = 0 + let mutable previousWasCarriageReturn = false + while index < offset do + match input.[index] with + | '\r' -> + line <- line + 1L + previousWasCarriageReturn <- true + | '\n' -> + if not previousWasCarriageReturn then line <- line + 1L + previousWasCarriageReturn <- false + | _ -> previousWasCarriageReturn <- false + index <- index + 1 + line + + /// The closers that would balance `input.[start..]`, outermost last, + /// respecting strings and comments as `tryNestingError` does. Empty when + /// the region is balanced, over-closed, or ends inside a string -- none + /// of which appending brackets can fix. + let private unclosedBrackets (input: string) (start: int) = + let mutable stack = [] + let mutable inString = false + let mutable escaped = false + let mutable inComment = false + for index in start .. input.Length - 1 do + let character = input.[index] + if inComment then + if character = '\r' || character = '\n' then inComment <- false + elif inString then + if escaped then escaped <- false + elif character = '\\' then escaped <- true + elif character = '"' then inString <- false + else + match character with + | ';' -> inComment <- true + | '"' -> inString <- true + | '(' -> stack <- ')' :: stack + | '[' -> stack <- ']' :: stack + | ')' | ']' -> + match stack with + | _ :: rest -> stack <- rest + | [] -> () + | _ -> () + if inString || List.isEmpty stack then "" + else + // A trailing comment would swallow closers appended on its line. + let closers = System.String(Array.ofList stack) + if inComment then "\n" + closers else closers + + /// Parse as much of `input` as possible: every form that parses, plus one + /// error per broken region, all in source order. Recovery resumes at the + /// next top-level form start; the final broken region is re-parsed with + /// its unclosed brackets closed, so the form being typed still yields a + /// tree. `readLocatedExprList` stays the strict reader `compile` uses. + let readLocatedExprListRecovering sourceName (input: string) : LocatedValue list * LispError list = + match tryNestingError sourceName input with + | Some error -> [], [error] + | None -> + let asManyAsParse = + ws >>. many (attempt (parseLocatedExpr .>> ws)) .>>. getPosition + let forms = ResizeArray() + let errors = ResizeArray() + let pointError (position: SourcePosition) message = + let span = + { sourceName = sourceName + startPosition = position + endPosition = position } + LocatedError(span, sourceLineAt input position.line, Parser message) + let rec parseWindow (baseOffset: int) (baseLine: int64) (baseColumn: int64) = + let window = + { offset = int64 baseOffset; line = baseLine; column = baseColumn } + match runParserOnString asManyAsParse () sourceName (input.Substring baseOffset) with + | Failure _ -> () // `many` of an `attempt` cannot fail. + | Success((parsed, stopPosition), _, _) -> + for form in parsed do + forms.Add(remapLocated window form) + let stopOffset = baseOffset + int stopPosition.Index + if stopOffset < input.Length then + let stop = remapPosition window (sourcePosition stopPosition) + // Word this region's error exactly as the strict + // reader would. + let errorWindow = + { offset = int64 stopOffset; line = stop.line; column = stop.column } + (match runParserOnString + (ws >>. parseLocatedExpr .>> ws .>> eof) + () + sourceName + (input.Substring stopOffset) with + | Failure(message, parserError, _) -> + let position = + remapPosition errorWindow (sourcePosition parserError.Position) + errors.Add(pointError position (conciseParseMessage message)) + | Success _ -> + // Unreachable: the region stopped parsing above. + errors.Add(pointError stop "invalid syntax")) + match nextTopLevelStart input stopOffset with + | Some start -> parseWindow start (lineAt input start) 1L + | None -> + match unclosedBrackets input stopOffset with + | "" -> () + | closers -> + let completed = input.Substring stopOffset + closers + match runParserOnString + (ws >>. many (parseLocatedExpr .>> ws) .>> eof) + () + sourceName + completed with + | Success(parsed, _, _) -> + for form in parsed do + forms.Add(remapLocated errorWindow form) + | Failure _ -> () + parseWindow 0 1L 1L + List.ofSeq forms, List.ofSeq errors + let readExprFromSource sourceName input = match readLocatedExpr sourceName input with | Choice1Of2 error -> throwError error diff --git a/docs/adr/0009-editor-tooling-from-the-runtime-outward.md b/docs/adr/0009-editor-tooling-from-the-runtime-outward.md index 4274b67..bd3dc71 100644 --- a/docs/adr/0009-editor-tooling-from-the-runtime-outward.md +++ b/docs/adr/0009-editor-tooling-from-the-runtime-outward.md @@ -1,6 +1,6 @@ # ADR 0009: Editor tooling grows from the runtime outward -Status: Accepted — phases 1 through 3 implemented +Status: Accepted — phases 1 through 4 implemented ## Decision @@ -152,10 +152,32 @@ parse — a cache that parser recovery will later make unnecessary. Marketplace publishing remains a human decision: it needs a publisher account, and CI already builds the `.vsix` on every push. -**Phase 4 — parser error recovery.** A first failure with no partial tree is -acceptable for check-on-save and fatal for as-you-type diagnostics and -mid-edit completion. This is the largest single piece and nothing above -depends on it, which is why it is fourth and not first. +**Phase 4 — parser error recovery.** *Done.* Recovery lives outside the +grammar: `readLocatedExprListRecovering` re-windows broken input and re-runs +the same strict parsers, remapping positions exactly (a window position on +line 1 shifts by the base column; every later line already has real columns). +The alternative — error-recovery alternatives woven through the FParsec +grammar — was rejected because it would distort the strict path that +`compile` depends on; here the strict readers are untouched and recovery is +composition around them. + +Semantics: one error per broken region, worded by the strict parser; parsing +resumes at the next line that opens a form at column 1 (the convention that +makes top-level forms recovery points); and the final broken region is +re-parsed with its unclosed brackets closed — respecting strings and +comments — so the form being typed still yields a tree. What it deliberately +does not do: a mid-file broken region's own content is dropped up to the +resync point, only the trailing region gets bracket completion, and error +positions are the strict reader's — an unclosed opener still reports at end +of input rather than at the opener. + +Two predictions corrected. "The largest single piece" it was not: it landed +smaller than the language server, because composing recovery from the strict +parsers plus one remap rule avoided the rewrite the estimate priced in. And +the payoff arrived where phase 3 said it would: the last-good-parse defines +cache is deleted, completion and semantic tokens run against the actual +buffer mid-edit, `ik check` reports every broken region in a file, and the +server publishes every error instead of the first. **Phase 5 — the visible surfaces.** Environment inspector (a UI over phase 2), a profile status-bar item that also reads the project's own ``,