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)
godotengine · Nov 18, 2024 · f648c37 · f648c37
1 parent 709fa1b
commit f648c37
Show file tree

Hide file tree

Showing 23 changed files with 519 additions and 171 deletions.
diff --git a/.vscode/test_files.code-snippets b/.vscode/test_files.code-snippets
@@ -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"
+    },
+}
diff --git a/src/formatter/formatter.test.ts b/src/formatter/formatter.test.ts
@@ -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 normalizeLineEndings(str: string) {
+	return str.replace(/\r?\n/g, "\n");
+}
+
+const defaultOptions: FormatterOptions = {
+	maxEmptyLines: 2,
+	denseFunctionParameters: false,
+};
 
-function get_options(testFolderPath: string) {
-	const options: FormatterOptions = {
-		maxEmptyLines: 2,
-		denseFunctionParameters: false,
-	};
-	const optionsPath = path.join(testFolderPath, "config.json");
+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 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 {};
 	}
-	return options;
+}
+
+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);
-
-	// 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"));
-
-				const documentIn = await vscode.workspace.openTextDocument(uriIn);
-				const documentOut = await vscode.workspace.openTextDocument(uriOut);
-
-				const options = get_options(testFolderPath);
-				const edits = format_document(documentIn, options);
+	const testFiles = fs.readdirSync(snapshotsFolderPath, { withFileTypes: true, recursive: true });
+
+	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();
+
+			for (const test of parse_test_file(text)) {
+				const editor = await set_content(test.in);
+				const document = editor.document;
+
+				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);
+		});
+	}
 });
diff --git a/src/formatter/snapshots/README.md b/src/formatter/snapshots/README.md
@@ -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
+```