Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree-sitter rolling fixes, 1.120 edition #1062

Merged
merged 8 commits into from
Aug 16, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ parser: 'tree-sitter-hyperlink'

injectionRegex: 'hyperlink'
treeSitter:
parserSource: 'github:savetheclocktower/tree-sitter-hyperlink#04c3a667ba432236578ac99bbacd0412f88d6fac'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 (re: 8090558)

grammar: 'ts/tree-sitter-hyperlink.wasm'
highlightsQuery: 'ts/highlights.scm'
Binary file not shown.
24 changes: 20 additions & 4 deletions packages/language-javascript/grammars/tree-sitter/indents.scm
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,32 @@
(#is? test.last true))
(#set! indent.matchIndentOf parent.startPosition))

; By default, `case` and `default` need to be indented one level more than their containing
; `switch`.
([(switch_case "case" @match) (switch_default "default" @match)]
; By default, `case` and `default` need to be indented one level more than
; their containing `switch`.
([
(switch_case "case" @match)
(switch_default "default" @match)
; We include this one (and check for a switch_statment ancestor) to handle
; a commonly encountered error state when the user is in the middle of typing
; a switch statement.
(ERROR "case" @match)
]
(#is? test.descendantOfType switch_statement)
(#set! indent.matchIndentOf parent.parent.startPosition)
(#set! indent.offsetIndent 1)
(#is-not? test.config "language-javascript.indentation.alignCaseWithSwitch"))

; When this config setting is enabled, `case` and `default` need to be indented
; to match their containing `switch`.
([(switch_case "case" @match) (switch_default "default" @match)]
([
(switch_case "case" @match)
(switch_default "default" @match)
; We include this one (and check for a switch_statment ancestor) to handle
; a commonly encountered error state when the user is in the middle of typing
; a switch statement.
(ERROR "case" @match)
]
(#is? test.descendantOfType switch_statement)
(#set! indent.matchIndentOf parent.parent.startPosition)
(#set! indent.offsetIndent 0)
(#is? test.config "language-javascript.indentation.alignCaseWithSwitch"))
Expand Down
3 changes: 3 additions & 0 deletions packages/language-typescript/grammars/common/highlights.scm
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,9 @@
"new" @keyword.operator.new._LANG_

"=" @keyword.operator.assignment._LANG_

["&" "|" "<<" ">>" ">>>" "~" "^"] @keyword.operator.bitwise.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(re: af133b0) Was going to ask if we should include bitwise assignment operators somehow, but I see they are already there later in the file...

I guess that's the part we did already. Heh.

Well, good to get these as well!

(For reference, if we want to check if we've got all of these, though I'm not requesting that as a reviewer for the present PR. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators)


(non_null_expression "!" @keyword.operator.non-null._LANG_)
(unary_expression "!" @keyword.operator.unary._LANG_)

Expand Down
20 changes: 18 additions & 2 deletions packages/language-typescript/grammars/common/indents.scm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(re 11e2f75)

handle a commonly encountered error state when the user is in the middle of typing a switch statement.

Sounds reasonable to me!

Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,31 @@

; By default, `case` and `default` need to be indented one level more than their containing
; `switch`.
([(switch_case "case" @match) (switch_default "default" @match)]
([
(switch_case "case" @match)
(switch_default "default" @match)
; We include this one (and check for a switch_statment ancestor) to handle
; a commonly encountered error state when the user is in the middle of typing
; a switch statement.
(ERROR "case" @match)
]
(#is? test.descendantOfType switch_statement)
(#set! indent.matchIndentOf parent.parent.startPosition)
(#set! indent.offsetIndent 1)
(#is-not? test.config "language-typescript.indentation.alignCaseWithSwitch"))

; When this config setting is enabled, `case` and `default` need to be indented
; to match their containing `switch`.

([(switch_case "case" @match) (switch_default "default" @match)]
([
(switch_case "case" @match)
(switch_default "default" @match)
; We include this one (and check for a switch_statment ancestor) to handle
; a commonly encountered error state when the user is in the middle of typing
; a switch statement.
(ERROR "case" @match)
]
(#is? test.descendantOfType switch_statement)
(#set! indent.matchIndentOf parent.parent.startPosition)
(#set! indent.offsetIndent 0)
(#is? test.config "language-typescript.indentation.alignCaseWithSwitch"))
Expand Down
11 changes: 8 additions & 3 deletions packages/symbol-provider-tree-sitter/lib/tree-sitter-provider.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
const CaptureOrganizer = require('./capture-organizer');
const { Emitter } = require('atom');

function layerHasTagsQuery(layer) {
return layer.queries?.tagsQuery ?? layer.tagsQuery;
}
Comment on lines +4 to +6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(re: 6846b9c)

Had to brush up on optional chaining to see that this checks out. It does appear to check out as valid use of optional chaining.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is me being incredibly paranoid, since we'd only fall back to the right side of that ?? expression if someone extracted the newer version of this package and added it to an older version of Pulsar.


class TreeSitterProvider {
constructor() {
this.packageName = 'symbol-provider-tree-sitter';
Expand Down Expand Up @@ -37,7 +41,7 @@ class TreeSitterProvider {
}

// This provider needs at least one layer to have a tags query.
let layers = languageMode.getAllLanguageLayers(l => !!l.tagsQuery);
let layers = languageMode.getAllLanguageLayers(layerHasTagsQuery);
if (layers.length === 0) {
return false;
}
Expand All @@ -64,13 +68,14 @@ class TreeSitterProvider {
// The symbols-view package might've cancelled us in the interim.
if (signal.aborted) return null;

let layers = languageMode.getAllLanguageLayers(l => !!l.tagsQuery);
let layers = languageMode.getAllLanguageLayers(layerHasTagsQuery);
if (layers.length === 0) return null;

for (let layer of layers) {
let extent = layer.getExtent();

let captures = layer.tagsQuery.captures(
let tagsQuery = layer.queries?.tagsQuery ?? layer.tagsQuery;
let captures = tagsQuery.captures(
layer.tree.rootNode,
extent.start,
extent.end
Expand Down
195 changes: 195 additions & 0 deletions spec/wasm-tree-sitter-language-mode-spec.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(re 4388bce)

Specs are appreciated! 👍

Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,59 @@ describe('WASMTreeSitterLanguageMode', () => {
]);
});

describe('when a highlighting query changes after load', () => {
it('updates the highlighting to reflect the new content', async () => {
jasmine.useRealClock();
const grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

await grammar.setQueryForTest('highlightsQuery', scm`
(identifier) @variable
`);

buffer.setText('abc;');

const languageMode = new WASMTreeSitterLanguageMode({
buffer,
grammar
});
buffer.setLanguageMode(languageMode);
await languageMode.ready;
await wait(0);

expectTokensToEqual(editor, [
[
{ text: 'abc', scopes: ['variable'] },
{ text: ';', scopes: [] }
]
]);

// Set up a promise that resolves when highlighting updates after a
// query change.
let highlightingDidUpdate = new Promise((resolve) => {
let disposable = languageMode.onDidChangeHighlighting(
() => {
disposable.dispose();
resolve();
}
)
});

// Change the highlighting query.
await grammar.setQueryForTest('highlightsQuery', scm`
(identifier) @constant
`);
await highlightingDidUpdate;

// The language mode should automatically reload the query.
expectTokensToEqual(editor, [
[
{ text: 'abc', scopes: ['constant'] },
{ text: ';', scopes: [] }
]
]);
});
});

// TODO: Ignoring these specs because web-tree-sitter doesn't seem to do
// async. We can rehabilitate them if we ever figure it out.
xdescribe('when the buffer changes during a parse', () => {
Expand Down Expand Up @@ -1750,6 +1803,53 @@ describe('WASMTreeSitterLanguageMode', () => {

expect(Array.from(map.values())).toEqual([0, 1, 1, 2, 1, 0]);
});

it('works correctly when straddling an injection boundary, even in the presence of whitespace', async () => {
const jsGrammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

jsGrammar.addInjectionPoint(HTML_TEMPLATE_LITERAL_INJECTION_POINT);

const htmlGrammar = new WASMTreeSitterGrammar(
atom.grammars,
htmlGrammarPath,
htmlConfig
);

htmlGrammar.addInjectionPoint(SCRIPT_TAG_INJECTION_POINT);

atom.grammars.addGrammar(jsGrammar);
atom.grammars.addGrammar(htmlGrammar);

// This is just like the test above, except that we're indented a bit.
// Now the edge of the injection isn't at the beginning of the line; it's
// at the beginning of the first _text_ on the line.
buffer.setText(dedent`
<html>
<head>
<script>
let foo;
if (foo) {
debug(true);
}
</script>
</head>
</html>
`);

const languageMode = new WASMTreeSitterLanguageMode({
grammar: htmlGrammar,
buffer,
config: atom.config,
grammars: atom.grammars
});

buffer.setLanguageMode(languageMode);
await languageMode.ready;

let map = languageMode.suggestedIndentForBufferRows(0, 9, editor.getTabLength());

expect(Array.from(map.values())).toEqual([0, 1, 2, 3, 3, 4, 3, 2, 1, 0]);
})
});

describe('folding', () => {
Expand Down Expand Up @@ -2208,6 +2308,101 @@ describe('WASMTreeSitterLanguageMode', () => {
`);
});

it('does not enumerate redundant folds', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

await grammar.setQueryForTest('foldsQuery', `
(statement_block) @fold
(object) @fold
`);

// This odd way of formatting code produces a scenario where two folds
// would start on the same line. The second of the two folds would never
// be seen when toggling the fold on that line, so we shouldn't treat it
// as a valid fold for any other purpose.
buffer.setText(dedent`
if (foo) {results.push({
bar: 'baz'
})}
`);

const languageMode = new WASMTreeSitterLanguageMode({ grammar, buffer });
buffer.setLanguageMode(languageMode);
await languageMode.ready;

let ranges = languageMode.getFoldableRanges();
expect(ranges.length).toBe(1);
})

it('is not flummoxed by redundant folds when performing foldAllAtIndentLevel', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

await grammar.setQueryForTest('foldsQuery', `
(statement_block) @fold
(object) @fold
`);

buffer.setText(dedent`
function foo() {
if (true) {
if (foo) {results.push({
bar: 'baz'
})}
}
}

function bar() {
if (false) {
// TODO
}
}
`);

const languageMode = new WASMTreeSitterLanguageMode({ grammar, buffer });
buffer.setLanguageMode(languageMode);
await languageMode.ready;

editor.foldAllAtIndentLevel(1);
expect(getDisplayText(editor)).toBe(dedent`
function foo() {
if (true) {…}
}

function bar() {
if (false) {…}
}
`);

buffer.setText(dedent`
function foo() {
if (true) {
if (foo) {
results.push({
bar: 'baz'
})}
}
}

function bar() {
if (false) {
// TODO
}
}
`);
await languageMode.atTransactionEnd();

editor.foldAllAtIndentLevel(1);
expect(getDisplayText(editor)).toBe(dedent`
function foo() {
if (true) {…}
}

function bar() {
if (false) {…}
}
`);
})

it('can target named vs anonymous nodes as fold boundaries', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, rubyGrammarPath, rubyConfig);

Expand Down
Loading
Loading