From a97157ef7ea905cf325f924f8840455595cc2740 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Thu, 10 Sep 2026 08:47:56 +0100 Subject: [PATCH] Drop structure highlighting runs with a zero-width header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A document starting with a colon recovers as a compound statement whose header is a zero-length error node: ":\n pass\n" TryStatement children: ⚠(0,0) Body(0,11) "if a:\n pass\n" IfStatement children: if(0,2) VariableName(3,4) Body(4,15) start and bodyStart are then both 0, so positionsForNode asked skipBodyTrailers for position -1 and doc.lineAt threw. That accounts for 4181 events in Sentry since April 2024, the largest single group in the project, and it is still present in v3.1.11. Skip this shape as we cannot meaningfully render it. Fixes #1318 --- .../structure-highlighting/blocks.ts | 21 ++++++++++----- .../structure-highlighting/grammar.test.ts | 26 +++++++++++++++++++ .../codemirror/structure-highlighting/view.ts | 3 ++- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/editor/codemirror/structure-highlighting/blocks.ts b/src/editor/codemirror/structure-highlighting/blocks.ts index 34bfbb99f..f53472ac1 100644 --- a/src/editor/codemirror/structure-highlighting/blocks.ts +++ b/src/editor/codemirror/structure-highlighting/blocks.ts @@ -34,6 +34,7 @@ export interface CodeBlock { * Start of the first header node of the run. */ start: number; + /** Always past {@link start}, so `bodyStart - 1` is a valid position. */ bodyStart: number; bodyEnd: number; /** @@ -80,13 +81,19 @@ export const codeBlocks = (state: EditorState): CodeBlock[] => { let runStart = 0; for (let i = 0; i < children.length; ++i) { if (children[i].name === "Body") { - blocks.push({ - statement: leaving.name, - start: children[runStart].start, - bodyStart: children[i].start, - bodyEnd: children[i].end, - depth, - }); + // Error recovery can give a compound statement a zero-length + // error node in place of its header, leaving the body starting + // where the header does. There's no header to draw, and the + // view relies on bodyStart being past start. + if (children[i].start > children[runStart].start) { + blocks.push({ + statement: leaving.name, + start: children[runStart].start, + bodyStart: children[i].start, + bodyEnd: children[i].end, + depth, + }); + } runStart = i + 1; } } diff --git a/src/editor/codemirror/structure-highlighting/grammar.test.ts b/src/editor/codemirror/structure-highlighting/grammar.test.ts index e65f18ff2..b26a9b703 100644 --- a/src/editor/codemirror/structure-highlighting/grammar.test.ts +++ b/src/editor/codemirror/structure-highlighting/grammar.test.ts @@ -150,6 +150,32 @@ describe("structure highlighting grammar contract", () => { ]); }); + it("drops runs whose header is only an error node", () => { + // A leading colon recovers as a TryStatement whose children are a + // zero-length error node then the body, so start and bodyStart are both 0. + // view.ts would take bodyStart - 1 and ask for line -1. + const state = createState(":\n pass\n"); + expect(codeBlocks(state)).toEqual([]); + }); + + it("keeps bodyStart past start for every run", () => { + const samples = [ + "if a:\n pass\n", + "if a:\n pass\nelif b:\n pass\nelse:\n pass\n", + "try:\n pass\nexcept:\n pass\nfinally:\n pass\n", + "if a: pass\n", + ":\n pass\n", + "::\n pass\n", + " pass\n", + "else:\n pass\n", + ]; + for (const doc of samples) { + for (const block of codeBlocks(createState(doc))) { + expect(block.bodyStart).toBeGreaterThan(block.start); + } + } + }); + it("covers every compound statement name in grammarInfo", () => { const samples = [ "if a:\n pass\n", diff --git a/src/editor/codemirror/structure-highlighting/view.ts b/src/editor/codemirror/structure-highlighting/view.ts index a7a73d6a0..a541d5b7a 100644 --- a/src/editor/codemirror/structure-highlighting/view.ts +++ b/src/editor/codemirror/structure-highlighting/view.ts @@ -79,7 +79,8 @@ export const codeStructureView = (option: "full" | "simple") => * * @param view The view. * @param start The start position. - * @param end The end position. + * @param end The end position. Must be greater than zero, as we look + * at the character before it; see CodeBlock.bodyStart. * @param depth Current indent depth (1 per indent level starting at 0). * @param parent The parent positions (e.g. for the while block) if we're calculating body positions, otherwise undefined. * @returns The positions for the block denoted by start/end or undefined if highlighting should be skipped.