From 3734366122acc051eda7ce853aab3fb2f213ee37 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 7 Jul 2021 08:29:03 -0400 Subject: [PATCH 1/3] WIP: JSON patching --- .../converters/convertVSCodeConfig.ts | 122 +++++++++++++++--- 1 file changed, 102 insertions(+), 20 deletions(-) diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts index 5a7738d9e..a58dc0616 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -1,5 +1,5 @@ -import { merge } from "lodash"; import * as path from "path"; +import * as ts from "typescript"; import { parseJson } from "../../../utils"; import { EditorConfigConverter } from "../types"; @@ -12,8 +12,19 @@ const knownMissingSettings = [ "tslint.suppressWhileTypeErrorsPresent", ]; +const getJsonRoot = (sourceFile: ts.SourceFile) => { + const [rootStatement] = sourceFile.statements; + + return ts.isExpressionStatement(rootStatement) && ts.isObjectLiteralExpression(rootStatement.expression) + ? rootStatement.expression + : undefined; +} + + export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, settings) => { const editorSettings: Record = parseJson(rawEditorSettings); + const missing = knownMissingSettings.filter((setting) => editorSettings[setting]); + const autoFixOnSave = editorSettings["editor.codeActionsOnSave"] && typeof editorSettings["editor.codeActionsOnSave"] === "object" && @@ -28,26 +39,97 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se path.dirname(settings.config), ); - const contents = JSON.stringify( - merge( - {}, - editorSettings, - autoFixOnSave !== undefined && { - "editor.codeActionsOnSave": { - "eslint.autoFixOnSave": autoFixOnSave, - }, - }, - eslintPathMatches && { - "eslint.options": { - configFile: settings.config, - }, - }, - ), - null, - 4, - ); + // We can bail without making changes if there are no changes we need to make... + if (!autoFixOnSave && !eslintPathMatches) { + return { contents: rawEditorSettings, missing }; + } - const missing = knownMissingSettings.filter((setting) => editorSettings[setting]); + // ...or the JSON file doesn't seem to be a normal {} object root + const sourceFile = ts.createSourceFile("settings.json", rawEditorSettings, ts.ScriptTarget.Latest, /*setParentNodes*/ true, ts.ScriptKind.JSON); + const jsonRoot = getJsonRoot(sourceFile); + if (!jsonRoot) { + return { contents: rawEditorSettings, missing }; + } + + const propertyIndexByName = (properties: ts.NodeArray, name: string) => + properties.findIndex(property => property.name && ts.isStringLiteral(property.name) && property.name.text === name); + + const transformer = (context: ts.TransformationContext) => (rootNode: ts.SourceFile): ts.SourceFile => { + const upsertProperties = (node: ts.ObjectLiteralExpression, additions: readonly [string, string, unknown][]) => { + const originalProperties = node.properties; + + for (const [parent, setting, value] of additions) { + const createNewChild = (properties?: ts.NodeArray) => { + return context.factory.createPropertyAssignment( + `"${parent}"`, + context.factory.createObjectLiteralExpression( + [ + ...properties ?? [], + context.factory.createPropertyAssignment( + `"${setting}"`, + typeof value === "string" + ? context.factory.createStringLiteral(value) + : value + ? context.factory.createTrue() + : context.factory.createFalse() + ) + ], + true + ), + ); + } + + const existingIndex = propertyIndexByName(originalProperties, parent); + + if (existingIndex !== -1) { + const existingProperty = originalProperties[existingIndex]; + if ( + !ts.isPropertyAssignment(existingProperty) + || !ts.isObjectLiteralExpression(existingProperty.initializer) + || propertyIndexByName(existingProperty.initializer.properties, `"${parent}"`) === -1) { + return node; + } + + const updatedProperties = [...node.properties]; + updatedProperties[existingIndex] = createNewChild(existingProperty.initializer.properties) + node = context.factory.createObjectLiteralExpression(updatedProperties, true); + } else { + node = context.factory.createObjectLiteralExpression([...node.properties, createNewChild()], true); + } + } + + return node; + }; + + const visit = (node: ts.Node) => { + node = ts.visitEachChild(node, visit, context); + + if (node !== jsonRoot) { + return node; + } + + const additions: [string, string, unknown][] = []; + + if (autoFixOnSave !== undefined) { + additions.push(["editor.codeActionsOnSave", "eslint.autoFixOnSave", autoFixOnSave]); + } + + if (eslintPathMatches !== undefined) { + additions.push(["eslint.options", "configFile", settings.config]); + } + + return upsertProperties(jsonRoot, additions); + }; + + return ts.visitNode(rootNode, visit) + }; + + const printer = ts.createPrinter(undefined); + const result = ts.transform(sourceFile, [transformer]); + const contents = printer.printFile(result.transformed[0]) + .replace(/^\(/giu, "") + .replace(/\);(\r\n|\r|\n)*$/giu, "$1") + result.dispose(); return { contents, missing }; }; From eb735043bb9a487849f9795f9a872d6a7e0bb09a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 7 Jul 2021 09:08:14 -0400 Subject: [PATCH 2/3] Fixed up tests --- .../converters/convertVSCodeConfig.test.ts | 150 ++++++++++++------ .../converters/convertVSCodeConfig.ts | 33 +--- 2 files changed, 108 insertions(+), 75 deletions(-) diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts index 1b3852e9d..9efdca710 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts @@ -10,42 +10,87 @@ describe("convertVSCodeConfig", () => { const editorSettings = { unrelated: true }; // Act - const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); // Assert - expect(result).toEqual({ - contents: JSON.stringify(editorSettings, null, 4), - missing: [], - }); + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "{ + \\"unrelated\\": true +}", + "missing": Array [], +} +`); }); - it("includes eslint.autoFixOnSave when source.fixAll.tslint exists", () => { + it("preserves original settings when the input structure is not an object", () => { + // Arrange + const editorSettings: never[] = []; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); + + // Assert + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "[]", + "missing": Array [], +} +`); + }); + + it("does not include eslint.autoFixOnSave when source.fixAll.tslint is false", () => { // Arrange const editorSettings = { "editor.codeActionsOnSave": { - "source.fixAll.tslint": true, + "source.fixAll.tslint": false, }, unrelated: true, }; // Act - const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); // Assert - expect(result).toEqual({ - contents: JSON.stringify( - { - "editor.codeActionsOnSave": { - "source.fixAll.tslint": true, - "eslint.autoFixOnSave": true, - }, - unrelated: true, - }, - null, - 4, - ), - missing: [], - }); + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "{ + \\"editor.codeActionsOnSave\\": { + \\"source.fixAll.tslint\\": false + }, + \\"unrelated\\": true +}", + "missing": Array [], +} +`); + }); + + it("includes eslint.autoFixOnSave when source.fixAll.tslint is true", () => { + // Arrange + const editorSettings = { + "editor.codeActionsOnSave": { + "source.fixAll.tslint": true, + }, + unrelated: false, + }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); + + // Assert + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "{ + \\"editor.codeActionsOnSave\\": { + \\"source.fixAll.tslint\\": true, + \\"eslint.autoFixOnSave\\": true + }, + \\"unrelated\\": false +} +", + "missing": Array [], +} +`); }); it("does not include configFile when tslint.configFile does not match the output config", () => { @@ -56,7 +101,7 @@ describe("convertVSCodeConfig", () => { }; // Act - const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); // Assert expect(result).toEqual({ @@ -73,23 +118,22 @@ describe("convertVSCodeConfig", () => { }; // Act - const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); // Assert - expect(result).toEqual({ - contents: JSON.stringify( - { - "tslint.configFile": "./tslint.json", - unrelated: true, - "eslint.options": { - configFile: stubSettings.config, - }, - }, - null, - 4, - ), - missing: [], - }); + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "{ + \\"tslint.configFile\\": \\"./tslint.json\\", + \\"unrelated\\": true, + \\"eslint.options\\": { + \\"configFile\\": \\".eslintrc.js\\" + } +} +", + "missing": Array [], +} +`); }); it("includes missing notices when known missing settings are included", () => { @@ -103,18 +147,26 @@ describe("convertVSCodeConfig", () => { }; // Act - const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + const result = convertVSCodeConfig(JSON.stringify(editorSettings, null, 4), stubSettings); // Assert - expect(result).toEqual({ - contents: JSON.stringify(editorSettings, null, 4), - missing: [ - "tslint.alwaysShowRuleFailuresAsWarnings", - "tslint.exclude", - "tslint.ignoreDefinitionFiles", - "tslint.jsEnable", - "tslint.suppressWhileTypeErrorsPresent", - ], - }); + expect(result).toMatchInlineSnapshot(` +Object { + "contents": "{ + \\"tslint.alwaysShowRuleFailuresAsWarnings\\": true, + \\"tslint.exclude\\": true, + \\"tslint.ignoreDefinitionFiles\\": true, + \\"tslint.jsEnable\\": true, + \\"tslint.suppressWhileTypeErrorsPresent\\": true +}", + "missing": Array [ + "tslint.alwaysShowRuleFailuresAsWarnings", + "tslint.exclude", + "tslint.ignoreDefinitionFiles", + "tslint.jsEnable", + "tslint.suppressWhileTypeErrorsPresent", + ], +} +`); }); }); diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts index a58dc0616..4f710970c 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -12,15 +12,6 @@ const knownMissingSettings = [ "tslint.suppressWhileTypeErrorsPresent", ]; -const getJsonRoot = (sourceFile: ts.SourceFile) => { - const [rootStatement] = sourceFile.statements; - - return ts.isExpressionStatement(rootStatement) && ts.isObjectLiteralExpression(rootStatement.expression) - ? rootStatement.expression - : undefined; -} - - export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, settings) => { const editorSettings: Record = parseJson(rawEditorSettings); const missing = knownMissingSettings.filter((setting) => editorSettings[setting]); @@ -39,17 +30,14 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se path.dirname(settings.config), ); - // We can bail without making changes if there are no changes we need to make... + // We can bail without making changes if there are no changes we need to make if (!autoFixOnSave && !eslintPathMatches) { return { contents: rawEditorSettings, missing }; } - // ...or the JSON file doesn't seem to be a normal {} object root + // Since we've found at least one matching setting, we know the source structure is a proper {} const sourceFile = ts.createSourceFile("settings.json", rawEditorSettings, ts.ScriptTarget.Latest, /*setParentNodes*/ true, ts.ScriptKind.JSON); - const jsonRoot = getJsonRoot(sourceFile); - if (!jsonRoot) { - return { contents: rawEditorSettings, missing }; - } + const jsonRoot = (sourceFile.statements[0] as ts.ExpressionStatement).expression as ts.ObjectLiteralExpression; const propertyIndexByName = (properties: ts.NodeArray, name: string) => properties.findIndex(property => property.name && ts.isStringLiteral(property.name) && property.name.text === name); @@ -69,9 +57,7 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se `"${setting}"`, typeof value === "string" ? context.factory.createStringLiteral(value) - : value - ? context.factory.createTrue() - : context.factory.createFalse() + : context.factory.createTrue() ) ], true @@ -83,15 +69,10 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se if (existingIndex !== -1) { const existingProperty = originalProperties[existingIndex]; - if ( - !ts.isPropertyAssignment(existingProperty) - || !ts.isObjectLiteralExpression(existingProperty.initializer) - || propertyIndexByName(existingProperty.initializer.properties, `"${parent}"`) === -1) { - return node; - } - const updatedProperties = [...node.properties]; - updatedProperties[existingIndex] = createNewChild(existingProperty.initializer.properties) + + // We know these casts should be safe because we previously found a matching parent object for the property + updatedProperties[existingIndex] = createNewChild(((existingProperty as ts.PropertyAssignment).initializer as ts.ObjectLiteralExpression).properties as ts.NodeArray | undefined) node = context.factory.createObjectLiteralExpression(updatedProperties, true); } else { node = context.factory.createObjectLiteralExpression([...node.properties, createNewChild()], true); From 9fae612cb3ad60687d8701d74142fbf31854dee1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 7 Jul 2021 11:25:05 -0400 Subject: [PATCH 3/3] Looks like my Prettier hook is not running locally or in CI --- .../converters/convertVSCodeConfig.ts | 154 +++++++++++------- src/input/findPackagesConfiguration.ts | 14 +- 2 files changed, 102 insertions(+), 66 deletions(-) diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts index 4f710970c..bf98cd344 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -36,80 +36,116 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se } // Since we've found at least one matching setting, we know the source structure is a proper {} - const sourceFile = ts.createSourceFile("settings.json", rawEditorSettings, ts.ScriptTarget.Latest, /*setParentNodes*/ true, ts.ScriptKind.JSON); - const jsonRoot = (sourceFile.statements[0] as ts.ExpressionStatement).expression as ts.ObjectLiteralExpression; - - const propertyIndexByName = (properties: ts.NodeArray, name: string) => - properties.findIndex(property => property.name && ts.isStringLiteral(property.name) && property.name.text === name); - - const transformer = (context: ts.TransformationContext) => (rootNode: ts.SourceFile): ts.SourceFile => { - const upsertProperties = (node: ts.ObjectLiteralExpression, additions: readonly [string, string, unknown][]) => { - const originalProperties = node.properties; - - for (const [parent, setting, value] of additions) { - const createNewChild = (properties?: ts.NodeArray) => { - return context.factory.createPropertyAssignment( - `"${parent}"`, - context.factory.createObjectLiteralExpression( - [ - ...properties ?? [], - context.factory.createPropertyAssignment( - `"${setting}"`, - typeof value === "string" - ? context.factory.createStringLiteral(value) - : context.factory.createTrue() - ) - ], - true - ), - ); + const sourceFile = ts.createSourceFile( + "settings.json", + rawEditorSettings, + ts.ScriptTarget.Latest, + /*setParentNodes*/ true, + ts.ScriptKind.JSON, + ); + const jsonRoot = (sourceFile.statements[0] as ts.ExpressionStatement) + .expression as ts.ObjectLiteralExpression; + + const propertyIndexByName = ( + properties: ts.NodeArray, + name: string, + ) => + properties.findIndex( + (property) => + property.name && ts.isStringLiteral(property.name) && property.name.text === name, + ); + + const transformer = + (context: ts.TransformationContext) => + (rootNode: ts.SourceFile): ts.SourceFile => { + const upsertProperties = ( + node: ts.ObjectLiteralExpression, + additions: readonly [string, string, unknown][], + ) => { + const originalProperties = node.properties; + + for (const [parent, setting, value] of additions) { + const createNewChild = ( + properties?: ts.NodeArray, + ) => { + return context.factory.createPropertyAssignment( + `"${parent}"`, + context.factory.createObjectLiteralExpression( + [ + ...(properties ?? []), + context.factory.createPropertyAssignment( + `"${setting}"`, + typeof value === "string" + ? context.factory.createStringLiteral(value) + : context.factory.createTrue(), + ), + ], + true, + ), + ); + }; + + const existingIndex = propertyIndexByName(originalProperties, parent); + + if (existingIndex !== -1) { + const existingProperty = originalProperties[existingIndex]; + const updatedProperties = [...node.properties]; + + // We know these casts should be safe because we previously found a matching parent object for the property + updatedProperties[existingIndex] = createNewChild( + ( + (existingProperty as ts.PropertyAssignment) + .initializer as ts.ObjectLiteralExpression + ).properties as ts.NodeArray | undefined, + ); + node = context.factory.createObjectLiteralExpression( + updatedProperties, + true, + ); + } else { + node = context.factory.createObjectLiteralExpression( + [...node.properties, createNewChild()], + true, + ); + } } - const existingIndex = propertyIndexByName(originalProperties, parent); + return node; + }; - if (existingIndex !== -1) { - const existingProperty = originalProperties[existingIndex]; - const updatedProperties = [...node.properties]; + const visit = (node: ts.Node) => { + node = ts.visitEachChild(node, visit, context); - // We know these casts should be safe because we previously found a matching parent object for the property - updatedProperties[existingIndex] = createNewChild(((existingProperty as ts.PropertyAssignment).initializer as ts.ObjectLiteralExpression).properties as ts.NodeArray | undefined) - node = context.factory.createObjectLiteralExpression(updatedProperties, true); - } else { - node = context.factory.createObjectLiteralExpression([...node.properties, createNewChild()], true); + if (node !== jsonRoot) { + return node; } - } - - return node; - }; - const visit = (node: ts.Node) => { - node = ts.visitEachChild(node, visit, context); + const additions: [string, string, unknown][] = []; - if (node !== jsonRoot) { - return node; - } - - const additions: [string, string, unknown][] = []; + if (autoFixOnSave !== undefined) { + additions.push([ + "editor.codeActionsOnSave", + "eslint.autoFixOnSave", + autoFixOnSave, + ]); + } - if (autoFixOnSave !== undefined) { - additions.push(["editor.codeActionsOnSave", "eslint.autoFixOnSave", autoFixOnSave]); - } + if (eslintPathMatches !== undefined) { + additions.push(["eslint.options", "configFile", settings.config]); + } - if (eslintPathMatches !== undefined) { - additions.push(["eslint.options", "configFile", settings.config]); - } + return upsertProperties(jsonRoot, additions); + }; - return upsertProperties(jsonRoot, additions); + return ts.visitNode(rootNode, visit); }; - return ts.visitNode(rootNode, visit) - }; - const printer = ts.createPrinter(undefined); const result = ts.transform(sourceFile, [transformer]); - const contents = printer.printFile(result.transformed[0]) + const contents = printer + .printFile(result.transformed[0]) .replace(/^\(/giu, "") - .replace(/\);(\r\n|\r|\n)*$/giu, "$1") + .replace(/\);(\r\n|\r|\n)*$/giu, "$1"); result.dispose(); return { contents, missing }; diff --git a/src/input/findPackagesConfiguration.ts b/src/input/findPackagesConfiguration.ts index 900acc0da..5e614e0d2 100644 --- a/src/input/findPackagesConfiguration.ts +++ b/src/input/findPackagesConfiguration.ts @@ -21,11 +21,11 @@ export const findPackagesConfiguration = async ( return rawConfiguration instanceof Error ? rawConfiguration : { - dependencies: { - ...rawConfiguration.dependencies, - }, - devDependencies: { - ...rawConfiguration.devDependencies, - }, - }; + dependencies: { + ...rawConfiguration.dependencies, + }, + devDependencies: { + ...rawConfiguration.devDependencies, + }, + }; };