From ff1b9a03ead9a7533e310cacf6e8edf9a379cdf6 Mon Sep 17 00:00:00 2001 From: Bill Barry Date: Sun, 31 May 2020 20:27:05 -0400 Subject: [PATCH 1/2] improve type safety with regards to config files --- src/creation/eslint/createEnv.test.ts | 14 ++++ src/creation/eslint/createEnv.ts | 4 +- src/input/findESLintConfiguration.ts | 10 +-- src/input/findOriginalConfigurations.ts | 3 +- src/input/findPackagesConfiguration.ts | 4 +- src/input/findReportedConfiguration.test.ts | 21 ++++- src/input/findReportedConfiguration.ts | 2 +- src/input/findTSLintConfiguration.ts | 2 +- src/input/findTypeScriptConfiguration.ts | 4 +- src/input/mergeLintConfigurations.ts | 2 +- src/rules/convertRules.test.ts | 13 +++ src/rules/convertRules.ts | 87 +++++++++++---------- 12 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/creation/eslint/createEnv.test.ts b/src/creation/eslint/createEnv.test.ts index c648ca567..a2cfe2738 100644 --- a/src/creation/eslint/createEnv.test.ts +++ b/src/creation/eslint/createEnv.test.ts @@ -63,6 +63,20 @@ describe("createEnv", () => { }); }); + it("handles an environment where typescript configuration files are mostly undefined", () => { + // Arrange + const packages = undefined; + const typescript = {}; + + // Act + const env = createEnv({ packages, typescript }); + + // Assert + expect(env).not.toContain({ + browser: expect.any(Boolean), + }); + }); + it("returns browser as true if a typescript lib is dom", () => { // Arrange const packages = undefined; diff --git a/src/creation/eslint/createEnv.ts b/src/creation/eslint/createEnv.ts index a02b617da..771cd6780 100644 --- a/src/creation/eslint/createEnv.ts +++ b/src/creation/eslint/createEnv.ts @@ -6,12 +6,12 @@ export const createEnv = ({ }: Pick) => { const browser = typescript === undefined || - typescript.compilerOptions.lib === undefined || + typescript.compilerOptions?.lib === undefined || typescript.compilerOptions.lib.includes("dom"); const es6 = typescript === undefined || - !["es3", "es5"].includes(typescript.compilerOptions.target.toLowerCase()); + !["es3", "es5"].includes(typescript.compilerOptions?.target?.toLowerCase() ?? ""); const node = packages === undefined || diff --git a/src/input/findESLintConfiguration.ts b/src/input/findESLintConfiguration.ts index 5b301d2fc..34175eabb 100644 --- a/src/input/findESLintConfiguration.ts +++ b/src/input/findESLintConfiguration.ts @@ -9,10 +9,10 @@ import { OriginalConfigurations } from "./findOriginalConfigurations"; import { importer } from "./importer"; export type ESLintConfiguration = { - env: Record; - extends: string | string[]; - globals?: Record; - rules: ESLintConfigurationRules; + env?: Record; + extends?: string | string[]; + globals?: Record; + rules?: ESLintConfigurationRules; }; export type ESLintConfigurationRules = { @@ -43,7 +43,7 @@ export const findESLintConfiguration = async ( findRawConfiguration(dependencies.importer, filePath, { extends: [], }), - findReportedConfiguration>( + findReportedConfiguration( dependencies.exec, "eslint --print-config", filePath, diff --git a/src/input/findOriginalConfigurations.ts b/src/input/findOriginalConfigurations.ts index 4364c47a6..654d7c5f5 100644 --- a/src/input/findOriginalConfigurations.ts +++ b/src/input/findOriginalConfigurations.ts @@ -14,6 +14,7 @@ import { } from "./findTypeScriptConfiguration"; import { findTSLintConfiguration, TSLintConfiguration } from "./findTSLintConfiguration"; import { mergeLintConfigurations } from "./mergeLintConfigurations"; +import { DeepPartial } from "./findReportedConfiguration"; export type FindOriginalConfigurationsDependencies = { findESLintConfiguration: SansDependencies; @@ -35,7 +36,7 @@ export type OriginalConfigurations = { /** * Raw import results from `import`ing the configuration file. */ - raw: Partial; + raw: DeepPartial; }; export type AllOriginalConfigurations = { diff --git a/src/input/findPackagesConfiguration.ts b/src/input/findPackagesConfiguration.ts index 42c79b160..5e614e0d2 100644 --- a/src/input/findPackagesConfiguration.ts +++ b/src/input/findPackagesConfiguration.ts @@ -4,8 +4,8 @@ import { } from "./findReportedConfiguration"; export type PackagesConfiguration = { - dependencies: Record; - devDependencies: Record; + dependencies: Record; + devDependencies: Record; }; export const findPackagesConfiguration = async ( diff --git a/src/input/findReportedConfiguration.test.ts b/src/input/findReportedConfiguration.test.ts index 55a2390a4..4493552d9 100644 --- a/src/input/findReportedConfiguration.test.ts +++ b/src/input/findReportedConfiguration.test.ts @@ -1,5 +1,5 @@ import { createStubExec, createStubThrowingExec } from "../adapters/exec.stubs"; -import { findReportedConfiguration } from "./findReportedConfiguration"; +import { findReportedConfiguration, DeepPartial } from "./findReportedConfiguration"; describe("findReportedConfiguration", () => { it("returns stderr as an error when the command fails with a zero exit code", async () => { @@ -56,4 +56,23 @@ describe("findReportedConfiguration", () => { rules, }); }); + + it("declares a correct DeepPartial type", () => { + type RulesType = { + "rule-a": boolean; + "rule-b": { component: string; deeper: { key: boolean } }; + }; + type PartialRules = DeepPartial; + + type Expected = { + "rule-a"?: boolean; + "rule-b"?: { component?: string; deeper?: { key?: boolean } }; + }; + + type Equals = (() => T extends X ? 1 : 2) extends () => T extends Y ? 1 : 2 + ? true + : false; + const assertion: Equals = true; + expect(assertion).toBeTruthy(); + }); }); diff --git a/src/input/findReportedConfiguration.ts b/src/input/findReportedConfiguration.ts index 915e99b97..df199f56e 100644 --- a/src/input/findReportedConfiguration.ts +++ b/src/input/findReportedConfiguration.ts @@ -1,7 +1,7 @@ import { Exec } from "../adapters/exec"; export type DeepPartial = { - [P in keyof T]?: T[P] extends Record ? DeepPartial : T[P]; + [P in keyof T]?: T[P] extends Record ? DeepPartial : T[P]; }; export type FindReportedConfigurationDependencies = { diff --git a/src/input/findTSLintConfiguration.ts b/src/input/findTSLintConfiguration.ts index 0219baa17..e932c2337 100644 --- a/src/input/findTSLintConfiguration.ts +++ b/src/input/findTSLintConfiguration.ts @@ -8,7 +8,7 @@ import { importer } from "./importer"; export type TSLintConfiguration = { extends?: string[]; rulesDirectory?: string[]; - rules: TSLintConfigurationRules; + rules?: TSLintConfigurationRules; }; export type TSLintConfigurationRules = Record; diff --git a/src/input/findTypeScriptConfiguration.ts b/src/input/findTypeScriptConfiguration.ts index f9c423ae6..afc46e333 100644 --- a/src/input/findTypeScriptConfiguration.ts +++ b/src/input/findTypeScriptConfiguration.ts @@ -4,9 +4,9 @@ import { } from "./findReportedConfiguration"; export type TypeScriptConfiguration = { - compilerOptions: { + compilerOptions?: { lib?: string[]; - target: string; + target?: string; }; }; diff --git a/src/input/mergeLintConfigurations.ts b/src/input/mergeLintConfigurations.ts index de2bb80fd..711a6d23c 100644 --- a/src/input/mergeLintConfigurations.ts +++ b/src/input/mergeLintConfigurations.ts @@ -10,7 +10,7 @@ export const mergeLintConfigurations = ( return tslint; } - const mappedConfig = eslint.full.rules["@typescript-eslint/tslint/config"]; + const mappedConfig = eslint.full.rules?.["@typescript-eslint/tslint/config"]; if (!(mappedConfig instanceof Array) || mappedConfig[0] === "off") { return tslint; } diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index d542a407a..42714f6e5 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -5,6 +5,19 @@ import { RuleConverter, ConversionResult } from "./converter"; import { RuleMerger } from "./merger"; describe("convertRules", () => { + it("doesn't crash with a type error when passed undefined configuration", () => { + // Arrange + const { converters, mergers } = setupConversionEnvironment({ + ruleSeverity: "off", + }); + + // Act + const { missing } = convertRules({ converters, mergers }, undefined); + + // Assert + expect(missing).toEqual([]); + }); + it("doesn't marks a disabled rule as missing when its converter returns undefined", () => { // Arrange const { tslintRule, converters, mergers } = setupConversionEnvironment({ diff --git a/src/rules/convertRules.ts b/src/rules/convertRules.ts index e4a77816c..29db1b5f7 100644 --- a/src/rules/convertRules.ts +++ b/src/rules/convertRules.ts @@ -22,66 +22,67 @@ export type RuleConversionResults = { export const convertRules = ( dependencies: ConvertRulesDependencies, - rawTslintRules: TSLintConfigurationRules, + rawTslintRules?: TSLintConfigurationRules, ): RuleConversionResults => { const converted = new Map(); const failed: ConversionError[] = []; const missing: TSLintRuleOptions[] = []; const plugins = new Set(); - for (const [ruleName, value] of Object.entries(rawTslintRules)) { - const tslintRule = formatRawTslintRule(ruleName, value); - const conversion = convertRule(tslintRule, dependencies.converters); + if (rawTslintRules !== undefined) { + for (const [ruleName, value] of Object.entries(rawTslintRules)) { + const tslintRule = formatRawTslintRule(ruleName, value); + const conversion = convertRule(tslintRule, dependencies.converters); - if (conversion === undefined) { - if (tslintRule.ruleSeverity !== "off") { - missing.push(tslintRule); - } - - continue; - } + if (conversion === undefined) { + if (tslintRule.ruleSeverity !== "off") { + missing.push(tslintRule); + } - if (conversion instanceof ConversionError) { - failed.push(conversion); - continue; - } - - for (const changes of conversion.rules) { - const existingConversion = converted.get(changes.ruleName); - const newConversion = { - ...changes, - ruleSeverity: convertTSLintRuleSeverity(tslintRule.ruleSeverity), - }; + continue; + } - if (existingConversion === undefined) { - converted.set(changes.ruleName, newConversion); + if (conversion instanceof ConversionError) { + failed.push(conversion); continue; } - const merger = dependencies.mergers.get(changes.ruleName); - if (merger === undefined) { - failed.push(ConversionError.forMerger(changes.ruleName)); - } else { - const existingNotices = existingConversion.notices ?? []; - const newNotices = newConversion.notices ?? []; + for (const changes of conversion.rules) { + const existingConversion = converted.get(changes.ruleName); + const newConversion = { + ...changes, + ruleSeverity: convertTSLintRuleSeverity(tslintRule.ruleSeverity), + }; + + if (existingConversion === undefined) { + converted.set(changes.ruleName, newConversion); + continue; + } + + const merger = dependencies.mergers.get(changes.ruleName); + if (merger === undefined) { + failed.push(ConversionError.forMerger(changes.ruleName)); + } else { + const existingNotices = existingConversion.notices ?? []; + const newNotices = newConversion.notices ?? []; - converted.set(changes.ruleName, { - ...existingConversion, - ruleArguments: merger( - existingConversion.ruleArguments, - newConversion.ruleArguments, - ), - notices: Array.from(new Set([...existingNotices, ...newNotices])), - }); + converted.set(changes.ruleName, { + ...existingConversion, + ruleArguments: merger( + existingConversion.ruleArguments, + newConversion.ruleArguments, + ), + notices: Array.from(new Set([...existingNotices, ...newNotices])), + }); + } } - } - if (conversion.plugins !== undefined) { - for (const newPlugin of conversion.plugins) { - plugins.add(newPlugin); + if (conversion.plugins !== undefined) { + for (const newPlugin of conversion.plugins) { + plugins.add(newPlugin); + } } } } - return { converted, failed, missing, plugins }; }; From f4511b92746df2c937707915df921029a2f16979 Mon Sep 17 00:00:00 2001 From: Bill Barry Date: Mon, 1 Jun 2020 17:13:57 -0400 Subject: [PATCH 2/2] removing type test + cleaning up test description --- src/input/findReportedConfiguration.test.ts | 21 +-------------------- src/rules/convertRules.test.ts | 2 +- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/input/findReportedConfiguration.test.ts b/src/input/findReportedConfiguration.test.ts index 4493552d9..55a2390a4 100644 --- a/src/input/findReportedConfiguration.test.ts +++ b/src/input/findReportedConfiguration.test.ts @@ -1,5 +1,5 @@ import { createStubExec, createStubThrowingExec } from "../adapters/exec.stubs"; -import { findReportedConfiguration, DeepPartial } from "./findReportedConfiguration"; +import { findReportedConfiguration } from "./findReportedConfiguration"; describe("findReportedConfiguration", () => { it("returns stderr as an error when the command fails with a zero exit code", async () => { @@ -56,23 +56,4 @@ describe("findReportedConfiguration", () => { rules, }); }); - - it("declares a correct DeepPartial type", () => { - type RulesType = { - "rule-a": boolean; - "rule-b": { component: string; deeper: { key: boolean } }; - }; - type PartialRules = DeepPartial; - - type Expected = { - "rule-a"?: boolean; - "rule-b"?: { component?: string; deeper?: { key?: boolean } }; - }; - - type Equals = (() => T extends X ? 1 : 2) extends () => T extends Y ? 1 : 2 - ? true - : false; - const assertion: Equals = true; - expect(assertion).toBeTruthy(); - }); }); diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index 42714f6e5..1d220118c 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -5,7 +5,7 @@ import { RuleConverter, ConversionResult } from "./converter"; import { RuleMerger } from "./merger"; describe("convertRules", () => { - it("doesn't crash with a type error when passed undefined configuration", () => { + it("doesn't crash when passed an undefined configuration", () => { // Arrange const { converters, mergers } = setupConversionEnvironment({ ruleSeverity: "off",