Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion packages/lib-engine/src/actions/ShowParseTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ function parseTree(
const resultPlayground: string[] = [];
const resultQuery: string[] = [];

parseCursor(resultPlayground, resultQuery, contentRange, tree.walk(), 0);
const cursor = tree.walk();
try {
parseCursor(resultPlayground, resultQuery, contentRange, cursor, 0);
} finally {
cursor.delete();
}

return [
`## ${document.filename} [${contentRange}]\n`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ export function getChildNodesForFieldName(
): Node[] {
const nodes = [];
const treeCursor = node.walk();
let hasNext = treeCursor.gotoFirstChild();
try {
let hasNext = treeCursor.gotoFirstChild();

while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
nodes.push(treeCursor.currentNode);
while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
nodes.push(treeCursor.currentNode);
}
hasNext = treeCursor.gotoNextSibling();
}
hasNext = treeCursor.gotoNextSibling();
}

return nodes;
return nodes;
} finally {
treeCursor.delete();
}
}
24 changes: 14 additions & 10 deletions packages/lib-engine/src/languages/TreeSitterQuery/isEven.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ export function isEven(node: Node, fieldName: string): boolean {
}

const treeCursor = node.parent.walk();
let hasNext = treeCursor.gotoFirstChild();
let even = true;
try {
let hasNext = treeCursor.gotoFirstChild();
let even = true;

while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
if (treeCursor.currentNode.id === node.id) {
return even;
while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
if (treeCursor.currentNode.id === node.id) {
return even;
}
even = !even;
}
even = !even;
hasNext = treeCursor.gotoNextSibling();
}
hasNext = treeCursor.gotoNextSibling();
}

throw new Error(`Node not found in parent for field: ${fieldName}`);
throw new Error(`Node not found in parent for field: ${fieldName}`);
} finally {
treeCursor.delete();
}
}
85 changes: 85 additions & 0 deletions packages/lib-engine/src/util/treeSitterUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import assert from "node:assert/strict";
import { mock } from "node:test";
import type { Tree, TreeCursor } from "web-tree-sitter";
import { Node } from "web-tree-sitter";
import { FakeIDE, InMemoryTextEditor } from "@cursorless/lib-common";
import { getChildNodesForFieldName } from "../languages/TreeSitterQuery/getChildNodesForFieldName";
import { isEven } from "../languages/TreeSitterQuery/isEven";
import { TestTreeSitter } from "../testUtil/TestTreeSitter";
import { getChildNodesForFieldName as getChildren } from "./treeSitterUtils";

suite("Tree-sitter cursor lifetime", () => {
const treeSitter = new TestTreeSitter();
let tree: Tree;
const deletions: {
cursor: TreeCursor;
deletion: ReturnType<typeof mock.method>;
}[] = [];

suiteSetup(async () => {
await treeSitter.loadLanguage("clojure");
});

setup(() => {
const { document } = new InMemoryTextEditor({
ide: new FakeIDE(),
content: "{:a 1 :b 2}",
languageId: "clojure",
});
tree = treeSitter.getTree(document);
const walk = Node.prototype.walk;
mock.method(Node.prototype, "walk", function (this: Node) {
const cursor = walk.call(this);
deletions.push({ cursor, deletion: mock.method(cursor, "delete") });
return cursor;
});
});

teardown(() => {
try {
assert.ok(deletions.length > 0, "Expected traversal to create a cursor");
for (const { deletion } of deletions) {
assert.equal(deletion.mock.callCount(), 1);
}
} finally {
// Also clean up cursors if a regression leaves them unfreed. Their WASM
// finalizers can crash the process, obscuring the assertion failure.
for (const { cursor, deletion } of deletions) {
if (deletion.mock.callCount() === 0) {
cursor.delete();
}
}
deletions.length = 0;
mock.restoreAll();
tree.delete();
}
});

test("releases the cursor on an early return from isEven", () => {
const node = tree.rootNode.descendantsOfType("kwd_lit")[0];
assert.equal(isEven(node, "value"), true);
});

test("releases the cursor when isEven throws for a missing field", () => {
const node = tree.rootNode.descendantsOfType("kwd_lit")[0];
assert.throws(() => isEven(node, "missing"), /Node not found in parent/u);
});

for (const [name, getNodes] of [
["query", getChildNodesForFieldName],
["utility", getChildren],
] as const) {
test(`${name} traversal releases the cursor after collecting children`, () => {
const node = tree.rootNode.descendantsOfType("map_lit")[0];
assert.deepEqual(
getNodes(node, "value").map((child) => child.text),
[":a", "1", ":b", "2"],
);
});

test(`${name} traversal releases the cursor for a leaf`, () => {
const node = tree.rootNode.descendantsOfType("num_lit")[0];
assert.deepEqual(getNodes(node, "value"), []);
});
}
});
22 changes: 13 additions & 9 deletions packages/lib-engine/src/util/treeSitterUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,25 @@ export function getChildNodesForFieldName(
fieldName: string,
): Node[] {
const treeCursor = node.walk();
treeCursor.gotoFirstChild();
try {
treeCursor.gotoFirstChild();

const ret = [];
const ret = [];

let hasNext = true;
let hasNext = true;

while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
ret.push(treeCursor.currentNode);
while (hasNext) {
if (treeCursor.currentFieldName === fieldName) {
ret.push(treeCursor.currentNode);
}

hasNext = treeCursor.gotoNextSibling();
}

hasNext = treeCursor.gotoNextSibling();
return ret;
} finally {
treeCursor.delete();
}

return ret;
}

/**
Expand Down
55 changes: 47 additions & 8 deletions packages/test-vscode-e2e/src/suite/recorded.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import {
} from "@cursorless/lib-node-common";
import {
getReusableEditor,
getParseTreeApi,
getTestHelpers,
resetReusableEditor,
runCursorlessCommand,
} from "@cursorless/lib-vscode-common";
import { endToEndTestSetup, sleepWithBackoff } from "../endToEndTestSetup";
Expand All @@ -36,19 +38,56 @@ suite("recorded test cases", function () {
this.ctx.skip();
}

await runRecordedTest({
path,
spyIde: getSpy(),
openNewTestEditor,
sleepWithBackoff,
testHelpers: await getTestHelpers(),
runCursorlessCommand,
});
try {
await runRecordedTest({
path,
spyIde: getSpy(),
openNewTestEditor,
sleepWithBackoff,
testHelpers: await getTestHelpers(),
runCursorlessCommand,
});
} catch (error) {
// A retry must not inherit the failed attempt's selections or
// incrementally updated parse tree.
resetReusableEditor();
await logFailureContext(name);
throw error;
}
}),
);
}
});

async function logFailureContext(name: string): Promise<void> {
const editor = vscode.window.activeTextEditor;
if (editor == null) {
console.error(`Recorded test failed: ${name}; no active editor`);
return;
}

const { document, selections } = editor;
console.error(`Recorded test failed: ${name}`, {
uri: document.uri.toString(),
languageId: document.languageId,
version: document.version,
content: document.getText(),
selections: selections.map(({ anchor, active }) => ({ anchor, active })),
});

try {
const parseTree = await getParseTreeApi();
console.error(
"Parse tree:",
parseTree.getTreeForUri(document.uri).rootNode.toString(),
);
} catch (error) {
// Diagnostics should preserve the original test failure, including for
// languages without Tree-sitter support.
console.error("Could not read parse tree:", error);
}
}

async function openNewTestEditor(
content: string,
languageId: string,
Expand Down
Loading