From cca25099c4a154137de962a60dcf2fba74c6cc31 Mon Sep 17 00:00:00 2001 From: David Kincaid Date: Sat, 29 Jun 2024 13:08:24 -0700 Subject: [PATCH] More Formatter Fixes (#672) * Fix nodepath function highlighting/tokenization * Reverted dangerous line removal behavior change * Fix detection of match keyword vs .match() function * Rearrange formatter options * Fix option default value * Add biome linter/formatter config file * Fix linter errors * Add system to supply custom config values in tests * Remove unused variable * Implement tests for both formatter options * Clean up formatter option handling * Fix extra space inserted in list of nodepaths * Add token rules for square and curly braces --- biome.json | 15 ++++ package.json | 18 ++--- src/formatter/formatter.test.ts | 26 ++++++- .../snapshots/max-empty-lines-1/config.json | 3 + .../in.gd | 0 .../out.gd | 3 - .../snapshots/max-empty-lines-2/in.gd | 28 ++++++++ .../snapshots/max-empty-lines-2/out.gd | 20 ++++++ src/formatter/symbols.ts | 1 - src/formatter/textmate.ts | 68 ++++++++----------- syntaxes/GDScript.tmLanguage.json | 34 +++++++++- syntaxes/examples/gdscript2.gd | 6 ++ 12 files changed, 164 insertions(+), 58 deletions(-) create mode 100644 biome.json create mode 100644 src/formatter/snapshots/max-empty-lines-1/config.json rename src/formatter/snapshots/{consecutive-empty-lines-are-removed => max-empty-lines-1}/in.gd (100%) rename src/formatter/snapshots/{consecutive-empty-lines-are-removed => max-empty-lines-1}/out.gd (97%) create mode 100644 src/formatter/snapshots/max-empty-lines-2/in.gd create mode 100644 src/formatter/snapshots/max-empty-lines-2/out.gd diff --git a/biome.json b/biome.json new file mode 100644 index 0000000..cdf5421 --- /dev/null +++ b/biome.json @@ -0,0 +1,15 @@ +{ + "formatter": { + "enabled": true, + "formatWithErrors": false, + "indentStyle": "tab", + "indentWidth": 4, + "lineWidth": 120, + "lineEnding": "lf", + "include": ["src/**/*.ts"] + }, + "files": { + "include": ["src/**/*.ts"], + "ignore": ["node_modules"] + } +} diff --git a/package.json b/package.json index 8163b0f..1f28ad6 100644 --- a/package.json +++ b/package.json @@ -271,23 +271,23 @@ "default": true, "description": "Whether to reveal the terminal when launching the Godot Editor" }, - "godotTools.formatter.emptyLinesBeforeFunctions": { + "godotTools.formatter.maxEmptyLines": { "type": "string", "enum": [ - "one", - "two" + "1", + "2" ], "enumDescriptions": [ - "One line before functions. A more compact style.", - "Two lines before functions. Conforms to the official GDScript style guide." + "1 empty line. A more compact style.", + "2 empty lines. Conforms to the official GDScript style guide." ], - "default": "two", - "description": "Number of blank lines to leave before functions." + "default": "2", + "description": "Number of empty lines allowed anywhere in the file" }, - "godotTools.formatter.denseFunctionDeclarations": { + "godotTools.formatter.denseFunctionParameters": { "type": "boolean", "default": false, - "description": "Whether extra space should be removed from function declarations" + "description": "Whether extra space should be removed from function parameter lists" }, "godotTools.lsp.serverProtocol": { "type": [ diff --git a/src/formatter/formatter.test.ts b/src/formatter/formatter.test.ts index 4a7c358..342b70a 100644 --- a/src/formatter/formatter.test.ts +++ b/src/formatter/formatter.test.ts @@ -2,7 +2,7 @@ import * as vscode from "vscode"; import * as path from "node:path"; import * as fs from "node:fs"; -import { format_document } from "./textmate"; +import { format_document, type FormatterOptions } from "./textmate"; import * as chai from "chai"; const expect = chai.expect; @@ -10,6 +10,20 @@ const expect = chai.expect; const dots = ["..", "..", ".."]; const basePath = path.join(__filename, ...dots); +function get_options(testFolderPath: string) { + const options: FormatterOptions = { + maxEmptyLines: 2, + denseFunctionParameters: false, + }; + const optionsPath = path.join(testFolderPath, "config.json"); + if (fs.existsSync(optionsPath)) { + const file = fs.readFileSync(optionsPath).toString(); + const config = JSON.parse(file); + return { ...options, ...config } as FormatterOptions; + } + return options; +} + suite("GDScript Formatter Tests", () => { // Search for all folders in the snapshots folder and run a test for each // comparing the output of the formatter with the expected output. @@ -18,15 +32,19 @@ suite("GDScript Formatter Tests", () => { const snapshotsFolderPath = path.join(basePath, "src/formatter/snapshots"); const testFolders = fs.readdirSync(snapshotsFolderPath); + // biome-ignore lint/complexity/noForEach: testFolders.forEach((testFolder) => { const testFolderPath = path.join(snapshotsFolderPath, testFolder); if (fs.statSync(testFolderPath).isDirectory()) { test(`Snapshot Test: ${testFolder}`, async () => { const uriIn = vscode.Uri.file(path.join(testFolderPath, "in.gd")); const uriOut = vscode.Uri.file(path.join(testFolderPath, "out.gd")); + const documentIn = await vscode.workspace.openTextDocument(uriIn); const documentOut = await vscode.workspace.openTextDocument(uriOut); - const edits = format_document(documentIn); + + const options = get_options(testFolderPath); + const edits = format_document(documentIn, options); // Apply the formatting edits const workspaceEdit = new vscode.WorkspaceEdit(); @@ -34,7 +52,9 @@ suite("GDScript Formatter Tests", () => { await vscode.workspace.applyEdit(workspaceEdit); // Compare the result with the expected output - expect(documentIn.getText().replace("\r\n", "\n")).to.equal(documentOut.getText().replace("\r\n", "\n")); + expect(documentIn.getText().replace("\r\n", "\n")).to.equal( + documentOut.getText().replace("\r\n", "\n"), + ); }); } }); diff --git a/src/formatter/snapshots/max-empty-lines-1/config.json b/src/formatter/snapshots/max-empty-lines-1/config.json new file mode 100644 index 0000000..e8b343a --- /dev/null +++ b/src/formatter/snapshots/max-empty-lines-1/config.json @@ -0,0 +1,3 @@ +{ + "maxEmptyLines": 1 +} \ No newline at end of file diff --git a/src/formatter/snapshots/consecutive-empty-lines-are-removed/in.gd b/src/formatter/snapshots/max-empty-lines-1/in.gd similarity index 100% rename from src/formatter/snapshots/consecutive-empty-lines-are-removed/in.gd rename to src/formatter/snapshots/max-empty-lines-1/in.gd diff --git a/src/formatter/snapshots/consecutive-empty-lines-are-removed/out.gd b/src/formatter/snapshots/max-empty-lines-1/out.gd similarity index 97% rename from src/formatter/snapshots/consecutive-empty-lines-are-removed/out.gd rename to src/formatter/snapshots/max-empty-lines-1/out.gd index edd5e19..cb6fb2b 100644 --- a/src/formatter/snapshots/consecutive-empty-lines-are-removed/out.gd +++ b/src/formatter/snapshots/max-empty-lines-1/out.gd @@ -1,16 +1,13 @@ class Test: - func _ready(): pass - func test(): pass - # comments func with_comments(): diff --git a/src/formatter/snapshots/max-empty-lines-2/in.gd b/src/formatter/snapshots/max-empty-lines-2/in.gd new file mode 100644 index 0000000..5937cc2 --- /dev/null +++ b/src/formatter/snapshots/max-empty-lines-2/in.gd @@ -0,0 +1,28 @@ + + + +class Test: + + + + + func _ready(): + + + pass + + + +func test(): + + + pass + + + + +# comments +func with_comments(): + + + pass diff --git a/src/formatter/snapshots/max-empty-lines-2/out.gd b/src/formatter/snapshots/max-empty-lines-2/out.gd new file mode 100644 index 0000000..9c8e646 --- /dev/null +++ b/src/formatter/snapshots/max-empty-lines-2/out.gd @@ -0,0 +1,20 @@ +class Test: + + + func _ready(): + + + pass + + +func test(): + + + pass + + +# comments +func with_comments(): + + + pass diff --git a/src/formatter/symbols.ts b/src/formatter/symbols.ts index a83f4a3..0f3dbe2 100644 --- a/src/formatter/symbols.ts +++ b/src/formatter/symbols.ts @@ -20,7 +20,6 @@ export const keywords = [ "is", "master", "mastersync", - "match", "when", "not", "onready", diff --git a/src/formatter/textmate.ts b/src/formatter/textmate.ts index 24b1862..763ad21 100644 --- a/src/formatter/textmate.ts +++ b/src/formatter/textmate.ts @@ -1,4 +1,5 @@ -import { Range, type TextDocument, TextEdit, TextLine } from "vscode"; +import { TextEdit } from "vscode"; +import type { TextDocument, TextLine } from "vscode"; import * as fs from "node:fs"; import * as vsctm from "vscode-textmate"; import * as oniguruma from "vscode-oniguruma"; @@ -51,14 +52,18 @@ interface Token { skip?: boolean; } -class FormatterOptions { - emptyLinesBeforeFunctions: "one" | "two"; - denseFunctionDeclarations: boolean; +export interface FormatterOptions { + maxEmptyLines: 1 | 2; + denseFunctionParameters: boolean; +} - constructor() { - this.emptyLinesBeforeFunctions = get_configuration("formatter.emptyLinesBeforeFunctions"); - this.denseFunctionDeclarations = get_configuration("formatter.denseFunctionDeclarations"); - } +function get_formatter_options() { + const options: FormatterOptions = { + maxEmptyLines: get_configuration("formatter.maxEmptyLines") === "1" ? 1 : 2, + denseFunctionParameters: get_configuration("formatter.denseFunctionParameters"), + }; + + return options; } function parse_token(token: Token) { @@ -70,6 +75,12 @@ function parse_token(token: Token) { } if (token.scopes.includes("meta.literal.nodepath.gdscript")) { token.skip = true; + token.type = "nodepath"; + return; + } + if (token.scopes.includes("keyword.control.flow.gdscript")) { + token.type = "keyword"; + return; } if (keywords.includes(token.value)) { token.type = "keyword"; @@ -114,7 +125,7 @@ function between(tokens: Token[], current: number, options: FormatterOptions) { if (prev === "(") return ""; if (nextToken.param) { - if (options.denseFunctionDeclarations) { + if (options.denseFunctionParameters) { if (prev === "-") { if (tokens[current - 2]?.value === "=") return ""; if (["keyword", "symbol"].includes(tokens[current - 2].type)) { @@ -171,6 +182,7 @@ function between(tokens: Token[], current: number, options: FormatterOptions) { if (prev === ")" && nextToken.type === "keyword") return " "; if (prev === "[" && nextToken.type === "symbol") return ""; + if (prev === "[" && nextToken.type === "nodepath") return ""; if (prev === ":") return " "; if (prev === ";") return " "; if (prev === "##") return " "; @@ -205,34 +217,28 @@ function is_comment(line: TextLine): boolean { return line.text[line.firstNonWhitespaceCharacterIndex] === "#"; } -export function format_document(document: TextDocument): TextEdit[] { +export function format_document(document: TextDocument, _options?: FormatterOptions): TextEdit[] { // quit early if grammar is not loaded if (!grammar) { return []; } const edits: TextEdit[] = []; - const options = new FormatterOptions(); - + const options = _options ?? get_formatter_options(); + let lineTokens: vsctm.ITokenizeLineResult = null; let onlyEmptyLinesSoFar = true; let emptyLineCount = 0; - let firstEmptyLine = 0; for (let lineNum = 0; lineNum < document.lineCount; lineNum++) { const line = document.lineAt(lineNum); // skip empty lines - if (line.isEmptyOrWhitespace || is_comment(line)) { + if (line.isEmptyOrWhitespace) { // delete empty lines at the beginning of the file if (onlyEmptyLinesSoFar) { edits.push(TextEdit.delete(line.rangeIncludingLineBreak)); } else { - if (emptyLineCount === 0) { - firstEmptyLine = lineNum; - } - if (!is_comment(line)) { - emptyLineCount++; - } + emptyLineCount++; } // delete empty lines at the end of the file @@ -247,26 +253,8 @@ export function format_document(document: TextDocument): TextEdit[] { // delete consecutive empty lines if (emptyLineCount) { - let maxEmptyLines = 1; - - const start = line.text.trimStart(); - if (options.emptyLinesBeforeFunctions === "two") { - if (start.startsWith("func") || start.startsWith("static func")) { - maxEmptyLines++; - } - } - if (start.startsWith("class")) { - maxEmptyLines++; - } - let i = 0; - let deletedLines = 0; - const linesToDelete = emptyLineCount - maxEmptyLines; - while (i < lineNum && deletedLines < linesToDelete) { - const candidate = document.lineAt(firstEmptyLine + i++); - if (candidate.isEmptyOrWhitespace) { - edits.push(TextEdit.delete(candidate.rangeIncludingLineBreak)); - deletedLines++; - } + for (let i = emptyLineCount - options.maxEmptyLines; i > 0; i--) { + edits.push(TextEdit.delete(document.lineAt(lineNum - i).rangeIncludingLineBreak)); } emptyLineCount = 0; } diff --git a/syntaxes/GDScript.tmLanguage.json b/syntaxes/GDScript.tmLanguage.json index cfe2bd4..ad37633 100644 --- a/syntaxes/GDScript.tmLanguage.json +++ b/syntaxes/GDScript.tmLanguage.json @@ -69,6 +69,9 @@ { "include": "#assignment_operator" }, { "include": "#in_keyword" }, { "include": "#control_flow" }, + { "include": "#match_keyword" }, + { "include": "#curly_braces" }, + { "include": "#square_braces" }, { "include": "#round_braces" }, { "include": "#function_call" }, { "include": "#comment" }, @@ -146,6 +149,8 @@ ] }, "nodepath_function": { + "name": "meta.function.gdscript", + "contentName": "meta.function.parameters.gdscript", "begin": "(get_node_or_null|has_node|has_node_and_resource|find_node|get_node)\\s*(\\()", "beginCaptures": { "1": { "name": "entity.name.function.gdscript" }, @@ -164,7 +169,8 @@ "name": "keyword.control.flow" } ] - } + }, + { "include": "#base_expression" } ] }, "self": { @@ -231,9 +237,13 @@ "name": "keyword.operator.assignment.gdscript" }, "control_flow": { - "match": "\\b(?:if|elif|else|while|break|continue|pass|return|match|when|yield|await)\\b", + "match": "\\b(?:if|elif|else|while|break|continue|pass|return|when|yield|await)\\b", "name": "keyword.control.gdscript" }, + "match_keyword": { + "match": "^\n\\s*(match)", + "captures": { "1": { "name": "keyword.control.gdscript" } } + }, "keywords": { "match": "\\b(?:class|class_name|is|onready|tool|static|export|as|void|enum|preload|assert|breakpoint|sync|remote|master|puppet|slave|remotesync|mastersync|puppetsync|trait|namespace)\\b", "name": "keyword.language.gdscript" @@ -544,6 +554,26 @@ } ] }, + "curly_braces": { + "begin": "\\{", + "end": "\\}", + "beginCaptures": { "0": { "name": "punctuation.definition.dict.begin.gdscript" } }, + "endCaptures": { "0": { "name": "punctuation.definition.dict.end.gdscript" } }, + "patterns": [ + { "include": "#base_expression" }, + { "include": "#any_variable" } + ] + }, + "square_braces": { + "begin": "\\[", + "end": "\\]", + "beginCaptures": { "0": { "name": "punctuation.definition.list.begin.gdscript" } }, + "endCaptures": { "0": { "name": "punctuation.definition.list.end.gdscript" } }, + "patterns": [ + { "include": "#base_expression" }, + { "include": "#any_variable" } + ] + }, "round_braces": { "begin": "\\(", "end": "\\)", diff --git a/syntaxes/examples/gdscript2.gd b/syntaxes/examples/gdscript2.gd index 5f5e29d..fe73a15 100644 --- a/syntaxes/examples/gdscript2.gd +++ b/syntaxes/examples/gdscript2.gd @@ -45,6 +45,12 @@ func f(): super() super.some_function() + match param3: + 3: + print("param3 is 3!") + _: + print("param3 is not 3!") + for i in range(1): # `in` is a control keyword print(i in range(1)) # `in` is an operator keyword