Various Formatter Improvements (#746)

* add new style of formatter snapshot tests
* add many new test cases
* fix several open issues( #728, #624, #657, #717, #734, likely more)
This commit is contained in:
David Kincaid
2024-11-18 11:16:16 -05:00
committed by GitHub
parent 709fa1bbad
commit f648c37353
23 changed files with 516 additions and 168 deletions

48
.vscode/test_files.code-snippets vendored Normal file
View File

@@ -0,0 +1,48 @@
{
"# --- IN ---": {
"scope": "gdscript",
"prefix": "#IN",
"body": [
"# --- IN ---"
],
"description": "Snapshot Test #IN block"
},
"# --- OUT ---": {
"scope": "gdscript",
"prefix": "#OUT",
"body": [
"# --- OUT ---"
],
"description": "Snapshot Test #OUT block"
},
"# --- END ---": {
"scope": "gdscript",
"prefix": "#END",
"body": [
"# --- END ---"
],
"description": "Snapshot Test #END block"
},
"# --- CONFIG ---": {
"scope": "gdscript",
"prefix": [
"#CO",
"#CONFIG"
],
"body": [
"# --- CONFIG ---"
],
"description": "Snapshot Test #CONFIG block"
},
"# --- CONFIG ALL ---": {
"scope": "gdscript",
"prefix": [
"#CA",
"#CONFIG ALL"
],
"body": [
"# --- CONFIG ALL ---"
],
"description": "Snapshot Test #CONFIG ALL block"
},
}

View File

@@ -1,61 +1,195 @@
import * as vscode from "vscode";
import * as path from "node:path";
import * as fs from "node:fs";
import * as path from "node:path";
import * as vscode from "vscode";
import { format_document, type FormatterOptions } from "./textmate";
import * as chai from "chai";
const expect = chai.expect;
import { expect } from "chai";
const dots = ["..", "..", ".."];
const basePath = path.join(__filename, ...dots);
const snapshotsFolderPath = path.join(basePath, "src/formatter/snapshots");
function get_options(testFolderPath: string) {
const options: FormatterOptions = {
maxEmptyLines: 2,
denseFunctionParameters: false,
};
const optionsPath = path.join(testFolderPath, "config.json");
function normalizeLineEndings(str: string) {
return str.replace(/\r?\n/g, "\n");
}
const defaultOptions: FormatterOptions = {
maxEmptyLines: 2,
denseFunctionParameters: false,
};
function get_options(folder: fs.Dirent) {
const optionsPath = path.join(folder.path, folder.name, "config.json");
if (fs.existsSync(optionsPath)) {
const file = fs.readFileSync(optionsPath).toString();
const config = JSON.parse(file);
return { ...options, ...config } as FormatterOptions;
return { ...defaultOptions, ...config } as FormatterOptions;
}
return options;
return defaultOptions;
}
function set_content(content: string) {
return vscode.workspace
.openTextDocument()
.then((doc) => vscode.window.showTextDocument(doc))
.then((editor) => {
const editBuilder = (textEdit) => {
textEdit.insert(new vscode.Position(0, 0), String(content));
};
return editor
.edit(editBuilder, {
undoStopBefore: true,
undoStopAfter: false,
})
.then(() => editor);
});
}
function build_config(lines: string[]) {
try {
return JSON.parse(lines.join("\n"));
} catch (e) {
return {};
}
}
class TestLines {
config: string[] = [];
in: string[] = [];
out: string[] = [];
parse(_config) {
const config = { ...defaultOptions, ..._config, ...build_config(this.config) };
const test: Test = {
in: this.in.join("\n"),
out: this.out.join("\n"),
config: config,
};
if (test.out === "") {
test.out = this.in.join("\n");
}
if (!config.strictTrailingNewlines) {
test.in = test.in.trimEnd();
test.out = test.out.trimEnd();
}
return test;
}
}
interface Test {
config?: FormatterOptions;
in: string;
out: string;
}
const CONFIG_ALL = "# --- CONFIG ALL ---";
const CONFIG = "# --- CONFIG ---";
const IN = "# --- IN ---";
const OUT = "# --- OUT ---";
const END = "# --- END ---";
const MODES = [CONFIG_ALL, CONFIG, IN, OUT, END];
function parse_test_file(content: string): Test[] {
let defaultConfig = null;
let defaultConfigString: string[] = [];
const tests: Test[] = [];
let mode = null;
let test = new TestLines();
for (const _line of content.split("\n")) {
const line = _line.trim();
if (MODES.includes(line)) {
if (line === CONFIG || line === IN) {
if (test.in.length !== 0) {
tests.push(test.parse(defaultConfig));
test = new TestLines();
}
}
if (defaultConfigString.length !== 0) {
defaultConfig = build_config(defaultConfigString);
defaultConfigString = [];
}
mode = line;
continue;
}
if (mode === CONFIG_ALL) defaultConfigString.push(line);
if (mode === CONFIG) test.config.push(line);
if (mode === IN) test.in.push(line);
if (mode === OUT) test.out.push(line);
}
if (test.in.length !== 0) {
tests.push(test.parse(defaultConfig));
}
return tests;
}
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.
// To add a new test, create a new folder in the snapshots folder
// and add two files, `in.gd` and `out.gd` for the input and expected output.
const snapshotsFolderPath = path.join(basePath, "src/formatter/snapshots");
const testFolders = fs.readdirSync(snapshotsFolderPath);
const testFiles = fs.readdirSync(snapshotsFolderPath, { withFileTypes: true, recursive: true });
// biome-ignore lint/complexity/noForEach: <explanation>
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"));
for (const file of testFiles.filter((f) => f.isFile())) {
if (["in.gd", "out.gd"].includes(file.name) || !file.name.endsWith(".gd")) {
continue;
}
test(`Snapshot Test: ${file.name}`, async () => {
const uri = vscode.Uri.file(path.join(snapshotsFolderPath, file.name));
const inDoc = await vscode.workspace.openTextDocument(uri);
const text = inDoc.getText();
const documentIn = await vscode.workspace.openTextDocument(uriIn);
const documentOut = await vscode.workspace.openTextDocument(uriOut);
for (const test of parse_test_file(text)) {
const editor = await set_content(test.in);
const document = editor.document;
const options = get_options(testFolderPath);
const edits = format_document(documentIn, options);
const edits = format_document(document, test.config);
// Apply the formatting edits
const workspaceEdit = new vscode.WorkspaceEdit();
workspaceEdit.set(uriIn, edits);
workspaceEdit.set(document.uri, edits);
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"),
);
});
const actual = normalizeLineEndings(document.getText());
const expected = normalizeLineEndings(test.out);
expect(actual).to.equal(expected);
}
});
}
for (const folder of testFiles.filter((f) => f.isDirectory())) {
const pathIn = path.join(folder.path, folder.name, "in.gd");
const pathOut = path.join(folder.path, folder.name, "out.gd");
if (!(fs.existsSync(pathIn) && fs.existsSync(pathOut))) {
continue;
}
});
test(`Snapshot Pair Test: ${folder.name}`, async () => {
const uriIn = vscode.Uri.file(path.join(folder.path, folder.name, "in.gd"));
const uriOut = vscode.Uri.file(path.join(folder.path, folder.name, "out.gd"));
const documentIn = await vscode.workspace.openTextDocument(uriIn);
const documentOut = await vscode.workspace.openTextDocument(uriOut);
const options = get_options(folder);
const edits = format_document(documentIn, options);
// Apply the formatting edits
const workspaceEdit = new vscode.WorkspaceEdit();
workspaceEdit.set(uriIn, edits);
await vscode.workspace.applyEdit(workspaceEdit);
// Compare the result with the expected output
const actual = normalizeLineEndings(documentIn.getText());
const expected = normalizeLineEndings(documentOut.getText());
expect(actual).to.equal(expected);
});
}
});

View File

@@ -0,0 +1,101 @@
## An `IN` block is fed into the formatter and the output is compared to the `OUT` block
```
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
```
## Trailing newlines in `IN` and `OUT` blocks is automatically removed
```
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
# --- IN ---
var b = 'ten'
# --- OUT ---
var b = 'ten'
```
## An `IN` block by itself will be reused at the `OUT` target
Many test cases can simply be expressed as "do not change this":
```
# --- IN ---
var a = """ {
level_file: '%s',
md5_hash: %s,
}
"""
```
## Formatter and test harness options can be controlled with `CONFIG` blocks
This test will fail because `strictTrailingNewlines: true` disables trailing newline removal.
```
# --- CONFIG ---
{"strictTrailingNewlines": true}
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
```
## `CONFIG ALL` set the default options moving forward, and `END` blocks allow additional layout flexibility
```
# --- CONFIG ALL ---
{"strictTrailingNewlines": true}
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
# --- END ---
# anything I want goes here
# --- IN ---
var b = 'ten'
# --- OUT ---
var b = 'ten'
```
## `CONFIG` blocks override `CONFIG ALL`, and the configs are merged for a given test
This test will pass, because the second test has a `CONFIG` that overrides the `CONFIG ALL` at the top.
```
# --- CONFIG ALL ---
{"strictTrailingNewlines": true}
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
# --- END ---
# anything I want goes here
# --- CONFIG ---
{"strictTrailingNewlines": false}
# --- IN ---
var b = 'ten'
# --- OUT ---
var b = 'ten'
# --- IN ---
var c = true
# --- OUT ---
var c = true
```

View File

@@ -1,15 +0,0 @@
func f():
# arithmetic
x += 1
x -= 1
x *= 1
x /= 1
x %= 1
# bitwise
x |= 1
x &= 1
x ~= 1
x /= 1
x >>= 1
x <<= 1

View File

@@ -1,5 +0,0 @@
func f():
collision_mask = 1 << 1 | 1 << 3
collision_mask = 1 << 1 & 1 << 3
collision_mask = ~1
collision_mask = 1 ^ ~ 1

View File

@@ -1,5 +0,0 @@
func f():
collision_mask = 1 << 1 | 1 << 3
collision_mask = 1 << 1 & 1 << 3
collision_mask = ~1
collision_mask = 1 ^ ~1

View File

@@ -0,0 +1,5 @@
# --- IN ---
func handleDeath() -> void:
var signalConnections: Array[Dictionary] = self.get_incoming_connections()
for connection in signalConnections:
connection.signal.disconnect(connection.callable)

View File

@@ -0,0 +1,4 @@
# --- IN ---
poll_animation.on_complete(func() -> void:
highlight.visible = false
)

View File

@@ -0,0 +1,16 @@
# --- IN ---
func dump() -> String:
return """
{
level_file: '%s',
md5_hash: %s,
text: '%s',
level_size: %s,
world_pos: %s,
preview_size: %s,
preview_pos: %s,
preview_texture: %s,
explorer_layer: %s,
connections: %s,
}
"""

View File

@@ -1,3 +0,0 @@
{
"maxEmptyLines": 1
}

View File

@@ -1,27 +0,0 @@
class Test:
func _ready():
pass
func test():
pass
# comments
func with_comments():
pass

View File

@@ -1,14 +0,0 @@
class Test:
func _ready():
pass
func test():
pass
# comments
func with_comments():
pass

View File

@@ -1,28 +0,0 @@
class Test:
func _ready():
pass
func test():
pass
# comments
func with_comments():
pass

View File

@@ -1,20 +0,0 @@
class Test:
func _ready():
pass
func test():
pass
# comments
func with_comments():
pass

View File

@@ -0,0 +1,72 @@
# --- IN ---
func test():
pass
# --- OUT ---
func test():
pass
# --- IN ---
class Test:
func _ready():
pass
# --- OUT ---
class Test:
func _ready():
pass
# --- IN ---
func test(): # with comment
pass
# --- OUT ---
func test(): # with comment
pass
# --- IN ---
class Test: # with comment
func _ready(): # with comment
pass
# --- OUT ---
class Test: # with comment
func _ready(): # with comment
pass
# --- CONFIG ---
{"maxEmptyLines": 1}
# --- IN ---
func a():
pass
func b():
pass
# --- OUT ---
func a():
pass
func b():
pass
# --- CONFIG ---
{"maxEmptyLines": 2}
# --- IN ---
func a():
pass
func b():
pass
# --- OUT ---
func a():
pass
func b():
pass

View File

@@ -0,0 +1,23 @@
# --- IN ---
var test1 := deg_to_rad(
-90
)
# --- IN ---
var test2 := Vector2(
-0.0,
1.0
)
# --- IN ---
var test3 := Vector3(
0.0,
-0.0,
0.0
)
# --- IN ---
func get_audio_compensation() -> float:
return AudioServer.get_time_since_last_mix() \
- AudioServer.get_output_latency() \
+ (1 / Engine.get_frames_per_second()) * 2

View File

@@ -44,6 +44,9 @@ var a = $Child/%Unique/ChildOfUnique
var a = %Unique
var a = %Unique/Child
var a = %Unique/%UniqueChild
var a = %"Unique"
var a = %'Unique/Child'
var a = %'Unique/%UniqueChild'
var a = $"%Unique"
var a = get_node("%Unique")

View File

@@ -44,6 +44,9 @@ var a = $Child/%Unique/ChildOfUnique
var a = %Unique
var a = %Unique/Child
var a = %Unique/%UniqueChild
var a = %"Unique"
var a = %'Unique/Child'
var a = %'Unique/%UniqueChild'
var a = $"%Unique"
var a = get_node("%Unique")

View File

@@ -1,3 +1,4 @@
# --- IN ---
func f():
# arithmetic
x += 1
@@ -5,11 +6,19 @@ func f():
x *= 1
x /= 1
x %= 1
x = 2 ** 2
x = 2 * -1
# bitwise
x |= 1
x &= 1
x ^= 1
x ~= 1
x = ~1
x /= 1
x >>= 1
x <<= 1
x = 1 << 1 | 1 >> 3
x = 1 << 1 & 1 >> 3
x = 1 ^ ~1

View File

@@ -0,0 +1,6 @@
# --- IN ---
var a = 1e-6
var b = 4e-09
var c = 58.1e-10
var d = 58.1e+10
var e = 9.732e-06

View File

@@ -0,0 +1,14 @@
# --- CONFIG ---
{"strictTrailingNewlines": true}
# --- IN ---
var a = 10
# --- OUT ---
var a = 10
# --- END ---
# anything I want goes here
# --- IN ---
var b = 'ten'
# --- OUT ---
var b = 'ten'

View File

@@ -4,7 +4,7 @@ import * as fs from "node:fs";
import * as vsctm from "vscode-textmate";
import * as oniguruma from "vscode-oniguruma";
import { keywords, symbols } from "./symbols";
import { get_configuration, get_extension_uri, createLogger } from "../utils";
import { get_configuration, get_extension_uri, createLogger, is_debug_mode } from "../utils";
const log = createLogger("formatter.tm");
@@ -50,10 +50,11 @@ interface Token {
param?: boolean;
string?: boolean;
skip?: boolean;
identifier?: boolean;
}
export interface FormatterOptions {
maxEmptyLines: 1 | 2;
maxEmptyLines: 0 | 1 | 2;
denseFunctionParameters: boolean;
}
@@ -73,6 +74,9 @@ function parse_token(token: Token) {
if (token.scopes.includes("meta.function.parameters.gdscript")) {
token.param = true;
}
if (token.value.match(/[A-Za-z_]\w+/)) {
token.identifier = true;
}
if (token.scopes.includes("meta.literal.nodepath.gdscript")) {
token.skip = true;
token.type = "nodepath";
@@ -111,6 +115,10 @@ function parse_token(token: Token) {
token.type = "variable";
return;
}
if (token.scopes.includes("comment.line.number-sign.gdscript")) {
token.type = "comment";
return;
}
}
function between(tokens: Token[], current: number, options: FormatterOptions) {
@@ -128,12 +136,17 @@ function between(tokens: Token[], current: number, options: FormatterOptions) {
if (prevToken.skip && nextToken.skip) return "";
if (prev === "(") return "";
if (prev === ".") {
if (nextToken?.type === "symbol") return " ";
return "";
}
if (next === ".") return "";
if (nextToken.param) {
if (options.denseFunctionParameters) {
if (prev === "-") {
if (tokens[current - 2]?.value === "=") return "";
if (["keyword", "symbol"].includes(tokens[current - 2].type)) {
if (["keyword", "symbol"].includes(tokens[current - 2]?.type)) {
return "";
}
if ([",", "("].includes(tokens[current - 2]?.value)) {
@@ -169,7 +182,9 @@ function between(tokens: Token[], current: number, options: FormatterOptions) {
if (prev === "@") return "";
if (prev === "-") {
if (["keyword", "symbol"].includes(tokens[current - 2].type)) {
if (nextToken.identifier) return " ";
if (current === 1) return "";
if (["keyword", "symbol"].includes(tokens[current - 2]?.type)) {
return "";
}
if ([",", "(", "["].includes(tokens[current - 2]?.value)) {
@@ -232,6 +247,7 @@ export function format_document(document: TextDocument, _options?: FormatterOpti
const options = _options ?? get_formatter_options();
let lastToken = null;
let lineTokens: vsctm.ITokenizeLineResult = null;
let onlyEmptyLinesSoFar = true;
let emptyLineCount = 0;
@@ -259,7 +275,11 @@ export function format_document(document: TextDocument, _options?: FormatterOpti
// delete consecutive empty lines
if (emptyLineCount) {
for (let i = emptyLineCount - options.maxEmptyLines; i > 0; i--) {
let maxEmptyLines = options.maxEmptyLines;
if (lastToken === ":") {
maxEmptyLines = 0;
}
for (let i = emptyLineCount - maxEmptyLines; i > 0; i--) {
edits.push(TextEdit.delete(document.lineAt(lineNum - i).rangeIncludingLineBreak));
}
emptyLineCount = 0;
@@ -296,12 +316,19 @@ export function format_document(document: TextDocument, _options?: FormatterOpti
tokens.push(token);
}
for (let i = 0; i < tokens.length; i++) {
log.debug(i, tokens[i].value, tokens[i]);
if (i > 0 && tokens[i - 1].string === true && tokens[i].string === true) {
if (is_debug_mode()) log.debug(i, tokens[i].value, tokens[i]);
if (i === 0 && tokens[i].string) {
// leading whitespace is already accounted for
nextLine += tokens[i].original.trimStart();
} else if (i > 0 && tokens[i - 1].string && tokens[i].string) {
nextLine += tokens[i].original;
} else {
nextLine += between(tokens, i, options) + tokens[i].value.trim();
}
if (tokens[i].type !== "comment") {
lastToken = tokens[i].value;
}
}
edits.push(TextEdit.replace(line.range, nextLine));

View File

@@ -274,6 +274,11 @@
"match": "[-]?([0-9][0-9_]*e[\\-\\+]?\\[0-9_])",
"name": "constant.numeric.float.gdscript"
},
{
"name": "constant.numeric.float.gdscript",
"match": "(?x)\n (?<! \\w)(?:\n (?:\n \\.[0-9](?: _?[0-9] )*\n |\n [0-9](?: _?[0-9] )* \\. [0-9](?: _?[0-9] )*\n |\n [0-9](?: _?[0-9] )* \\.\n ) (?: [eE][+-]?[0-9](?: _?[0-9] )* )?\n |\n [0-9](?: _?[0-9] )* (?: [eE][+-]?[0-9](?: _?[0-9] )* )\n )([jJ])?\\b\n",
"captures": { "1": { "name": "storage.type.imaginary.number.gdscript" } }
},
{
"match": "[-]?[0-9][0-9_]*",
"name": "constant.numeric.integer.gdscript"
@@ -386,7 +391,7 @@
},
"builtin_get_node_shorthand_quoted": {
"name": "string.quoted.gdscript meta.literal.nodepath.gdscript constant.character.escape.gdscript",
"begin": "(?:(\\$)|(&|\\^|@))(\"|')",
"begin": "(?:(\\$|%)|(&|\\^|@))(\"|')",
"beginCaptures": {
"1": { "name": "keyword.control.flow.gdscript" },
"2": { "name": "variable.other.enummember.gdscript" }
@@ -498,13 +503,8 @@
"1": { "name": "keyword.language.gdscript storage.type.function.gdscript" },
"2": { "name": "entity.name.function.gdscript" }
},
"end": "(:|(?=[#'\"\\n]))",
"end2": "(\\s*(\\-\\>)\\s*(void\\w*)|([a-zA-Z_]\\w*)\\s*\\:)",
"endCaptures2": {
"1": { "name": "punctuation.separator.annotation.result.gdscript" },
"2": { "name": "keyword.language.void.gdscript" },
"3": { "name": "entity.name.type.class.gdscript markup.italic" }
},
"end": "(:)",
"endCaptures": { "1": { "name": "punctuation.section.function.begin.gdscript" } },
"patterns": [
{ "include": "#parameters" },
{ "include": "#line_continuation" },