diff --git a/build.zig.zon b/build.zig.zon index f905035..bf92742 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,8 +5,8 @@ .minimum_zig_version = "0.16.0", .dependencies = .{ .srf = .{ - .url = "git+https://git.lerch.org/lobo/srf#08f06df810684f8d32ad0ea5a8f2b3ee61733d2c", - .hash = "srf-0.0.0-qZj571YvAgCNHXtzQCOdoAKNzjhLp5o_Ap-GMFmO5-CW", + .url = "git+https://git.lerch.org/lobo/srf.git#7f3a43e838fc472bdb1af6c1b692f2255dd19f48", + .hash = "srf-0.0.0-qZj57-17AgAXn2at0WH1xV75oJhr3vTFeU5FxU7kAhtf", }, }, .paths = .{ diff --git a/nix/package.nix b/nix/package.nix index bc120cf..4bac32c 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -22,7 +22,7 @@ stdenv.mkDerivation (finalAttrs: { zigDeps = zig.fetchDeps { inherit (finalAttrs) pname version src; fetchAll = true; - hash = "sha256-hS69otO9YY1acs+Vn8IdFj+mV8QjoeDVtxtsCrXD3RY="; + hash = "sha256-0A2IlywPDkgX8+oiZ1Xd2zF0o07T4moBRngddwl1GBk="; }; postConfigure = '' diff --git a/src/analysis.zig b/src/analysis.zig index a0f1116..ffe530d 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -192,20 +192,13 @@ pub fn analyze( return sink.items.toOwnedSlice(allocator); } -/// Reports length-prefix declarations that srf does not usefully report itself. +/// Reports length prefixes that run past the end of the file. /// -/// srf covers the compact-format case as of 08f06df: a value that does not end on -/// the delimiter gets a diagnostic and a fatal `ParseFailed`. We stay out of its -/// way there, because two squiggles for one defect is worse than one. -/// -/// What is left to us: -/// -/// * **Long format overruns.** `srf.zig:848` takes the `field_delimiter == '\n'` -/// branch and calls `nextLine()` unconditionally, discarding whatever was left -/// of the line. No diagnostic, no failure, no log: `k:3:hello` silently -/// becomes `hel`. Silent data loss is the worst kind, so we report it. -/// * **Declarations past the end of the file.** srf fails with a bare -/// `EndOfStream` and no location at all. +/// This used to cover short prefixes too, but srf reports those itself as of +/// 4bec474, in both formats, and with a better message: it suggests the corrected +/// byte count. Duplicating it would put two squiggles on one defect, so the only +/// case left to us is a declaration longer than the remaining document, which srf +/// still fails on with no location at all. fn appendLengthProblems( allocator: std.mem.Allocator, sink: *Sink, @@ -219,49 +212,27 @@ fn appendLengthProblems( else => continue, }; const problem = located.field.length_problem orelse continue; - - // srf already diagnoses a compact-format overrun, and does it fatally. - if (problem == .overrun and !located.field.long_mode) continue; - - const message = switch (problem) { - .truncated => |t| try std.fmt.allocPrint( - allocator, - "length prefix declares {d} bytes but only {d} remain in the file", - .{ t.declared, t.available }, - ), - .overrun => |o| try std.fmt.allocPrint( - allocator, - "length prefix declares {d} bytes, so srf will silently discard the {d} byte(s) highlighted here", - .{ o.declared, o.surplus }, - ), + const truncated = switch (problem) { + .truncated => |t| t, + // srf's own diagnostic is better than anything we would write here. + .overrun => continue, }; + + const message = try std.fmt.allocPrint( + allocator, + "length prefix declares {d} bytes but only {d} remain in the file", + .{ truncated.declared, truncated.available }, + ); errdefer allocator.free(message); sink.items.append(allocator, .{ - // Underline what is wrong. For an overrun that is the surplus text - // past the declared end, not the bytes that do fit: highlighting the - // part srf keeps would point at the one bit that is correct. - .range = switch (problem) { - .truncated => positions.rangeFromSpan( - text, - located.value_span.start, - located.value_span.end, - encoding, - ), - .overrun => |o| positions.rangeFromSpan( - text, - o.surplus_start, - o.surplus_start + o.surplus, - encoding, - ), - }, - // A long-format overrun still parses, so the cost is losing data - // rather than failing to read the file. A truncated declaration stops - // the parse outright. - .severity = switch (problem) { - .truncated => .err, - .overrun => .warning, - }, + .range = positions.rangeFromSpan( + text, + located.value_span.start, + located.value_span.end, + encoding, + ), + .severity = .err, .message = message, }) catch { allocator.free(message); @@ -588,16 +559,19 @@ test "an error reported at end of line underlines the whole line" { try testing.expectEqual(@as(u32, "broken line here".len), range.end.character); } -test "a long-format length overrun is reported, which srf does not do" { - // srf keeps "hel" and drops "lo" without a word. That is silent data loss, - // so we report it ourselves. - const diags = try analyzeForTest("#!srfv1\n#!long\nk:3:hello\n"); - defer freeDiagnostics(testing.allocator, diags); - try testing.expectEqual(@as(usize, 1), diags.len); - try testing.expectEqual(Severity.warning, diags[0].severity); - try testing.expect(std.mem.indexOf(u8, diags[0].message, "declares 3 bytes") != null); - try testing.expect(std.mem.indexOf(u8, diags[0].message, "silently discard the 2 byte(s)") != null); - try testing.expectEqual(@as(u32, 2), diags[0].range.start.line); +test "a long-format overrun is left to srf, which reports it as of 4bec474" { + // srf used to keep "hel" and drop "lo" silently. It now reports it and + // suggests the corrected count, so we add nothing. + const ours = try lengthDiagnosticsForTest("#!srfv1\n#!long\nk:3:hello\n"); + defer freeDiagnostics(testing.allocator, ours); + try testing.expectEqual(@as(usize, 0), ours.len); + + const all = try analyzeForTest("#!srfv1\n#!long\nk:3:hello\n"); + defer freeDiagnostics(testing.allocator, all); + try testing.expectEqual(@as(usize, 1), all.len); + try testing.expect(std.mem.indexOf(u8, all[0].message, "additional bytes after field value") != null); + try testing.expect(std.mem.indexOf(u8, all[0].message, "restated as 5") != null); + try testing.expectEqual(@as(u32, 2), all[0].range.start.line); } test "a length prefix past the end of the file is an error" { @@ -608,22 +582,32 @@ test "a length prefix past the end of the file is an error" { try testing.expect(std.mem.indexOf(u8, diags[0].message, "declares 99 bytes but only 6 remain") != null); } -test "a compact-format overrun is left to srf, which now reports it" { - // srf 08f06df diagnoses this fatally, so we add nothing: one defect, one - // diagnostic. - const ours = try lengthDiagnosticsForTest("#!srfv1\nk:2:abc\n"); +test "a compact-format overrun is left to srf, which reports it" { + // srf recovers by jumping to the next delimiter and reports once (c5e83f7). + const ours = try lengthDiagnosticsForTest("#!srfv1\nk:3:hello,foo::bar\n"); defer freeDiagnostics(testing.allocator, ours); try testing.expectEqual(@as(usize, 0), ours.len); - const all = try analyzeForTest("#!srfv1\nk:2:abc\n"); + const all = try analyzeForTest("#!srfv1\nk:3:hello,foo::bar\n"); defer freeDiagnostics(testing.allocator, all); try testing.expectEqual(@as(usize, 1), all.len); try testing.expectEqual(Severity.err, all[0].severity); - // srf's own wording, located on the offending line. - try testing.expect(std.mem.indexOf(u8, all[0].message, "reset line for next item") != null); try testing.expectEqual(@as(u32, 1), all[0].range.start.line); + try testing.expect(std.mem.indexOf(u8, all[0].message, "2 additional bytes") != null); + try testing.expect(std.mem.indexOf(u8, all[0].message, "restated as 5") != null); } +test "a compact overrun with no following delimiter still names the real problem" { + // Recovery needs a delimiter later on the line to jump to. Without one the + // leftover bytes are parsed as a field, so srf adds a second, less useful + // "no type data or value after key". The primary diagnostic is still first + // and still correct, which is what an editor shows on the line. + const all = try analyzeForTest("#!srfv1\nk:2:abc\n"); + defer freeDiagnostics(testing.allocator, all); + try testing.expect(all.len >= 1); + try testing.expect(std.mem.indexOf(u8, all[0].message, "1 additional bytes") != null); + try testing.expect(std.mem.indexOf(u8, all[0].message, "restated as 3") != null); +} test "a correct length prefix produces no diagnostic" { const diags = try analyzeForTest("#!srfv1\n#!long\nk:5:hello\n"); defer freeDiagnostics(testing.allocator, diags); @@ -642,18 +626,6 @@ test "a multi-line length-prefixed value produces no diagnostic" { try testing.expectEqual(@as(usize, 0), diags.len); } -test "an overrun diagnostic underlines the surplus, not the bytes that fit" { - // "k:3:hello": the value starts at character 4 and declares 3 bytes, so the - // surplus "lo" runs from character 7 to 9. Highlighting 4..7 would point at - // the one part of the line that is correct. - const text = "#!srfv1\n#!long\nk:3:hello\n"; - const diags = try analyzeForTest(text); - defer freeDiagnostics(testing.allocator, diags); - try testing.expectEqual(@as(usize, 1), diags.len); - try testing.expectEqual(@as(u32, 7), diags[0].range.start.character); - try testing.expectEqual(@as(u32, 9), diags[0].range.end.character); -} - test "a truncation diagnostic underlines the value it does have" { const diags = try lengthDiagnosticsForTest("#!srfv1\n#!long\nk:99:hello\n"); defer freeDiagnostics(testing.allocator, diags); @@ -661,8 +633,9 @@ test "a truncation diagnostic underlines the value it does have" { } test "length diagnostics coexist with srf's own diagnostics" { - // Duplicate magic header (srf's finding) plus a bad length prefix (ours). - const diags = try analyzeForTest("#!srfv1\n#!srfv1\n#!long\nk:3:hello\n"); + // Duplicate magic header (srf's finding) plus a truncated length prefix + // (ours, since srf fails on that one without a location). + const diags = try analyzeForTest("#!srfv1\n#!srfv1\n#!long\nk:99:hello\n"); defer freeDiagnostics(testing.allocator, diags); var from_srf = false; var from_us = false; @@ -674,8 +647,8 @@ test "length diagnostics coexist with srf's own diagnostics" { try testing.expect(from_us); } -test "a compact overrun no longer produces the unlocated fallback" { - // srf now fails fatally *with* a diagnostic, so the generic +test "a compact overrun does not produce the unlocated fallback" { + // srf fails fatally *with* a diagnostic, so the generic // "reported no location" message must not appear alongside it. const diags = try analyzeForTest("#!srfv1\nk:2:abc\n"); defer freeDiagnostics(testing.allocator, diags); @@ -874,7 +847,7 @@ test "an unrecognised hint is left to srf, not counted as a type" { } test "type warnings coexist with length warnings without duplication" { - const text = "#!srfv1\n#!long\nk:num:5\n\nk::abc\n\nbio:3:hello\n"; + const text = "#!srfv1\n#!long\nk:num:5\n\nk::abc\n\nbio:99:hello\n"; const diags = try analyzeForTest(text); defer freeDiagnostics(testing.allocator, diags); var type_warnings: usize = 0; @@ -896,3 +869,163 @@ test "a consistent document produces no warnings at all" { defer freeDiagnostics(testing.allocator, diags); try testing.expectEqual(@as(usize, 0), diags.len); } + +test "whitespace after a length-prefixed value does not crash the parser" { + // srf panicked here before e2b4995: an all-whitespace remainder made + // `checkShortPrefix` index an empty slice. Driving the real parser from our + // own suite means a regression upstream shows up here rather than in an + // editor. + for ([_][]const u8{ + "#!srfv1\n#!long\nk:5:hello \n", + "#!srfv1\n#!long\nk:5:hello\t\n", + "#!srfv1\n#!long\nk:5:hello \n", + "#!srfv1\n#!long\nk:5:hello \t \n", + }) |text| { + const diags = try analyzeForTest(text); + defer freeDiagnostics(testing.allocator, diags); + // Trailing whitespace is acceptable in long format, same as a comment. + try testing.expectEqual(@as(usize, 0), diags.len); + } +} + +test "a comment after a length-prefixed value is accepted" { + for ([_][]const u8{ + "#!srfv1\n#!long\nk:5:hello # a note\n", + "#!srfv1\n#!long\nk:5:hello #tight\n", + "#!srfv1\n#!long\nk:5:hello\t# tabbed\n", + }) |text| { + const diags = try analyzeForTest(text); + defer freeDiagnostics(testing.allocator, diags); + try testing.expectEqual(@as(usize, 0), diags.len); + } +} + +test "compact format does not allow trailing whitespace after a length prefix" { + // No comment allowance there: the next byte must be a comma or the line ends. + // srf reports it, so we add nothing of our own. + const ours = try lengthDiagnosticsForTest("#!srfv1\nk:5:hello \n"); + defer freeDiagnostics(testing.allocator, ours); + try testing.expectEqual(@as(usize, 0), ours.len); + + const all = try analyzeForTest("#!srfv1\nk:5:hello \n"); + defer freeDiagnostics(testing.allocator, all); + try testing.expect(all.len > 0); + try testing.expect(std.mem.indexOf(u8, all[0].message, "additional bytes") != null); +} + +test "a bad length prefix followed by more lines does not crash the parser" { + // Three separate srf panics have lived in this area: an all-whitespace + // remainder indexing an empty slice, and two from `partial_line_column` + // drifting out of sync with `current_line` during intra-record recovery. All + // of them needed a *second* line to show up, which single-record tests never + // supplied. These drive the real parser over multi-line documents so a + // regression upstream surfaces here rather than by killing the server in + // someone's editor. + for ([_][]const u8{ + // compact, short prefix with a delimiter to recover to + "#!srfv1\nk:3:hello,foo::bar\nz::a\n", + "#!srfv1\nk:3:hello,foo::bar\nz::a,y::b\n", + // compact, short prefix with nothing to recover to + "#!srfv1\nk:2:abc\nz::a\n", + "#!srfv1\nk:5:hello \nz::a\n", + // several bad prefixes in a row + "#!srfv1\nk:2:abc\nj:3:hello,x::y\nm:1:zz\nn::ok\n", + // long format equivalents + "#!srfv1\n#!long\nk:3:hello\nz::a\n", + "#!srfv1\n#!long\nk:5:hello \nz::a\n", + "#!srfv1\n#!long\nk:5:hello # c\nz::a\n", + "#!srfv1\n#!long\nbio:5:foo\nbar\nz::a\n", + "#!srfv1\n#!long\nk:99:hello\n", + }) |text| { + const diags = try analyzeForTest(text); + freeDiagnostics(testing.allocator, diags); + } +} + +test "a recovered short prefix reports once when the rest of the line is valid" { + const diags = try analyzeForTest("#!srfv1\nk:3:hello,foo::bar\nz::a,y::b\n"); + defer freeDiagnostics(testing.allocator, diags); + try testing.expectEqual(@as(usize, 1), diags.len); + try testing.expectEqual(@as(u32, 1), diags[0].range.start.line); + try testing.expect(std.mem.indexOf(u8, diags[0].message, "restated as 5") != null); +} + +test "each bad length prefix in a document is reported once, on its own line" { + const diags = try analyzeForTest("#!srfv1\nk:2:abc\nj:3:hello,x::y\nm:1:zz\nn::ok\n"); + defer freeDiagnostics(testing.allocator, diags); + try testing.expectEqual(@as(usize, 3), diags.len); + for (diags, 1..) |d, line| { + try testing.expectEqual(@as(u32, @intCast(line)), d.range.start.line); + try testing.expect(std.mem.indexOf(u8, d.message, "additional bytes") != null); + } +} + +test "a multi-line bad length prefix followed by more lines does not crash" { + // Round four of the same family: `partial_line_column` and `current_line` + // drifting apart during recovery from a miskeyed length prefix. Every one of + // these needed a following line, and several needed a comma on it. Driving the + // real parser here means a regression upstream fails our build instead of + // taking the server down mid-edit. + for ([_][]const u8{ + // underflow that pulls bytes from the next line + "#!srfv1\nbio:5:foo\n# comment\nfinal::v\n", + "#!srfv1\nbio:5:foo\n indented::v\nfinal::v\n", + "#!srfv1\nbio:5:foo\n# com,ment\nfinal::v\n", + "#!srfv1\nbio:5:foo\nab\nfinal::v\n", + // deep underflows spanning several lines + "#!srfv1\nbio:2:foo\nbar\nbaz\n", + "#!srfv1\nbio:1:foo\nbar\n# c\n", + "#!srfv1\nbio:9:foo\nbar\nbaz\nqux\n", + "#!srfv1\nbio:4:a\nb\nc\nd\ne\n", + "#!srfv1\n#!long\nbio:2:foo\nbar\nbaz\n", + // exact lengths that consume into the following line + "#!srfv1\nbio:7:foo\nbar\nfinal::v\n", + "#!srfv1\nbio:7:foo\nbar,baz\nfinal::v\n", + "#!srfv1\nbio:7:foo\nz::a\nfinal::v\n", + }) |text| { + const diags = try analyzeForTest(text); + freeDiagnostics(testing.allocator, diags); + } +} + +test "multi-line recovery from a bad length prefix yields the rest of the record" { + // srf recovers by skipping the surplus bytes and the delimiter after them, + // landing where the corrected document would (srf 7f3a43e). These assert the + // recovered fields, not merely that nothing crashed: four separate panics have + // lived in this path, and "does not crash" would have passed for several of + // the wrong answers along the way. + const cases = [_]struct { text: []const u8, want: []const []const u8 }{ + // Declares 7, so bio is "foo\nz::" and the surplus is "a". + .{ + .text = "#!srfv1\nbio:7:foo\nz::a,y::b\nfinal::v\n", + .want = &.{ "bio", "y", "final" }, + }, + // Declares 12, so bio is "foo\nz:5:a,b," and the surplus is "c". + .{ + .text = "#!srfv1\nbio:12:foo\nz:5:a,b,c,q::x\nfinal::v\n", + .want = &.{ "bio", "q", "final" }, + }, + // Declares 10, so foo is "bar\nbaz,qu" and the surplus is "x". + .{ + .text = "#!srfv1\nfoo:10:bar\nbaz,qux,key::val\n", + .want = &.{ "foo", "key" }, + }, + }; + + for (cases) |case| { + var it = document.items(case.text); + var keys: std.ArrayList([]const u8) = .empty; + defer keys.deinit(testing.allocator); + while (it.next()) |item| switch (item) { + .field => |f| try keys.append(testing.allocator, f.field.key), + else => {}, + }; + try testing.expectEqual(case.want.len, keys.items.len); + for (case.want, keys.items) |w, g| try testing.expectEqualStrings(w, g); + + // And the parser agrees it is broken, exactly once. + const diags = try analyzeForTest(case.text); + defer freeDiagnostics(testing.allocator, diags); + try testing.expectEqual(@as(usize, 1), diags.len); + } +} diff --git a/src/document.zig b/src/document.zig index 35b335f..e50baea 100644 --- a/src/document.zig +++ b/src/document.zig @@ -367,12 +367,31 @@ const Scanner = struct { // cannot run us off the end of the document. const end = @min(value_start + declared, self.text.len); const new_line_end = self.lineEnd(end); - const comma_follows = !self.long_mode and end < new_line_end and self.text[end] == ','; + const problem = self.lengthProblem(declared, value_start); + + // Where the next field starts. When the count lands on a delimiter + // this is simply past it. When it does not, srf recovers by skipping + // the surplus bytes *and* the delimiter after them, so mirror that: + // otherwise hover would find no field where the parser reports one. + const next_field: ?usize = if (self.long_mode) + // srf discards the remainder of the line in long format, so + // nothing further on it is addressable. + null + else if (end < new_line_end and self.text[end] == ',') + end + 1 + else if (problem != null) + if (std.mem.indexOfScalarPos(u8, self.text[0..new_line_end], end, ',')) |comma| + comma + 1 + else + null + else + null; + return .{ .end = end, - .next_field = if (comma_follows) end + 1 else null, + .next_field = next_field, .line_end = new_line_end, - .length_problem = self.lengthProblem(declared, value_start), + .length_problem = problem, }; } @@ -419,17 +438,39 @@ const Scanner = struct { // End of file terminates a field just as well as a newline does. if (after >= self.text.len) return null; - const terminator = self.text[after]; - if (terminator == '\n') return null; - if (!self.long_mode and terminator == ',') return null; + if (self.text[after] == '\n') return null; + if (!self.long_mode) { + if (self.text[after] == ',') return null; + } else { + // Long format tolerates whitespace, and a comment, after the value. + // Mirrors srf's `checkShortPrefix`, so hover cannot flag something + // the parser is happy with. + var i = after; + while (i < self.text.len and isSpace(self.text[i])) i += 1; + if (i >= self.text.len or self.text[i] == '\n' or self.text[i] == '#') return null; + } // Measured from the declared end, not from the start of the value: a // multi-line length-prefixed value can put the surplus on a later line, // and a range built from the first line's end would come out inverted. + // + // How far the surplus runs has to match how srf counts it, or hover ends + // up suggesting a different corrected length than the diagnostic does. srf + // stops at the next delimiter in compact format (`extra_bytes` in + // `checkShortPrefix`) and runs to end of line in long format, where a + // comma is ordinary text. + const line_end = self.lineEnd(after); + const surplus_end = if (self.long_mode) + line_end + else if (std.mem.indexOfScalarPos(u8, self.text[0..line_end], after, ',')) |comma| + comma + else + line_end; + return .{ .overrun = .{ .declared = declared, .surplus_start = after, - .surplus = self.lineEnd(after) - after, + .surplus = surplus_end - after, } }; } @@ -738,8 +779,34 @@ test "in long format a trailing comma after the declared bytes is an overrun" { try testing.expectEqual(@as(usize, 1), problem.overrun.surplus); } -test "trailing whitespace after the declared bytes is an overrun" { - const problem = problemAt("#!srfv1\n#!long\nk:5:hello \n", "hello").?; +test "in long format, trailing whitespace after the declared bytes is accepted" { + // srf's checkShortPrefix trims whitespace before deciding, so flagging this + // would contradict the parser. + try testing.expectEqual( + @as(?LengthProblem, null), + problemAt("#!srfv1\n#!long\nk:5:hello \n", "hello"), + ); + try testing.expectEqual( + @as(?LengthProblem, null), + problemAt("#!srfv1\n#!long\nk:5:hello\t\n", "hello"), + ); +} + +test "in long format, a trailing comment after the declared bytes is accepted" { + try testing.expectEqual( + @as(?LengthProblem, null), + problemAt("#!srfv1\n#!long\nk:5:hello # a note\n", "hello"), + ); + try testing.expectEqual( + @as(?LengthProblem, null), + problemAt("#!srfv1\n#!long\nk:5:hello # spaced\n", "hello"), + ); +} + +test "in compact format, trailing whitespace is still an overrun" { + // Compact format has no comment-after-value allowance: the next byte must be + // a comma or the line must end. + const problem = problemAt("#!srfv1\nk:5:hello \n", "hello").?; try testing.expectEqual(@as(usize, 5), problem.overrun.declared); } @@ -870,3 +937,26 @@ test "value kinds describe themselves for a message" { try testing.expectEqualStrings("`bool`", ValueKind.boolean.describe()); try testing.expectEqualStrings("`binary`", ValueKind.bytes.describe()); } + +test "in compact format the surplus stops at the next delimiter, as srf counts it" { + // srf's `checkShortPrefix` reports `indexOfScalar(past_val, ',')` bytes, so + // measuring to end of line here would make hover suggest a different + // corrected length than the diagnostic does. + const problem = problemAt("#!srfv1\nim_worth:4:23,000,000,000,really:bool:false\n", "im_worth").?; + try testing.expectEqual(@as(usize, 4), problem.overrun.declared); + // "23,0" then "00" then a comma: two surplus bytes, so 4 + 2 = 6. + try testing.expectEqual(@as(usize, 2), problem.overrun.surplus); +} + +test "in compact format a surplus with no delimiter runs to end of line" { + const problem = problemAt("#!srfv1\nk:2:abcdef\n", "k:2:").?; + try testing.expectEqual(@as(usize, 4), problem.overrun.surplus); +} + +test "in long format the surplus runs to end of line, commas included" { + // Long format has no comma delimiter, so a comma is ordinary text and srf + // counts the whole remainder. + const problem = problemAt("#!srfv1\n#!long\nim_worth:4:23,000,000\n", "im_worth").?; + // Value is "23,0"; the remainder "00,000" is six bytes, commas and all. + try testing.expectEqual(@as(usize, 6), problem.overrun.surplus); +} diff --git a/src/hover.zig b/src/hover.zig index 9488c79..ff1856e 100644 --- a/src/hover.zig +++ b/src/hover.zig @@ -308,22 +308,22 @@ fn writeLengthProblem( "**Length mismatch:** declares {d} byte{s} but only {d} remain in the file.\n\n", .{ t.declared, plural(t.declared), t.available }, ), + // srf reports this one itself, fatally, and suggests the corrected count. + // Hover is still where the detail belongs, so state the byte arithmetic + // and the rule rather than just repeating that something is wrong. .overrun => |o| { try w.print( - "**Length mismatch:** declares {d} byte{s}, but the value does not end there", - .{ o.declared, plural(o.declared) }, + "**Length mismatch:** declares {d} byte{s}, but {d} more byte{s} follow before the end of the field.\n\n", + .{ o.declared, plural(o.declared), o.surplus, plural(o.surplus) }, ); + try w.print("Restating the length as {d} would cover them. ", .{o.declared + o.surplus}); if (f.long_mode) { - try w.print( - ": {d} more byte{s} follow.\n\nsrf keeps the first {d} and discards the rest without reporting it.\n\n", - .{ o.surplus, plural(o.surplus), o.declared }, + try w.writeAll( + "A length-prefixed value must be followed by end of line, optionally after whitespace or a comment.\n\n", ); } else { - // srf reports this one itself, fatally. Hover is still where the - // detail belongs, so explain the rule rather than just repeating - // that something is wrong. try w.writeAll( - ". A length-prefixed value must be followed by a comma or end of line, so srf rejects the document.\n\n", + "A length-prefixed value must be followed by a comma or end of line.\n\n", ); } }, @@ -703,11 +703,11 @@ test "looksLikeText accepts text and rejects control bytes" { try testing.expect(!looksLikeText("bad\x07bell")); } -test "an overrun length in long format explains srf's silent truncation" { +test "an overrun length in long format suggests the corrected count" { const text = "#!srfv1\n#!long\nk:3:hello\n"; - try expectHoverContains(text, "k:3:", "declares 3 bytes, but the value does not end there"); - try expectHoverContains(text, "k:3:", "2 more bytes follow"); - try expectHoverContains(text, "k:3:", "discards the rest without reporting it"); + try expectHoverContains(text, "k:3:", "declares 3 bytes, but 2 more bytes follow"); + try expectHoverContains(text, "k:3:", "Restating the length as 5"); + try expectHoverContains(text, "k:3:", "optionally after whitespace or a comment"); } test "a truncated length reports what is actually left" { @@ -718,8 +718,13 @@ test "a truncated length reports what is actually left" { test "an overrun length in compact format explains the terminator rule" { const text = "#!srfv1\nk:2:abc\n"; try expectHoverContains(text, "k:2:", "followed by a comma or end of line"); - // Hover explains it even though the diagnostic for it comes from srf. - try expectHoverContains(text, "k:2:", "srf rejects the document"); + try expectHoverContains(text, "k:2:", "Restating the length as 3"); +} + +test "a trailing comment in long format is not reported as a mismatch" { + const markdown = try hoverOn("#!srfv1\n#!long\nk:5:hello # fine\n", "hello"); + defer testing.allocator.free(markdown); + try testing.expect(std.mem.indexOf(u8, markdown, "Length mismatch") == null); } test "a correct length prefix reports no mismatch" {