From 60113e5651789577f496844d1fa5967be1966d80 Mon Sep 17 00:00:00 2001 From: Steven Chetwynd Date: Fri, 4 Jul 2025 21:15:00 +0100 Subject: [PATCH 1/6] =?UTF-8?q?test:=20=F0=9F=92=8D=20fix=20test=20runner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add test files to build when not in production Correct import of Mocha and Glob in test suite --- esbuild.js | 7 +++++-- src/test/suite/index.ts | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/esbuild.js b/esbuild.js index 95f04a5..40e19bb 100644 --- a/esbuild.js +++ b/esbuild.js @@ -2,6 +2,7 @@ // This may require changes when upgrading to esbuild 0.17+ const { build } = require("esbuild"); +const glob = require("glob"); const baseConfig = { bundle: true, @@ -9,13 +10,15 @@ const baseConfig = { sourcemap: process.env.NODE_ENV !== "production", }; +const testFiles = process.env.NODE_ENV !== "production" ? glob.sync("./src/test/**/*.ts") : []; + const extensionConfig = { ...baseConfig, platform: "node", mainFields: ["module", "main"], format: "cjs", - entryPoints: ["./src/extension.ts"], - outfile: "./out/extension.js", + entryPoints: ["./src/extension.ts", ...testFiles], + outdir: "./out", external: ["vscode"], }; diff --git a/src/test/suite/index.ts b/src/test/suite/index.ts index 80ad21c..c80ca20 100644 --- a/src/test/suite/index.ts +++ b/src/test/suite/index.ts @@ -1,5 +1,5 @@ -import * as glob from "glob"; -import * as Mocha from "mocha"; +import glob from "glob"; +import Mocha from "mocha"; import path from "path"; export function run(): Promise { From 07eaec15e46a046e5fa80b83cb748e1d5ae613f1 Mon Sep 17 00:00:00 2001 From: Steven Chetwynd Date: Sat, 5 Jul 2025 11:11:15 +0100 Subject: [PATCH 2/6] =?UTF-8?q?fix:=20=F0=9F=90=9B=20recognises=20options?= =?UTF-8?q?=20which=20have=20been=20overwritten?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add new util function to compare JSON to AST. Use isJSONDifferentToAST function to compare changes after edit. ✅ Closes: 7811 --- src/astUtils.ts | 62 ++++++++++++++++++++ src/diagnostics/importOverrideDiagnostics.ts | 19 +++--- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/astUtils.ts b/src/astUtils.ts index 405a2b4..af345e5 100644 --- a/src/astUtils.ts +++ b/src/astUtils.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; import { + ArrayASTNode, ASTNode, Location, ObjectASTNode, @@ -124,3 +125,64 @@ export function getOptionsFromParamDefinition( }); return options; } + +export function isJSONDifferentToAST(json: unknown, ast: ASTNode): boolean { + const jsonType = + typeof json === "object" + ? Array.isArray(json) + ? "array" + : "object" + : typeof json; + + if (jsonType !== ast.type) { + return true; + } + + if (isArrayASTNode(ast) && Array.isArray(json)) { + const arrayASTNode = ast; + if (arrayASTNode.items.length !== json.length) { + return true; + } + + for (let i = 0; i < arrayASTNode.items.length; i++) { + if (isJSONDifferentToAST(json[i], arrayASTNode.items[i])) { + return true; + } + } + + return false; + } + + if (isObjectASTNode(ast) && typeof json === "object" && json !== null) { + if (ast.properties.length !== Object.keys(json).length) { + return true; + } + + for (const property of ast.properties) { + if (!property.valueNode) { + return true; + } + + if ( + isJSONDifferentToAST( + (json as Record)[property.keyNode.value], + property.valueNode, + ) + ) { + return true; + } + } + + return false; + } + + return ast.value !== json; +} + +export function isArrayASTNode(node: ASTNode): node is ArrayASTNode { + return node.type === "array"; +} + +export function isObjectASTNode(node: ASTNode): node is ObjectASTNode { + return node.type === "object"; +} diff --git a/src/diagnostics/importOverrideDiagnostics.ts b/src/diagnostics/importOverrideDiagnostics.ts index 3139a23..6717793 100644 --- a/src/diagnostics/importOverrideDiagnostics.ts +++ b/src/diagnostics/importOverrideDiagnostics.ts @@ -2,6 +2,7 @@ import { ASTNode, PropertyASTNode } from "vscode-json-languageservice"; import { getPropertyNameFromNode, getPropertyValueFromNode, + isJSONDifferentToAST, nodeIsPropertyNameOrValue, rangeFromNode, } from "../astUtils"; @@ -62,14 +63,13 @@ export function generateImportOverrideDiagnostics( const properties = s .map((s) => config.getNodeFromSymbol(s)) .filter(nodeIsPropertyNameOrValue) - .map( - (n) => - [ - getPropertyNameFromNode(n), - getPropertyValueFromNode(n), - n, - ] as const, - ) + .map((n) => { + return [ + getPropertyNameFromNode(n), + n.parent.valueNode, + n, + ] as const; + }) .filter(([name]) => name in resolvedImport); return [resolvedImport, properties] as const; }, @@ -81,9 +81,8 @@ export function generateImportOverrideDiagnostics( for (const [name, value, propNode] of properties) { const originalValue = imp[name]; - const isUnchanged = value === originalValue; - if (isUnchanged) { + if (value && !isJSONDifferentToAST(originalValue, value)) { ret.push({ type: DiagnosticType.UnnecessaryImportOverride, range: rangeFromNode(config.original, propNode.parent), From b35f22c3b619a24d5e9a146514e7e660843791e1 Mon Sep 17 00:00:00 2001 From: Steven Chetwynd Date: Sat, 5 Jul 2025 11:16:33 +0100 Subject: [PATCH 3/6] =?UTF-8?q?test:=20=F0=9F=92=8D=20new=20function=20and?= =?UTF-8?q?=20importOverrideDiagnostics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/suite/astUtils.test.ts | 384 ++++++++++++++++++ .../importOverrideDiagnostics.test.ts | 143 +++++++ 2 files changed, 527 insertions(+) create mode 100644 src/test/suite/astUtils.test.ts create mode 100644 src/test/suite/diagnostics/importOverrideDiagnostics.test.ts diff --git a/src/test/suite/astUtils.test.ts b/src/test/suite/astUtils.test.ts new file mode 100644 index 0000000..3483f6c --- /dev/null +++ b/src/test/suite/astUtils.test.ts @@ -0,0 +1,384 @@ +import assert from "node:assert"; +import { + ArrayASTNode, + ASTNode, + ObjectASTNode, +} from "vscode-json-languageservice"; +import { isJSONDifferentToAST } from "../../astUtils"; + +suite("astUtils", () => { + suite("isJSONDifferentToAST", () => { + test("returns false if primitive values match", () => { + const ast = { + type: "number", + value: 5, + } as ASTNode; + + const json = 5; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, false); + }); + + test("returns true if AST is a number type and json is string", () => { + const ast = { + type: "number", + value: 5, + } as ASTNode; + + const json = "5"; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if AST is object type and json is array", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = [5]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if AST is array type and json is object", () => { + const ast = { + type: "array", + items: [ + { + type: "number", + value: 5, + }, + ], + } as ArrayASTNode; + + const json = { 0: 5 }; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if value in array is different", () => { + const ast = { + type: "array", + items: [ + { + type: "number", + value: 5, + }, + ], + } as ArrayASTNode; + + const json = [6]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if ast array is longer than json array", () => { + const ast = { + type: "array", + items: [ + { + type: "number", + value: 5, + }, + { + type: "number", + value: 5, + }, + ], + } as ArrayASTNode; + + const json = [5]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if ast array is longer than json array", () => { + const ast = { + type: "array", + items: [ + { + type: "number", + value: 5, + }, + ], + } as ArrayASTNode; + + const json = [5, 5]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if ast object has more properties than json object", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + { + type: "property", + keyNode: { + type: "string", + value: "test2", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = { test: 5 }; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if json object has more properties than ast object", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = { test: 5, test2: 5 }; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if json object has different property to ast object", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = { test2: 5 }; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns true if json object has different value to ast object", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = { test: 6 }; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + + test("returns false if complex json matches ast", () => { + const ast = { + items: [ + { + properties: [ + { + keyNode: { + type: "string", + value: "test", + }, + type: "property", + valueNode: { + type: "number", + value: 1, + }, + }, + { + keyNode: { + type: "string", + value: "test2", + }, + type: "property", + valueNode: { + type: "array", + items: [ + { + type: "object", + properties: [ + { + keyNode: { + type: "string", + value: "innerTest", + }, + type: "property", + valueNode: { + type: "boolean", + value: false, + }, + }, + ], + }, + ], + }, + }, + ], + type: "object", + }, + ], + type: "array", + } as ASTNode; + + const json = [ + { + test: 1, + test2: [ + { + innerTest: false, + }, + ], + }, + ]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, false); + }); + + test("returns true if complex json differs from ast", () => { + const ast = { + items: [ + { + properties: [ + { + keyNode: { + type: "string", + value: "test", + }, + type: "property", + valueNode: { + type: "number", + value: 1, + }, + }, + { + keyNode: { + type: "string", + value: "test2", + }, + type: "property", + valueNode: { + type: "array", + items: [ + { + type: "object", + properties: [ + { + keyNode: { + type: "string", + value: "innerTest", + }, + type: "property", + valueNode: { + type: "boolean", + // difference here + value: true, + }, + }, + ], + }, + ], + }, + }, + ], + type: "object", + }, + ], + type: "array", + } as ASTNode; + + const json = [ + { + test: 1, + test2: [ + { + innerTest: false, + }, + ], + }, + ]; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); + }); +}); diff --git a/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts new file mode 100644 index 0000000..288fb83 --- /dev/null +++ b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts @@ -0,0 +1,143 @@ +import * as assert from "node:assert"; +import { parseConfigDocument } from "../../../configDocument"; +import { generateImportOverrideDiagnostics } from "../../../diagnostics/importOverrideDiagnostics"; + +import * as vscode from "vscode"; +import { getLanguageService as getJsonLanguageService } from "vscode-json-languageservice"; +import { DiagnosticType } from "../../../diagnostics/diagnostics"; +import { My } from "../../../my"; + +suite("importOverrideDiagnostics", () => { + const languageService = getJsonLanguageService({}); + + async function setup(documentContent: object) { + const document = await vscode.workspace.openTextDocument({ + language: "JSON", + content: JSON.stringify(documentContent), + }); + + return parseConfigDocument({ ls: languageService } as My, document); + } + + suite("generateImportOverrideDiagnostics", () => { + test("returns an importOverride if a property has been overridden with a differnt value", async () => { + const documentContent = { + test_template: { + test: 1, + }, + paramInformation: [ + { + "#": 1, + $import: "#test_template", + test: 2, + }, + ], + }; + + const document = await setup(documentContent); + + const result = generateImportOverrideDiagnostics(document); + + assert.strictEqual(result.length, 1); + assert.strictEqual( + result[0].type, + DiagnosticType.ImportOverride, + "Incorrect Diagnostic Type", + ); + }); + + test("returns an importOverride if an options property has been overridden with a different value", async () => { + const documentContent = { + test_template: { + options: [ + { + test: 1, + }, + ], + }, + paramInformation: [ + { + "#": 1, + $import: "#test_template", + options: [ + { + test: 2, + }, + ], + }, + ], + }; + + const document = await setup(documentContent); + + const result = generateImportOverrideDiagnostics(document); + + assert.strictEqual(result.length, 1); + assert.strictEqual( + result[0].type, + DiagnosticType.ImportOverride, + "Incorrect Diagnostic Type", + ); + }); + + test("returns an unnescessaryImportOverride if a property has been overridden with the same value", async () => { + const documentContent = { + test_template: { + test: 1, + }, + paramInformation: [ + { + "#": 1, + $import: "#test_template", + test: 1, + }, + ], + }; + + const document = await setup(documentContent); + + const result = generateImportOverrideDiagnostics(document); + + assert.strictEqual(result.length, 1); + assert.strictEqual( + result[0].type, + DiagnosticType.UnnecessaryImportOverride, + "Incorrect Diagnostic Type", + ); + }); + + test("returns an unnescessaryImportOverride if an options property has been overridden with the same value", async () => { + const documentContent = { + test_template: { + options: [ + { + test: 1, + }, + ], + }, + paramInformation: [ + { + "#": 1, + $import: "#test_template", + options: [ + { + test: 1, + }, + ], + }, + ], + }; + + const document = await setup(documentContent); + + const result = generateImportOverrideDiagnostics(document); + + assert.strictEqual(result.length, 1); + assert.strictEqual( + result[0].type, + DiagnosticType.UnnecessaryImportOverride, + "Incorrect Diagnostic Type", + ); + }); + }); +}); From 9a46d4e6f508d08af68fba124a6e60a05a24ade5 Mon Sep 17 00:00:00 2001 From: Steven Chetwynd Date: Mon, 7 Jul 2025 16:58:36 +0100 Subject: [PATCH 4/6] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20remove=20unneces?= =?UTF-8?q?sary=20variable,=20add=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/astUtils.ts | 8 ++++---- src/test/suite/astUtils.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/astUtils.ts b/src/astUtils.ts index af345e5..3d5b241 100644 --- a/src/astUtils.ts +++ b/src/astUtils.ts @@ -134,18 +134,18 @@ export function isJSONDifferentToAST(json: unknown, ast: ASTNode): boolean { : "object" : typeof json; + // If the property is undefined in the JSON but present in the AST it will return here if (jsonType !== ast.type) { return true; } if (isArrayASTNode(ast) && Array.isArray(json)) { - const arrayASTNode = ast; - if (arrayASTNode.items.length !== json.length) { + if (ast.items.length !== json.length) { return true; } - for (let i = 0; i < arrayASTNode.items.length; i++) { - if (isJSONDifferentToAST(json[i], arrayASTNode.items[i])) { + for (let i = 0; i < ast.items.length; i++) { + if (isJSONDifferentToAST(json[i], ast.items[i])) { return true; } } diff --git a/src/test/suite/astUtils.test.ts b/src/test/suite/astUtils.test.ts index 3483f6c..5018457 100644 --- a/src/test/suite/astUtils.test.ts +++ b/src/test/suite/astUtils.test.ts @@ -380,5 +380,30 @@ suite("astUtils", () => { assert.strictEqual(result, true); }); + + test("it returns true if the property is in AST but not in the JSON", () => { + const ast = { + type: "object", + properties: [ + { + type: "property", + keyNode: { + type: "string", + value: "test", + }, + valueNode: { + type: "number", + value: 5, + }, + }, + ], + } as ObjectASTNode; + + const json = undefined; + + const result = isJSONDifferentToAST(json, ast); + + assert.strictEqual(result, true); + }); }); }); From 12d18ad6b75a4bad542eff18c7d97dfe4b42c7fd Mon Sep 17 00:00:00 2001 From: AlCalzone Date: Tue, 8 Jul 2025 16:07:35 +0200 Subject: [PATCH 5/6] fix typos Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../suite/diagnostics/importOverrideDiagnostics.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts index 288fb83..63f9bfb 100644 --- a/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts +++ b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts @@ -20,7 +20,7 @@ suite("importOverrideDiagnostics", () => { } suite("generateImportOverrideDiagnostics", () => { - test("returns an importOverride if a property has been overridden with a differnt value", async () => { + test("returns an importOverride if a property has been overridden with a different value", async () => { const documentContent = { test_template: { test: 1, @@ -80,7 +80,7 @@ suite("importOverrideDiagnostics", () => { ); }); - test("returns an unnescessaryImportOverride if a property has been overridden with the same value", async () => { + test("returns an unnecessaryImportOverride if a property has been overridden with the same value", async () => { const documentContent = { test_template: { test: 1, @@ -106,7 +106,7 @@ suite("importOverrideDiagnostics", () => { ); }); - test("returns an unnescessaryImportOverride if an options property has been overridden with the same value", async () => { + test("returns an unnecessaryImportOverride if an options property has been overridden with the same value", async () => { const documentContent = { test_template: { options: [ From 3fa5c226ca86145d90f707b011aa83c381425070 Mon Sep 17 00:00:00 2001 From: Steven Chetwynd Date: Wed, 9 Jul 2025 14:35:36 +0100 Subject: [PATCH 6/6] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20rename=20variabl?= =?UTF-8?q?e,=20change=20return=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit change return value when import is overwritten, but not unnecessarily --- src/diagnostics/importOverrideDiagnostics.ts | 6 ++--- .../importOverrideDiagnostics.test.ts | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/diagnostics/importOverrideDiagnostics.ts b/src/diagnostics/importOverrideDiagnostics.ts index 6717793..e2697d3 100644 --- a/src/diagnostics/importOverrideDiagnostics.ts +++ b/src/diagnostics/importOverrideDiagnostics.ts @@ -79,10 +79,10 @@ export function generateImportOverrideDiagnostics( if (!block) continue; const [imp, properties] = block; - for (const [name, value, propNode] of properties) { + for (const [name, valueNode, propNode] of properties) { const originalValue = imp[name]; - if (value && !isJSONDifferentToAST(originalValue, value)) { + if (valueNode && !isJSONDifferentToAST(originalValue, valueNode)) { ret.push({ type: DiagnosticType.UnnecessaryImportOverride, range: rangeFromNode(config.original, propNode.parent), @@ -91,7 +91,7 @@ export function generateImportOverrideDiagnostics( ret.push({ type: DiagnosticType.ImportOverride, range: rangeFromNode(config.original, propNode), - value, + value: getPropertyValueFromNode(propNode), originalValue, }); } diff --git a/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts index 63f9bfb..aec5009 100644 --- a/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts +++ b/src/test/suite/diagnostics/importOverrideDiagnostics.test.ts @@ -139,5 +139,32 @@ suite("importOverrideDiagnostics", () => { "Incorrect Diagnostic Type", ); }); + + test("returns an importOverride if a property is overwritten", async () => { + const documentContent = { + test_template: { + testProperty: 5, + }, + paramInformation: [ + { + "#": 1, + $import: "#test_template", + testProperty: 7, + }, + ], + }; + + const document = await setup(documentContent); + + const result = generateImportOverrideDiagnostics(document) as any; + + assert.strictEqual(result.length, 1); + assert.deepStrictEqual( + result[0].type, + DiagnosticType.ImportOverride, + ); + assert.deepStrictEqual(result[0].value, 7); + assert.deepStrictEqual(result[0].originalValue, 5); + }); }); });