From 42affedab7856f07b7f90460f51727526da3845f Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 01:42:03 +0100 Subject: [PATCH 01/10] Increase performance for Tree sitter matches --- .../src/languages/LanguageDefinition.ts | 2 +- .../languages/TreeSitterQuery/QueryCapture.ts | 11 ++ .../TreeSitterQuery/TreeSitterQuery.ts | 183 +++++++++++------- .../TreeSitterQuery/normalizeCaptureName.ts | 3 + .../TreeSitterQuery/positionToPoint.ts | 6 + .../TreeSitterQuery/rewriteStartOfEndOf.ts | 44 +++-- 6 files changed, 163 insertions(+), 86 deletions(-) create mode 100644 packages/cursorless-engine/src/languages/TreeSitterQuery/normalizeCaptureName.ts create mode 100644 packages/cursorless-engine/src/languages/TreeSitterQuery/positionToPoint.ts diff --git a/packages/cursorless-engine/src/languages/LanguageDefinition.ts b/packages/cursorless-engine/src/languages/LanguageDefinition.ts index e2f2ee9086..93e93fb6d0 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinition.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinition.ts @@ -74,7 +74,7 @@ export class LanguageDefinition { * legacy pathways */ getScopeHandler(scopeType: ScopeType) { - if (!this.query.captureNames.includes(scopeType.type)) { + if (!this.query.hasCapture(scopeType.type)) { return undefined; } diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts index 9df81fa3df..46fbded782 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts @@ -49,6 +49,17 @@ export interface QueryCapture { hasError(): boolean; } +/** + * A capture of a query pattern against a syntax tree that can be modified. + */ +export interface ModifiableQueryCapture { + readonly name: string; + range: Range; + allowMultiple: boolean; + insertionDelimiter: string | undefined; + hasError(): boolean; +} + /** * A match of a query pattern against a syntax tree. */ diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index 69785ed0bf..8c83b199ec 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -1,19 +1,25 @@ import type { Position, TextDocument } from "@cursorless/common"; import { showError, type TreeSitter } from "@cursorless/common"; -import { groupBy, uniq } from "lodash-es"; -import type { Point, Query } from "web-tree-sitter"; +import type Parser from "web-tree-sitter"; +import type { Query } from "web-tree-sitter"; import { ide } from "../../singletons/ide.singleton"; import { getNodeRange } from "../../util/nodeSelectors"; import type { + ModifiableQueryCapture, + MutableQueryCapture, MutableQueryMatch, - QueryCapture, QueryMatch, } from "./QueryCapture"; import { checkCaptureStartEnd } from "./checkCaptureStartEnd"; import { isContainedInErrorNode } from "./isContainedInErrorNode"; +import { normalizeCaptureName } from "./normalizeCaptureName"; import { parsePredicates } from "./parsePredicates"; +import { positionToPoint } from "./positionToPoint"; import { predicateToString } from "./predicateToString"; -import { rewriteStartOfEndOf } from "./rewriteStartOfEndOf"; +import { + getStartOfEndOfRange, + rewriteStartOfEndOf, +} from "./rewriteStartOfEndOf"; import { treeSitterQueryCache } from "./treeSitterQueryCache"; /** @@ -67,6 +73,12 @@ export class TreeSitterQuery { return new TreeSitterQuery(treeSitter, query, predicates); } + hasCapture(name: string): boolean { + return this.query.captureNames.some( + (n) => normalizeCaptureName(n) === name, + ); + } + matches( document: TextDocument, start?: Position, @@ -84,74 +96,111 @@ export class TreeSitterQuery { start?: Position, end?: Position, ): QueryMatch[] { - return this.query - .matches(this.treeSitter.getTree(document).rootNode, { - startPosition: start == null ? undefined : positionToPoint(start), - endPosition: end == null ? undefined : positionToPoint(end), - }) - .map( - ({ pattern, captures }): MutableQueryMatch => ({ - patternIdx: pattern, - captures: captures.map(({ name, node }) => ({ - name, - node, - document, - range: getNodeRange(node), - insertionDelimiter: undefined, - allowMultiple: false, - hasError: () => isContainedInErrorNode(node), - })), - }), - ) - .filter((match) => - this.patternPredicates[match.patternIdx].every((predicate) => - predicate(match), - ), - ) - .map((match): QueryMatch => { - // Merge the ranges of all captures with the same name into a single - // range and return one capture with that name. We consider captures - // with names `@foo`, `@foo.start`, and `@foo.end` to have the same - // name, for which we'd return a capture with name `foo`. - const captures: QueryCapture[] = Object.entries( - groupBy(match.captures, ({ name }) => normalizeCaptureName(name)), - ).map(([name, captures]) => { - captures = rewriteStartOfEndOf(captures); - const capturesAreValid = checkCaptureStartEnd( - captures, - ide().messages, - ); - - if (!capturesAreValid && ide().runMode === "test") { - throw new Error("Invalid captures"); - } - - return { - name, - range: captures - .map(({ range }) => range) - .reduce((accumulator, range) => range.union(accumulator)), - allowMultiple: captures.some((capture) => capture.allowMultiple), - insertionDelimiter: captures.find( - (capture) => capture.insertionDelimiter != null, - )?.insertionDelimiter, - hasError: () => captures.some((capture) => capture.hasError()), - }; - }); - - return { ...match, captures }; - }); + const results: QueryMatch[] = []; + const isTesting = ide().runMode === "test"; + + const matches = this.query.matches( + this.treeSitter.getTree(document).rootNode, + { + startPosition: start != null ? positionToPoint(start) : undefined, + endPosition: end != null ? positionToPoint(end) : undefined, + }, + ); + + for (const match of matches) { + const mutableMatch = createMutableQueryMatch(document, match); + + if (this.runPredicates(mutableMatch)) { + results.push(createQueryMatch(mutableMatch, isTesting)); + } + } + + return results; } - get captureNames() { - return uniq(this.query.captureNames.map(normalizeCaptureName)); + private runPredicates(match: MutableQueryMatch): boolean { + for (const predicate of this.patternPredicates[match.patternIdx]) { + if (!predicate(match)) { + return false; + } + } + return true; } } -function normalizeCaptureName(name: string): string { - return name.replace(/(\.(start|end))?(\.(startOf|endOf))?$/, ""); +function createMutableQueryMatch( + document: TextDocument, + match: Parser.QueryMatch, +): MutableQueryMatch { + return { + patternIdx: match.pattern, + captures: match.captures.map(({ name, node }) => ({ + name, + node, + document, + range: getNodeRange(node), + insertionDelimiter: undefined, + allowMultiple: false, + hasError: () => isContainedInErrorNode(node), + })), + }; } -function positionToPoint(start: Position): Point { - return { row: start.line, column: start.character }; +function createQueryMatch( + match: MutableQueryMatch, + isTesting: boolean, +): QueryMatch { + const result: ModifiableQueryCapture[] = []; + const resultMap = new Map< + string, + { acc: ModifiableQueryCapture; captures: MutableQueryCapture[] } + >(); + + // Merge the ranges of all captures with the same name into a single + // range and return one capture with that name. We consider captures + // with names `@foo`, `@foo.start`, and `@foo.end` to have the same + // name, for which we'd return a capture with name `foo`. + + for (const capture of match.captures) { + const name = normalizeCaptureName(capture.name); + const range = getStartOfEndOfRange(capture); + const existing = resultMap.get(name); + + if (existing == null) { + const accumulator = { + name, + range, + allowMultiple: capture.allowMultiple, + insertionDelimiter: capture.insertionDelimiter, + hasError: () => capture.hasError(), + }; + result.push(accumulator); + resultMap.set(name, { + acc: accumulator, + captures: [capture], + }); + } else { + existing.acc.range = existing.acc.range.union(range); + existing.acc.allowMultiple = + existing.acc.allowMultiple || capture.allowMultiple; + existing.acc.insertionDelimiter = + existing.acc.insertionDelimiter ?? capture.insertionDelimiter; + existing.acc.hasError = () => existing.captures.some((c) => c.hasError()); + existing.captures.push(capture); + } + } + + if (isTesting) { + for (const captureGroup of resultMap.values()) { + const capturesAreValid = checkCaptureStartEnd( + rewriteStartOfEndOf(captureGroup.captures), + ide().messages, + ); + if (!capturesAreValid) { + throw new Error("Invalid captures"); + } + } + } + + return { captures: result }; } diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/normalizeCaptureName.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/normalizeCaptureName.ts new file mode 100644 index 0000000000..5322ff1556 --- /dev/null +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/normalizeCaptureName.ts @@ -0,0 +1,3 @@ +export function normalizeCaptureName(name: string): string { + return name.replace(/(\.(start|end))?(\.(startOf|endOf))?$/, ""); +} diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/positionToPoint.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/positionToPoint.ts new file mode 100644 index 0000000000..af5650a309 --- /dev/null +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/positionToPoint.ts @@ -0,0 +1,6 @@ +import type { Position } from "@cursorless/common"; +import type { Point } from "web-tree-sitter"; + +export function positionToPoint(start: Position): Point { + return { row: start.line, column: start.character }; +} diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/rewriteStartOfEndOf.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/rewriteStartOfEndOf.ts index 463ba9a15c..76fc022411 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/rewriteStartOfEndOf.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/rewriteStartOfEndOf.ts @@ -1,3 +1,4 @@ +import type { Range } from "@cursorless/common"; import type { MutableQueryCapture } from "./QueryCapture"; /** @@ -11,22 +12,29 @@ import type { MutableQueryCapture } from "./QueryCapture"; export function rewriteStartOfEndOf( captures: MutableQueryCapture[], ): MutableQueryCapture[] { - return captures.map((capture) => { - // Remove trailing .startOf and .endOf, adjusting ranges. - if (capture.name.endsWith(".startOf")) { - return { - ...capture, - name: capture.name.replace(/\.startOf$/, ""), - range: capture.range.start.toEmptyRange(), - }; - } - if (capture.name.endsWith(".endOf")) { - return { - ...capture, - name: capture.name.replace(/\.endOf$/, ""), - range: capture.range.end.toEmptyRange(), - }; - } - return capture; - }); + return captures.map((capture) => ({ + ...capture, + range: getStartOfEndOfRange(capture), + name: getStartOfEndOfName(capture), + })); +} + +export function getStartOfEndOfRange(capture: MutableQueryCapture): Range { + if (capture.name.endsWith(".startOf")) { + return capture.range.start.toEmptyRange(); + } + if (capture.name.endsWith(".endOf")) { + return capture.range.end.toEmptyRange(); + } + return capture.range; +} + +function getStartOfEndOfName(capture: MutableQueryCapture): string { + if (capture.name.endsWith(".startOf")) { + return capture.name.slice(0, -8); + } + if (capture.name.endsWith(".endOf")) { + return capture.name.slice(0, -6); + } + return capture.name; } From 56c856ac833dce07ad172b493b0b2805d8330767 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 01:53:06 +0100 Subject: [PATCH 02/10] clean up --- .../src/languages/TreeSitterQuery/TreeSitterQuery.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index 8c83b199ec..f39a739119 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -96,8 +96,8 @@ export class TreeSitterQuery { start?: Position, end?: Position, ): QueryMatch[] { - const results: QueryMatch[] = []; const isTesting = ide().runMode === "test"; + const results: QueryMatch[] = []; const matches = this.query.matches( this.treeSitter.getTree(document).rootNode, From 98b02dba5222ebec8fcb4c8254942de416fca536 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 02:27:17 +0100 Subject: [PATCH 03/10] Simplification --- .../src/languages/TreeSitterQuery/QueryCapture.ts | 11 ----------- .../src/languages/TreeSitterQuery/TreeSitterQuery.ts | 9 +++------ 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts index 46fbded782..9df81fa3df 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/QueryCapture.ts @@ -49,17 +49,6 @@ export interface QueryCapture { hasError(): boolean; } -/** - * A capture of a query pattern against a syntax tree that can be modified. - */ -export interface ModifiableQueryCapture { - readonly name: string; - range: Range; - allowMultiple: boolean; - insertionDelimiter: string | undefined; - hasError(): boolean; -} - /** * A match of a query pattern against a syntax tree. */ diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index f39a739119..b9e6c5e269 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -5,7 +5,6 @@ import type { Query } from "web-tree-sitter"; import { ide } from "../../singletons/ide.singleton"; import { getNodeRange } from "../../util/nodeSelectors"; import type { - ModifiableQueryCapture, MutableQueryCapture, MutableQueryMatch, QueryMatch, @@ -150,10 +149,10 @@ function createQueryMatch( match: MutableQueryMatch, isTesting: boolean, ): QueryMatch { - const result: ModifiableQueryCapture[] = []; + const result: MutableQueryCapture[] = []; const resultMap = new Map< string, - { acc: ModifiableQueryCapture; captures: MutableQueryCapture[] } + { acc: MutableQueryCapture; captures: MutableQueryCapture[] } >(); // Merge the ranges of all captures with the same name into a single @@ -168,11 +167,9 @@ function createQueryMatch( if (existing == null) { const accumulator = { + ...capture, name, range, - allowMultiple: capture.allowMultiple, - insertionDelimiter: capture.insertionDelimiter, - hasError: () => capture.hasError(), }; result.push(accumulator); resultMap.set(name, { From c9e0c37f7945d3f01d004f6fafd5c6c306a35f65 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 03:13:14 +0100 Subject: [PATCH 04/10] Refactoring --- .../TreeSitterQuery/TreeSitterQuery.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index b9e6c5e269..7e3c8f4e1f 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -95,7 +95,7 @@ export class TreeSitterQuery { start?: Position, end?: Position, ): QueryMatch[] { - const isTesting = ide().runMode === "test"; + const checkCaptures = ide().runMode !== "production"; const results: QueryMatch[] = []; const matches = this.query.matches( @@ -110,7 +110,7 @@ export class TreeSitterQuery { const mutableMatch = createMutableQueryMatch(document, match); if (this.runPredicates(mutableMatch)) { - results.push(createQueryMatch(mutableMatch, isTesting)); + results.push(createQueryMatch(mutableMatch, checkCaptures)); } } @@ -147,10 +147,10 @@ function createMutableQueryMatch( function createQueryMatch( match: MutableQueryMatch, - isTesting: boolean, + checkCaptures: boolean, ): QueryMatch { const result: MutableQueryCapture[] = []; - const resultMap = new Map< + const map = new Map< string, { acc: MutableQueryCapture; captures: MutableQueryCapture[] } >(); @@ -163,37 +163,35 @@ function createQueryMatch( for (const capture of match.captures) { const name = normalizeCaptureName(capture.name); const range = getStartOfEndOfRange(capture); - const existing = resultMap.get(name); + const existing = map.get(name); if (existing == null) { - const accumulator = { + const captures = [capture]; + const acc = { ...capture, name, range, + hasError: () => captures.some((c) => c.hasError()), }; - result.push(accumulator); - resultMap.set(name, { - acc: accumulator, - captures: [capture], - }); + result.push(acc); + map.set(name, { acc, captures }); } else { existing.acc.range = existing.acc.range.union(range); existing.acc.allowMultiple = existing.acc.allowMultiple || capture.allowMultiple; existing.acc.insertionDelimiter = existing.acc.insertionDelimiter ?? capture.insertionDelimiter; - existing.acc.hasError = () => existing.captures.some((c) => c.hasError()); existing.captures.push(capture); } } - if (isTesting) { - for (const captureGroup of resultMap.values()) { + if (checkCaptures) { + for (const captureGroup of map.values()) { const capturesAreValid = checkCaptureStartEnd( rewriteStartOfEndOf(captureGroup.captures), ide().messages, ); - if (!capturesAreValid) { + if (!capturesAreValid && ide().runMode === "test") { throw new Error("Invalid captures"); } } From 65270f8bd0157253f9417d8ebfe248e14b3d9fc8 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 03:15:11 +0100 Subject: [PATCH 05/10] Clean up --- .../src/languages/TreeSitterQuery/TreeSitterQuery.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index 7e3c8f4e1f..ae7d38b9a8 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -1,7 +1,6 @@ import type { Position, TextDocument } from "@cursorless/common"; import { showError, type TreeSitter } from "@cursorless/common"; import type Parser from "web-tree-sitter"; -import type { Query } from "web-tree-sitter"; import { ide } from "../../singletons/ide.singleton"; import { getNodeRange } from "../../util/nodeSelectors"; import type { @@ -32,7 +31,7 @@ export class TreeSitterQuery { /** * The raw tree-sitter query as parsed by tree-sitter from the query file */ - private query: Query, + private query: Parser.Query, /** * The predicates for each pattern in the query. Each element of the outer @@ -42,7 +41,11 @@ export class TreeSitterQuery { private patternPredicates: ((match: MutableQueryMatch) => boolean)[][], ) {} - static create(languageId: string, treeSitter: TreeSitter, query: Query) { + static create( + languageId: string, + treeSitter: TreeSitter, + query: Parser.Query, + ) { const { errors, predicates } = parsePredicates(query.predicates); if (errors.length > 0) { From 30ae94ec38fb14784e5a227247253afefa0545c8 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 10 Jan 2025 03:16:27 +0100 Subject: [PATCH 06/10] Rename imports --- .../src/languages/TreeSitterQuery/TreeSitterQuery.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index ae7d38b9a8..afd43a9c32 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -1,6 +1,6 @@ import type { Position, TextDocument } from "@cursorless/common"; import { showError, type TreeSitter } from "@cursorless/common"; -import type Parser from "web-tree-sitter"; +import type * as treeSitter from "web-tree-sitter"; import { ide } from "../../singletons/ide.singleton"; import { getNodeRange } from "../../util/nodeSelectors"; import type { @@ -31,7 +31,7 @@ export class TreeSitterQuery { /** * The raw tree-sitter query as parsed by tree-sitter from the query file */ - private query: Parser.Query, + private query: treeSitter.Query, /** * The predicates for each pattern in the query. Each element of the outer @@ -44,7 +44,7 @@ export class TreeSitterQuery { static create( languageId: string, treeSitter: TreeSitter, - query: Parser.Query, + query: treeSitter.Query, ) { const { errors, predicates } = parsePredicates(query.predicates); @@ -132,7 +132,7 @@ export class TreeSitterQuery { function createMutableQueryMatch( document: TextDocument, - match: Parser.QueryMatch, + match: treeSitter.QueryMatch, ): MutableQueryMatch { return { patternIdx: match.pattern, From 9b3a05f393a5f114ca7ea922f8f9b7e343b87804 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sat, 11 Jan 2025 06:47:14 +0100 Subject: [PATCH 07/10] Small refactoring --- .../TreeSitterQuery/TreeSitterQuery.ts | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index afd43a9c32..96ae37eac5 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -98,28 +98,35 @@ export class TreeSitterQuery { start?: Position, end?: Position, ): QueryMatch[] { - const checkCaptures = ide().runMode !== "production"; + const shouldCheckCaptures = ide().runMode !== "production"; + const matches = this.getTreeMatches(document, start, end); const results: QueryMatch[] = []; - const matches = this.query.matches( - this.treeSitter.getTree(document).rootNode, - { - startPosition: start != null ? positionToPoint(start) : undefined, - endPosition: end != null ? positionToPoint(end) : undefined, - }, - ); - for (const match of matches) { const mutableMatch = createMutableQueryMatch(document, match); - if (this.runPredicates(mutableMatch)) { - results.push(createQueryMatch(mutableMatch, checkCaptures)); + if (!this.runPredicates(mutableMatch)) { + continue; } + + results.push(createQueryMatch(mutableMatch, shouldCheckCaptures)); } return results; } + private getTreeMatches( + document: TextDocument, + start?: Position, + end?: Position, + ) { + const { rootNode } = this.treeSitter.getTree(document); + return this.query.matches(rootNode, { + startPosition: start != null ? positionToPoint(start) : undefined, + endPosition: end != null ? positionToPoint(end) : undefined, + }); + } + private runPredicates(match: MutableQueryMatch): boolean { for (const predicate of this.patternPredicates[match.patternIdx]) { if (!predicate(match)) { @@ -150,7 +157,7 @@ function createMutableQueryMatch( function createQueryMatch( match: MutableQueryMatch, - checkCaptures: boolean, + shouldCheckCaptures: boolean, ): QueryMatch { const result: MutableQueryCapture[] = []; const map = new Map< @@ -188,17 +195,21 @@ function createQueryMatch( } } - if (checkCaptures) { - for (const captureGroup of map.values()) { - const capturesAreValid = checkCaptureStartEnd( - rewriteStartOfEndOf(captureGroup.captures), - ide().messages, - ); - if (!capturesAreValid && ide().runMode === "test") { - throw new Error("Invalid captures"); - } - } + if (shouldCheckCaptures) { + checkCaptures(Array.from(map.values())); } return { captures: result }; } + +function checkCaptures(matches: { captures: MutableQueryCapture[] }[]) { + for (const match of matches) { + const capturesAreValid = checkCaptureStartEnd( + rewriteStartOfEndOf(match.captures), + ide().messages, + ); + if (!capturesAreValid && ide().runMode === "test") { + throw new Error("Invalid captures"); + } + } +} From 1bd3ccaa5225eddef37421f4acdb097faa32e1dc Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sat, 11 Jan 2025 06:52:23 +0100 Subject: [PATCH 08/10] Move parse predicates with error handling to separate file --- .../TreeSitterQuery/TreeSitterQuery.ts | 31 ++------------- .../parsePredicatesWithErrorHandling.ts | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+), 28 deletions(-) create mode 100644 packages/cursorless-engine/src/languages/TreeSitterQuery/parsePredicatesWithErrorHandling.ts diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index 96ae37eac5..174991a5b8 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -1,5 +1,5 @@ import type { Position, TextDocument } from "@cursorless/common"; -import { showError, type TreeSitter } from "@cursorless/common"; +import { type TreeSitter } from "@cursorless/common"; import type * as treeSitter from "web-tree-sitter"; import { ide } from "../../singletons/ide.singleton"; import { getNodeRange } from "../../util/nodeSelectors"; @@ -11,9 +11,8 @@ import type { import { checkCaptureStartEnd } from "./checkCaptureStartEnd"; import { isContainedInErrorNode } from "./isContainedInErrorNode"; import { normalizeCaptureName } from "./normalizeCaptureName"; -import { parsePredicates } from "./parsePredicates"; +import { parsePredicatesWithErrorHandling } from "./parsePredicatesWithErrorHandling"; import { positionToPoint } from "./positionToPoint"; -import { predicateToString } from "./predicateToString"; import { getStartOfEndOfRange, rewriteStartOfEndOf, @@ -46,31 +45,7 @@ export class TreeSitterQuery { treeSitter: TreeSitter, query: treeSitter.Query, ) { - const { errors, predicates } = parsePredicates(query.predicates); - - if (errors.length > 0) { - for (const error of errors) { - const context = [ - `language ${languageId}`, - `pattern ${error.patternIdx}`, - `predicate \`${predicateToString( - query.predicates[error.patternIdx][error.predicateIdx], - )}\``, - ].join(", "); - - void showError( - ide().messages, - "TreeSitterQuery.parsePredicates", - `Error parsing predicate for ${context}: ${error.error}`, - ); - } - - // We show errors to the user, but we don't want to crash the extension - // unless we're in test mode - if (ide().runMode === "test") { - throw new Error("Invalid predicates"); - } - } + const predicates = parsePredicatesWithErrorHandling(languageId, query); return new TreeSitterQuery(treeSitter, query, predicates); } diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/parsePredicatesWithErrorHandling.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/parsePredicatesWithErrorHandling.ts new file mode 100644 index 0000000000..6798939249 --- /dev/null +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/parsePredicatesWithErrorHandling.ts @@ -0,0 +1,38 @@ +import { showError } from "@cursorless/common"; +import type { Query } from "web-tree-sitter"; +import { ide } from "../../singletons/ide.singleton"; +import { parsePredicates } from "./parsePredicates"; +import { predicateToString } from "./predicateToString"; + +export function parsePredicatesWithErrorHandling( + languageId: string, + query: Query, +) { + const { errors, predicates } = parsePredicates(query.predicates); + + if (errors.length > 0) { + for (const error of errors) { + const context = [ + `language ${languageId}`, + `pattern ${error.patternIdx}`, + `predicate \`${predicateToString( + query.predicates[error.patternIdx][error.predicateIdx], + )}\``, + ].join(", "); + + void showError( + ide().messages, + "TreeSitterQuery.parsePredicates", + `Error parsing predicate for ${context}: ${error.error}`, + ); + } + + // We show errors to the user, but we don't want to crash the extension + // unless we're in test mode + if (ide().runMode === "test") { + throw new Error("Invalid predicates"); + } + } + + return predicates; +} From 0ba887872f7ba13afcf46664509d0c1ba9f148c9 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sat, 11 Jan 2025 07:00:47 +0100 Subject: [PATCH 09/10] More cleanup --- .../TreeSitterQuery/TreeSitterQuery.ts | 148 +++++++++--------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index 174991a5b8..a522f5560b 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -24,6 +24,8 @@ import { treeSitterQueryCache } from "./treeSitterQueryCache"; * defines our own custom predicate operators */ export class TreeSitterQuery { + private shouldCheckCaptures: boolean; + private constructor( private treeSitter: TreeSitter, @@ -38,7 +40,9 @@ export class TreeSitterQuery { * corresponds to a predicate for that pattern. */ private patternPredicates: ((match: MutableQueryMatch) => boolean)[][], - ) {} + ) { + this.shouldCheckCaptures = ide().runMode !== "production"; + } static create( languageId: string, @@ -73,18 +77,17 @@ export class TreeSitterQuery { start?: Position, end?: Position, ): QueryMatch[] { - const shouldCheckCaptures = ide().runMode !== "production"; const matches = this.getTreeMatches(document, start, end); const results: QueryMatch[] = []; for (const match of matches) { - const mutableMatch = createMutableQueryMatch(document, match); + const mutableMatch = this.createMutableQueryMatch(document, match); if (!this.runPredicates(mutableMatch)) { continue; } - results.push(createQueryMatch(mutableMatch, shouldCheckCaptures)); + results.push(this.createQueryMatch(mutableMatch)); } return results; @@ -102,6 +105,24 @@ export class TreeSitterQuery { }); } + private createMutableQueryMatch( + document: TextDocument, + match: treeSitter.QueryMatch, + ): MutableQueryMatch { + return { + patternIdx: match.pattern, + captures: match.captures.map(({ name, node }) => ({ + name, + node, + document, + range: getNodeRange(node), + insertionDelimiter: undefined, + allowMultiple: false, + hasError: () => isContainedInErrorNode(node), + })), + }; + } + private runPredicates(match: MutableQueryMatch): boolean { for (const predicate of this.patternPredicates[match.patternIdx]) { if (!predicate(match)) { @@ -110,81 +131,60 @@ export class TreeSitterQuery { } return true; } -} - -function createMutableQueryMatch( - document: TextDocument, - match: treeSitter.QueryMatch, -): MutableQueryMatch { - return { - patternIdx: match.pattern, - captures: match.captures.map(({ name, node }) => ({ - name, - node, - document, - range: getNodeRange(node), - insertionDelimiter: undefined, - allowMultiple: false, - hasError: () => isContainedInErrorNode(node), - })), - }; -} -function createQueryMatch( - match: MutableQueryMatch, - shouldCheckCaptures: boolean, -): QueryMatch { - const result: MutableQueryCapture[] = []; - const map = new Map< - string, - { acc: MutableQueryCapture; captures: MutableQueryCapture[] } - >(); - - // Merge the ranges of all captures with the same name into a single - // range and return one capture with that name. We consider captures - // with names `@foo`, `@foo.start`, and `@foo.end` to have the same - // name, for which we'd return a capture with name `foo`. - - for (const capture of match.captures) { - const name = normalizeCaptureName(capture.name); - const range = getStartOfEndOfRange(capture); - const existing = map.get(name); - - if (existing == null) { - const captures = [capture]; - const acc = { - ...capture, - name, - range, - hasError: () => captures.some((c) => c.hasError()), - }; - result.push(acc); - map.set(name, { acc, captures }); - } else { - existing.acc.range = existing.acc.range.union(range); - existing.acc.allowMultiple = - existing.acc.allowMultiple || capture.allowMultiple; - existing.acc.insertionDelimiter = - existing.acc.insertionDelimiter ?? capture.insertionDelimiter; - existing.captures.push(capture); + private createQueryMatch(match: MutableQueryMatch): QueryMatch { + const result: MutableQueryCapture[] = []; + const map = new Map< + string, + { acc: MutableQueryCapture; captures: MutableQueryCapture[] } + >(); + + // Merge the ranges of all captures with the same name into a single + // range and return one capture with that name. We consider captures + // with names `@foo`, `@foo.start`, and `@foo.end` to have the same + // name, for which we'd return a capture with name `foo`. + + for (const capture of match.captures) { + const name = normalizeCaptureName(capture.name); + const range = getStartOfEndOfRange(capture); + const existing = map.get(name); + + if (existing == null) { + const captures = [capture]; + const acc = { + ...capture, + name, + range, + hasError: () => captures.some((c) => c.hasError()), + }; + result.push(acc); + map.set(name, { acc, captures }); + } else { + existing.acc.range = existing.acc.range.union(range); + existing.acc.allowMultiple = + existing.acc.allowMultiple || capture.allowMultiple; + existing.acc.insertionDelimiter = + existing.acc.insertionDelimiter ?? capture.insertionDelimiter; + existing.captures.push(capture); + } } - } - if (shouldCheckCaptures) { - checkCaptures(Array.from(map.values())); - } + // if (this.shouldCheckCaptures) { + // this.checkCaptures(Array.from(map.values())); + // } - return { captures: result }; -} + return { captures: result }; + } -function checkCaptures(matches: { captures: MutableQueryCapture[] }[]) { - for (const match of matches) { - const capturesAreValid = checkCaptureStartEnd( - rewriteStartOfEndOf(match.captures), - ide().messages, - ); - if (!capturesAreValid && ide().runMode === "test") { - throw new Error("Invalid captures"); + private checkCaptures(matches: { captures: MutableQueryCapture[] }[]) { + for (const match of matches) { + const capturesAreValid = checkCaptureStartEnd( + rewriteStartOfEndOf(match.captures), + ide().messages, + ); + if (!capturesAreValid && ide().runMode === "test") { + throw new Error("Invalid captures"); + } } } } From 76c87f60a279e7f7ac4146925624851f12989f62 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sat, 11 Jan 2025 07:01:08 +0100 Subject: [PATCH 10/10] Activate test --- .../src/languages/TreeSitterQuery/TreeSitterQuery.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts index a522f5560b..60d3763a02 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/TreeSitterQuery.ts @@ -169,9 +169,9 @@ export class TreeSitterQuery { } } - // if (this.shouldCheckCaptures) { - // this.checkCaptures(Array.from(map.values())); - // } + if (this.shouldCheckCaptures) { + this.checkCaptures(Array.from(map.values())); + } return { captures: result }; }