From 1be3cd9ac008571dcf4a1e9491a726bfa4cb65cb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 26 Apr 2020 22:25:49 -0400 Subject: [PATCH 1/3] WIP: excluded package names from install suggestion --- .eslintrc.js | 1 + src/cli/main.ts | 18 ++- src/conversion/convertConfig.test.ts | 1 + src/conversion/convertConfig.ts | 6 + .../simplifyPackageRules.test.ts | 5 +- .../simplification/simplifyPackageRules.ts | 5 +- src/input/findPackagesConfiguration.ts | 15 +-- .../packages/getPackageNameFromExtends.ts | 11 ++ .../packages/logMissingPackages.test.ts | 95 +++++++++++++++ src/reporting/packages/logMissingPackages.ts | 53 ++++++++ src/reporting/reportConversionResults.test.ts | 114 ++++-------------- src/reporting/reportConversionResults.ts | 8 -- src/reporting/reportOutputs.test.ts | 88 -------------- src/reporting/reportOutputs.ts | 29 ----- src/utils.ts | 4 +- 15 files changed, 219 insertions(+), 234 deletions(-) create mode 100644 src/reporting/packages/getPackageNameFromExtends.ts create mode 100644 src/reporting/packages/logMissingPackages.test.ts create mode 100644 src/reporting/packages/logMissingPackages.ts delete mode 100644 src/reporting/reportOutputs.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index c98de62b4..981a771fe 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -38,6 +38,7 @@ module.exports = { "@typescript-eslint/no-unsafe-return": "off", "@typescript-eslint/no-untyped-public-signature": "off", "@typescript-eslint/no-unused-vars": "off", + "@typescript-eslint/no-unused-vars-experimental": "off", "@typescript-eslint/no-use-before-define": "off", "@typescript-eslint/prefer-readonly-parameter-types": "off", "@typescript-eslint/prefer-reduce-type-parameter": "off", diff --git a/src/cli/main.ts b/src/cli/main.ts index bc2883176..a55117cfc 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -44,7 +44,14 @@ import { findTSLintConfiguration } from "../input/findTSLintConfiguration"; import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguration"; import { importer, ImporterDependencies } from "../input/importer"; import { mergeLintConfigurations } from "../input/mergeLintConfigurations"; -import { choosePackageManager } from "../reporting/packages/choosePackageManager"; +import { + ChoosePackageManagerDependencies, + choosePackageManager, +} from "../reporting/packages/choosePackageManager"; +import { + logMissingPackages, + LogMissingPackagesDependencies, +} from "../reporting/packages/logMissingPackages"; import { reportConversionResults, ReportConversionResultsDependencies, @@ -90,15 +97,19 @@ const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependen mergeLintConfigurations, }; -const choosePackageManagerDependencies = { +const choosePackageManagerDependencies: ChoosePackageManagerDependencies = { fileSystem: fsFileSystem, }; -const reportConversionResultsDependencies: ReportConversionResultsDependencies = { +const logMissingPackagesDependencies: LogMissingPackagesDependencies = { choosePackageManager: bind(choosePackageManager, choosePackageManagerDependencies), logger: processLogger, }; +const reportConversionResultsDependencies: ReportConversionResultsDependencies = { + logger: processLogger, +}; + const retrieveExtendsValuesDependencies: RetrieveExtendsValuesDependencies = { importer: boundImporter, }; @@ -132,6 +143,7 @@ const convertConfigDependencies: ConvertConfigDependencies = { findOriginalConfigurations, findOriginalConfigurationsDependencies, ), + logMissingPackages: bind(logMissingPackages, logMissingPackagesDependencies), reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies), simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies), writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies), diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index c3f4c8b4f..3fa1e53ea 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -22,6 +22,7 @@ const createStubDependencies = ( missing: [], plugins: new Set(), }), + logMissingPackages: jest.fn().mockReturnValue(Promise.resolve()), writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), ...overrides, }); diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index d7bbae2f3..4d965315e 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -2,6 +2,7 @@ import { SansDependencies } from "../binding"; import { simplifyPackageRules } from "../creation/simplification/simplifyPackageRules"; import { writeConversionResults } from "../creation/writeConversionResults"; import { findOriginalConfigurations } from "../input/findOriginalConfigurations"; +import { logMissingPackages } from "../reporting/packages/logMissingPackages"; import { reportConversionResults } from "../reporting/reportConversionResults"; import { convertRules } from "../rules/convertRules"; import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../types"; @@ -9,6 +10,7 @@ import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../types export type ConvertConfigDependencies = { convertRules: SansDependencies; findOriginalConfigurations: SansDependencies; + logMissingPackages: SansDependencies; reportConversionResults: SansDependencies; simplifyPackageRules: SansDependencies; writeConversionResults: SansDependencies; @@ -56,6 +58,10 @@ export const convertConfig = async ( // 5. A summary of the results is printed to the user's console await dependencies.reportConversionResults(settings.config, simplifiedConfiguration); + await dependencies.logMissingPackages( + simplifiedConfiguration, + originalConfigurations.data.packages, + ); return { status: ResultStatus.Succeeded, diff --git a/src/creation/simplification/simplifyPackageRules.test.ts b/src/creation/simplification/simplifyPackageRules.test.ts index ea119d452..07266dea9 100644 --- a/src/creation/simplification/simplifyPackageRules.test.ts +++ b/src/creation/simplification/simplifyPackageRules.test.ts @@ -68,7 +68,10 @@ describe("simplifyPackageRules", () => { expect(simplifiedResults).toEqual({ ...ruleConversionResults, converted: undefined, - extends: ["eslint-config-prettier", "eslint-config-prettier/@typescript-eslint"], + extends: [ + "plugin:eslint-config-prettier", + "plugin:eslint-config-prettier/@typescript-eslint", + ], }); }); diff --git a/src/creation/simplification/simplifyPackageRules.ts b/src/creation/simplification/simplifyPackageRules.ts index e16f18b71..3de99f5b0 100644 --- a/src/creation/simplification/simplifyPackageRules.ts +++ b/src/creation/simplification/simplifyPackageRules.ts @@ -36,7 +36,10 @@ export const simplifyPackageRules = async ( // 3a. If no output rules conflict with `eslint-config-prettier`, it's added in if (await dependencies.addPrettierExtensions(ruleConversionResults, prettierRequested)) { - allExtensions.push("eslint-config-prettier", "eslint-config-prettier/@typescript-eslint"); + allExtensions.push( + "plugin:eslint-config-prettier", + "plugin:eslint-config-prettier/@typescript-eslint", + ); } if (allExtensions.length === 0) { diff --git a/src/input/findPackagesConfiguration.ts b/src/input/findPackagesConfiguration.ts index 213f657c5..42c79b160 100644 --- a/src/input/findPackagesConfiguration.ts +++ b/src/input/findPackagesConfiguration.ts @@ -4,17 +4,8 @@ import { } from "./findReportedConfiguration"; export type PackagesConfiguration = { - dependencies: { - [i: string]: string; - }; - devDependencies: { - [i: string]: string; - }; -}; - -const defaultPackagesConfiguration = { - dependencies: {}, - devDependencies: {}, + dependencies: Record; + devDependencies: Record; }; export const findPackagesConfiguration = async ( @@ -32,11 +23,9 @@ export const findPackagesConfiguration = async ( : { dependencies: { ...rawConfiguration.dependencies, - ...defaultPackagesConfiguration.dependencies, }, devDependencies: { ...rawConfiguration.devDependencies, - ...defaultPackagesConfiguration.devDependencies, }, }; }; diff --git a/src/reporting/packages/getPackageNameFromExtends.ts b/src/reporting/packages/getPackageNameFromExtends.ts new file mode 100644 index 000000000..3bc69b61b --- /dev/null +++ b/src/reporting/packages/getPackageNameFromExtends.ts @@ -0,0 +1,11 @@ +export const getPackageNameFromExtends = (name: string) => { + if (name.startsWith("plugin:")) { + name = name.slice("plugin:".length); + } + + // This heuristic actually doesn't work :( + return name + .split("/") + .slice(0, name.startsWith("@") ? 2 : 1) + .join("/"); +}; diff --git a/src/reporting/packages/logMissingPackages.test.ts b/src/reporting/packages/logMissingPackages.test.ts new file mode 100644 index 000000000..eef4a8004 --- /dev/null +++ b/src/reporting/packages/logMissingPackages.test.ts @@ -0,0 +1,95 @@ +// import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs"; +// import { createEmptyConversionResults } from "../conversion/conversionResults.stubs"; +// import { PackageManager } from "./packages/packageManagers"; +// import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules"; + +import { createStubLogger, expectEqualWrites } from "../../adapters/logger.stubs"; +import { createEmptyConversionResults } from "../../conversion/conversionResults.stubs"; +import { SimplifiedResultsConfiguration } from "../../creation/simplification/simplifyPackageRules"; +import { logMissingPackages } from "./logMissingPackages"; +import { PackageManager } from "./packageManagers"; + +const createStubDependencies = ( + packageManager: PackageManager, + results?: Partial, +) => ({ + choosePackageManager: async () => packageManager, + logger: createStubLogger(), + ruleConversionResults: createEmptyConversionResults(results), +}); + +describe("logMissingPackages", () => { + it("reports an npm command when the package manager is npm", async () => { + // Arrange + const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( + PackageManager.npm, + ); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, + ); + }); + + it("reports a pnpm command when the package manager is pnpm", async () => { + // Arrange + const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( + PackageManager.pnpm, + ); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` pnpm add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, + ); + }); + + it("reports a Yarn command when the package manager is Yarn", async () => { + // Arrange + const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( + PackageManager.Yarn, + ); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --dev`, + ); + }); + + it("adds extensions to the missing packages list when they exist", async () => { + // Arrange + const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( + PackageManager.Yarn, + { + extends: [ + "plugin:eslint-config-prettier", + "plugin:eslint-config-prettier/@typescript-eslint", + ], + }, + ); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 4 new packages are required for this ESLint configuration. ⚡`, + ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, + ); + }); +}); diff --git a/src/reporting/packages/logMissingPackages.ts b/src/reporting/packages/logMissingPackages.ts new file mode 100644 index 000000000..337ce8290 --- /dev/null +++ b/src/reporting/packages/logMissingPackages.ts @@ -0,0 +1,53 @@ +import chalk from "chalk"; +import { EOL } from "os"; + +import { Logger } from "../../adapters/logger"; +import { SansDependencies } from "../../binding"; +import { SimplifiedResultsConfiguration } from "../../creation/simplification/simplifyPackageRules"; +import { PackagesConfiguration } from "../../input/findPackagesConfiguration"; +import { isTruthy } from "../../utils"; +import { installationMessages } from "../packages/packageManagers"; +import { choosePackageManager } from "./choosePackageManager"; +import { getPackageNameFromExtends } from "./getPackageNameFromExtends"; + +export type LogMissingPackagesDependencies = { + choosePackageManager: SansDependencies; + logger: Logger; +}; + +export const logMissingPackages = async ( + dependencies: LogMissingPackagesDependencies, + ruleConversionResults: Pick, + packageConfiguration?: PackagesConfiguration, +) => { + const packageManager = await dependencies.choosePackageManager(); + + const existingPackageNames = new Set([ + ...Object.keys(packageConfiguration?.dependencies ?? {}), + ...Object.keys(packageConfiguration?.devDependencies ?? {}), + ]); + + const requiredPackageNames = [ + "@typescript-eslint/eslint-plugin", + "@typescript-eslint/parser", + ruleConversionResults.missing.length !== 0 && "@typescript-eslint/eslint-plugin-tslint", + "eslint", + ...Array.from(ruleConversionResults.extends?.map(getPackageNameFromExtends) ?? []), + ...Array.from(ruleConversionResults.plugins), + ].filter(isTruthy); + + const missingPackageNames = requiredPackageNames.filter( + (packageName) => !existingPackageNames.has(packageName), + ); + + dependencies.logger.stdout.write(chalk.cyanBright(`${EOL}⚡ ${missingPackageNames.length}`)); + dependencies.logger.stdout.write( + chalk.cyan(" new packages are required for this ESLint configuration."), + ); + dependencies.logger.stdout.write(chalk.cyanBright(" ⚡")); + dependencies.logger.stdout.write(`${EOL} `); + dependencies.logger.stdout.write( + chalk.cyan(installationMessages[packageManager](missingPackageNames.join(" "))), + ); + dependencies.logger.stdout.write(EOL); +}; diff --git a/src/reporting/reportConversionResults.test.ts b/src/reporting/reportConversionResults.test.ts index 0b6a6e164..c5352e577 100644 --- a/src/reporting/reportConversionResults.test.ts +++ b/src/reporting/reportConversionResults.test.ts @@ -3,17 +3,12 @@ import { EOL } from "os"; import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs"; import { createEmptyConversionResults } from "../conversion/conversionResults.stubs"; import { ESLintRuleOptions } from "../rules/types"; -import { PackageManager } from "./packages/packageManagers"; import { reportConversionResults } from "./reportConversionResults"; -const createStubDependencies = (packageManager = PackageManager.Yarn) => { - const choosePackageManager = jest.fn().mockResolvedValueOnce(packageManager); - const logger = createStubLogger(); - - return { choosePackageManager, logger }; -}; - -const basicExtends = ["eslint-config-prettier", "eslint-config-prettier/@typescript-eslint"]; +const basicExtends = [ + "plugin:eslint-config-prettier", + "plugin:eslint-config-prettier/@typescript-eslint", +]; describe("reportConversionResults", () => { it("logs a successful conversion without notices when there is one converted rule without notices", async () => { @@ -32,23 +27,13 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert - expectEqualWrites( - logger.stdout.write, - `✨ 1 rule replaced with its ESLint equivalent. ✨`, - ``, - `⚡ 4 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, - ); + expectEqualWrites(logger.stdout.write, `✨ 1 rule replaced with its ESLint equivalent. ✨`); }); it("logs a successful conversion with notices when there is one converted rule with notices", async () => { @@ -68,14 +53,10 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -83,9 +64,6 @@ describe("reportConversionResults", () => { `✨ 1 rule replaced with its ESLint equivalent. ✨${EOL}`, `❗ 1 ESLint rule behaves differently from its TSLint counterpart ❗`, ` Check ${logger.debugFileName} for details.`, - ``, - `⚡ 4 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, ); expectEqualWrites( logger.info.write, @@ -122,14 +100,10 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -138,9 +112,6 @@ describe("reportConversionResults", () => { ``, `❗ 2 ESLint rules behave differently from their TSLint counterparts ❗`, ` Check ${logger.debugFileName} for details.`, - ``, - `⚡ 4 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, ); expectEqualWrites( logger.info.write, @@ -161,14 +132,10 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -185,14 +152,10 @@ describe("reportConversionResults", () => { failed: [{ getSummary: () => "It broke." }, { getSummary: () => "It really broke." }], }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -215,14 +178,10 @@ describe("reportConversionResults", () => { ], }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -230,9 +189,6 @@ describe("reportConversionResults", () => { `❓ 1 rule is not known by tslint-to-eslint-config to have an ESLint equivalent. ❓`, ` The "@typescript-eslint/tslint/config" section of .eslintrc.js configures eslint-plugin-tslint to run it in TSLint within ESLint.`, ` Check ${logger.debugFileName} for details.`, - ``, - `⚡ 5 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/eslint-plugin-tslint @typescript-eslint/parser eslint eslint-config-prettier --dev`, ); expectEqualWrites( logger.info.write, @@ -259,14 +215,10 @@ describe("reportConversionResults", () => { ], }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( @@ -274,9 +226,6 @@ describe("reportConversionResults", () => { `❓ 2 rules are not known by tslint-to-eslint-config to have ESLint equivalents. ❓`, ` The "@typescript-eslint/tslint/config" section of .eslintrc.js configures eslint-plugin-tslint to run them in TSLint within ESLint.`, ` Check ${logger.debugFileName} for details.`, - ``, - `⚡ 5 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/eslint-plugin-tslint @typescript-eslint/parser eslint eslint-config-prettier --dev`, ); expectEqualWrites( logger.info.write, @@ -293,21 +242,13 @@ describe("reportConversionResults", () => { plugins: new Set(["plugin-one", "plugin-two"]), }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert - expectEqualWrites( - logger.stdout.write, - `⚡ 6 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier plugin-one plugin-two --dev`, - ); + expectEqualWrites(); }); it("logs a Prettier recommendation when eslint-config-prettier isn't extended", async () => { @@ -316,20 +257,13 @@ describe("reportConversionResults", () => { extends: [], }); - const { choosePackageManager, logger } = createStubDependencies(); + const logger = createStubLogger(); // Act - await reportConversionResults( - { choosePackageManager, logger }, - ".eslintrc.js", - conversionResults, - ); + await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( - logger.stdout.write, - `⚡ 3 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --dev`, ``, `☠ Prettier plugins are missing from your configuration. ☠`, ` We highly recommend running tslint-to-eslint-config --prettier to disable formatting ESLint rules.`, diff --git a/src/reporting/reportConversionResults.ts b/src/reporting/reportConversionResults.ts index 30f1c3aa7..df9e32378 100644 --- a/src/reporting/reportConversionResults.ts +++ b/src/reporting/reportConversionResults.ts @@ -2,20 +2,16 @@ import chalk from "chalk"; import { EOL } from "os"; import { Logger } from "../adapters/logger"; -import { SansDependencies } from "../binding"; import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules"; import { ESLintRuleOptions, TSLintRuleOptions } from "../rules/types"; -import { choosePackageManager } from "./packages/choosePackageManager"; import { logFailedConversions, logMissingConversionTarget, - logMissingPackages, logSuccessfulConversions, } from "./reportOutputs"; export type ReportConversionResultsDependencies = { logger: Logger; - choosePackageManager: SansDependencies; }; export const reportConversionResults = async ( @@ -23,8 +19,6 @@ export const reportConversionResults = async ( outputPath: string, ruleConversionResults: SimplifiedResultsConfiguration, ) => { - const packageManager = await dependencies.choosePackageManager(); - if (ruleConversionResults.converted.size !== 0) { logSuccessfulConversions("rule", ruleConversionResults.converted, dependencies.logger); logNotices(ruleConversionResults.converted, dependencies.logger); @@ -48,8 +42,6 @@ export const reportConversionResults = async ( ); } - logMissingPackages(ruleConversionResults, packageManager, dependencies.logger); - if (!ruleConversionResults.extends?.includes("eslint-config-prettier")) { logPrettierExtension(dependencies.logger); } diff --git a/src/reporting/reportOutputs.test.ts b/src/reporting/reportOutputs.test.ts deleted file mode 100644 index 4e7f01ce5..000000000 --- a/src/reporting/reportOutputs.test.ts +++ /dev/null @@ -1,88 +0,0 @@ -import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs"; -import { createEmptyConversionResults } from "../conversion/conversionResults.stubs"; -import { PackageManager } from "./packages/packageManagers"; -import { logMissingPackages } from "./reportOutputs"; -import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules"; - -const createStubDependencies = ( - packageManager: PackageManager, - results?: Partial, -) => ({ - logger: createStubLogger(), - packageManager, - ruleConversionResults: createEmptyConversionResults(results), -}); - -describe("reportOutputs", () => { - it("reports an npm command when the package manager is npm", () => { - // Arrange - createEmptyConversionResults(); - const { logger, packageManager, ruleConversionResults } = createStubDependencies( - PackageManager.npm, - ); - - // Act - logMissingPackages(ruleConversionResults, packageManager, logger); - - // Assert - expectEqualWrites( - logger.stdout.write, - `⚡ 3 packages are required for this ESLint configuration. ⚡`, - ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, - ); - }); - - it("reports a pnpm command when the package manager is pnpm", () => { - // Arrange - const { logger, packageManager, ruleConversionResults } = createStubDependencies( - PackageManager.pnpm, - ); - - // Act - logMissingPackages(ruleConversionResults, packageManager, logger); - - // Assert - expectEqualWrites( - logger.stdout.write, - `⚡ 3 packages are required for this ESLint configuration. ⚡`, - ` pnpm add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, - ); - }); - - it("reports a Yarn command when the package manager is Yarn", () => { - // Arrange - const { logger, packageManager, ruleConversionResults } = createStubDependencies( - PackageManager.Yarn, - ); - - // Act - logMissingPackages(ruleConversionResults, packageManager, logger); - - // Assert - expectEqualWrites( - logger.stdout.write, - `⚡ 3 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --dev`, - ); - }); - - it("adds extensions to the missing packages list when they exist", () => { - // Arrange - const { logger, packageManager, ruleConversionResults } = createStubDependencies( - PackageManager.Yarn, - { - extends: ["eslint-config-prettier", "eslint-config-prettier/@typescript-eslint"], - }, - ); - - // Act - logMissingPackages(ruleConversionResults, packageManager, logger); - - // Assert - expectEqualWrites( - logger.stdout.write, - `⚡ 4 packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, - ); - }); -}); diff --git a/src/reporting/reportOutputs.ts b/src/reporting/reportOutputs.ts index 138b23273..113add239 100644 --- a/src/reporting/reportOutputs.ts +++ b/src/reporting/reportOutputs.ts @@ -2,12 +2,9 @@ import chalk from "chalk"; import { EOL } from "os"; import { Logger } from "../adapters/logger"; -import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules"; import { EditorSetting } from "../editorSettings/types"; import { ErrorSummary } from "../errors/errorSummary"; import { ESLintRuleOptions } from "../rules/types"; -import { uniqueFromSources } from "../utils"; -import { PackageManager, installationMessages } from "./packages/packageManagers"; export type EditorSettingEntry = Pick; @@ -69,29 +66,3 @@ export const logMissingConversionTarget = ( ); logger.info.write(EOL); }; - -export const logMissingPackages = ( - ruleConversionResults: Pick, - packageManager: PackageManager, - logger: Logger, -) => { - const packageNames = uniqueFromSources([ - "@typescript-eslint/eslint-plugin", - "@typescript-eslint/parser", - ruleConversionResults.missing.length !== 0 && "@typescript-eslint/eslint-plugin-tslint", - "eslint", - ...Array.from( - ruleConversionResults.extends?.map((extension) => extension.split("/")[0]) ?? [], - ), - ...Array.from(ruleConversionResults.plugins), - ]) - .filter(Boolean) - .sort(); - - logger.stdout.write(chalk.cyanBright(`${EOL}⚡ ${packageNames.length}`)); - logger.stdout.write(chalk.cyan(" packages are required for this ESLint configuration.")); - logger.stdout.write(chalk.cyanBright(" ⚡")); - logger.stdout.write(`${EOL} `); - logger.stdout.write(chalk.cyan(installationMessages[packageManager](packageNames.join(" ")))); - logger.stdout.write(EOL); -}; diff --git a/src/utils.ts b/src/utils.ts index c69d6f9df..29c8d5e70 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,9 @@ -export const isDefined = (item: Item | undefined): item is Item => !!item; +export const isDefined = (item: Item | undefined): item is Item => item !== undefined; export const isError = (item: Item | Error): item is Error => item instanceof Error; +export const isTruthy = (item: Item | false | undefined | null | 0): item is Item => !!item; + export type RemoveErrors = { [P in keyof Items]: Exclude; }; From aee767e1d96fe3895c326575b186ee1528446886 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 26 Apr 2020 22:53:44 -0400 Subject: [PATCH 2/3] Fixed up using extends --- .../packages/getPackageNameFromExtends.ts | 11 -- .../packages/logMissingPackages.test.ts | 139 ++++++++++++++---- src/reporting/packages/logMissingPackages.ts | 16 +- src/reporting/reportConversionResults.test.ts | 43 +++--- src/reporting/reportConversionResults.ts | 2 +- 5 files changed, 142 insertions(+), 69 deletions(-) delete mode 100644 src/reporting/packages/getPackageNameFromExtends.ts diff --git a/src/reporting/packages/getPackageNameFromExtends.ts b/src/reporting/packages/getPackageNameFromExtends.ts deleted file mode 100644 index 3bc69b61b..000000000 --- a/src/reporting/packages/getPackageNameFromExtends.ts +++ /dev/null @@ -1,11 +0,0 @@ -export const getPackageNameFromExtends = (name: string) => { - if (name.startsWith("plugin:")) { - name = name.slice("plugin:".length); - } - - // This heuristic actually doesn't work :( - return name - .split("/") - .slice(0, name.startsWith("@") ? 2 : 1) - .join("/"); -}; diff --git a/src/reporting/packages/logMissingPackages.test.ts b/src/reporting/packages/logMissingPackages.test.ts index eef4a8004..5a7474086 100644 --- a/src/reporting/packages/logMissingPackages.test.ts +++ b/src/reporting/packages/logMissingPackages.test.ts @@ -5,25 +5,43 @@ import { createStubLogger, expectEqualWrites } from "../../adapters/logger.stubs"; import { createEmptyConversionResults } from "../../conversion/conversionResults.stubs"; -import { SimplifiedResultsConfiguration } from "../../creation/simplification/simplifyPackageRules"; import { logMissingPackages } from "./logMissingPackages"; import { PackageManager } from "./packageManagers"; -const createStubDependencies = ( - packageManager: PackageManager, - results?: Partial, -) => ({ +const createStubDependencies = (packageManager = PackageManager.npm) => ({ choosePackageManager: async () => packageManager, logger: createStubLogger(), - ruleConversionResults: createEmptyConversionResults(results), }); describe("logMissingPackages", () => { - it("reports an npm command when the package manager is npm", async () => { + it("reports a singular message when one package is missing", async () => { // Arrange - const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( - PackageManager.npm, + const { choosePackageManager, logger } = createStubDependencies(PackageManager.npm); + const ruleConversionResults = createEmptyConversionResults(); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults, { + dependencies: { + "@typescript-eslint/eslint-plugin": "*", + "@typescript-eslint/parser": "*", + }, + devDependencies: {}, + }); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 1 new package is required for this ESLint configuration. ⚡`, + ` npm install eslint --save-dev`, ); + }); + + it("does not include eslint-config-prettier when there are no extensions", async () => { + // Arrange + const { choosePackageManager, logger } = createStubDependencies(); + const ruleConversionResults = createEmptyConversionResults({ + extends: undefined, + }); // Act await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); @@ -36,11 +54,86 @@ describe("logMissingPackages", () => { ); }); - it("reports a pnpm command when the package manager is pnpm", async () => { + it("does not include eslint-config-prettier when extensions don't include eslint-config-prettier", async () => { + // Arrange + const { choosePackageManager, logger } = createStubDependencies(); + const ruleConversionResults = createEmptyConversionResults({ + extends: [], + }); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, + ); + }); + + it("includes eslint-config-prettier when extensions include eslint-config-prettier", async () => { + // Arrange + const { choosePackageManager, logger } = createStubDependencies(); + const ruleConversionResults = createEmptyConversionResults({ + extends: ["plugin:eslint-config-prettier"], + }); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 4 new packages are required for this ESLint configuration. ⚡`, + ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --save-dev`, + ); + }); + + it("includes @typescript-eslint/eslint-plugin-tslint when there are missing conversions", async () => { // Arrange - const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( - PackageManager.pnpm, + const { choosePackageManager, logger } = createStubDependencies(); + const ruleConversionResults = createEmptyConversionResults({ + missing: [ + { + ruleArguments: [], + ruleName: "missing-rule", + ruleSeverity: "error", + }, + ], + }); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 4 new packages are required for this ESLint configuration. ⚡`, + ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/eslint-plugin-tslint @typescript-eslint/parser eslint --save-dev`, ); + }); + + it("reports an npm command when the package manager is npm", async () => { + // Arrange + const { choosePackageManager, logger } = createStubDependencies(PackageManager.npm); + const ruleConversionResults = createEmptyConversionResults(); + + // Act + await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` npm install @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --save-dev`, + ); + }); + + it("reports a pnpm command when the package manager is pnpm", async () => { + // Arrange + const { choosePackageManager, logger } = createStubDependencies(PackageManager.pnpm); + const ruleConversionResults = createEmptyConversionResults(); // Act await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); @@ -55,9 +148,8 @@ describe("logMissingPackages", () => { it("reports a Yarn command when the package manager is Yarn", async () => { // Arrange - const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( - PackageManager.Yarn, - ); + const { choosePackageManager, logger } = createStubDependencies(PackageManager.Yarn); + const ruleConversionResults = createEmptyConversionResults(); // Act await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); @@ -70,17 +162,10 @@ describe("logMissingPackages", () => { ); }); - it("adds extensions to the missing packages list when they exist", async () => { + it("reports a Yarn command when the package manager is Yarn", async () => { // Arrange - const { choosePackageManager, logger, ruleConversionResults } = createStubDependencies( - PackageManager.Yarn, - { - extends: [ - "plugin:eslint-config-prettier", - "plugin:eslint-config-prettier/@typescript-eslint", - ], - }, - ); + const { choosePackageManager, logger } = createStubDependencies(PackageManager.Yarn); + const ruleConversionResults = createEmptyConversionResults(); // Act await logMissingPackages({ choosePackageManager, logger }, ruleConversionResults); @@ -88,8 +173,8 @@ describe("logMissingPackages", () => { // Assert expectEqualWrites( logger.stdout.write, - `⚡ 4 new packages are required for this ESLint configuration. ⚡`, - ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier --dev`, + `⚡ 3 new packages are required for this ESLint configuration. ⚡`, + ` yarn add @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint --dev`, ); }); }); diff --git a/src/reporting/packages/logMissingPackages.ts b/src/reporting/packages/logMissingPackages.ts index 337ce8290..fc7a9e5e5 100644 --- a/src/reporting/packages/logMissingPackages.ts +++ b/src/reporting/packages/logMissingPackages.ts @@ -8,7 +8,6 @@ import { PackagesConfiguration } from "../../input/findPackagesConfiguration"; import { isTruthy } from "../../utils"; import { installationMessages } from "../packages/packageManagers"; import { choosePackageManager } from "./choosePackageManager"; -import { getPackageNameFromExtends } from "./getPackageNameFromExtends"; export type LogMissingPackagesDependencies = { choosePackageManager: SansDependencies; @@ -30,19 +29,24 @@ export const logMissingPackages = async ( const requiredPackageNames = [ "@typescript-eslint/eslint-plugin", "@typescript-eslint/parser", + ruleConversionResults.extends?.join("").includes("eslint-config-prettier") && + "eslint-config-prettier", ruleConversionResults.missing.length !== 0 && "@typescript-eslint/eslint-plugin-tslint", "eslint", - ...Array.from(ruleConversionResults.extends?.map(getPackageNameFromExtends) ?? []), ...Array.from(ruleConversionResults.plugins), ].filter(isTruthy); - const missingPackageNames = requiredPackageNames.filter( - (packageName) => !existingPackageNames.has(packageName), - ); + const missingPackageNames = requiredPackageNames + .filter((packageName) => !existingPackageNames.has(packageName)) + .sort(); dependencies.logger.stdout.write(chalk.cyanBright(`${EOL}⚡ ${missingPackageNames.length}`)); dependencies.logger.stdout.write( - chalk.cyan(" new packages are required for this ESLint configuration."), + chalk.cyan( + ` new package${ + missingPackageNames.length === 1 ? " is" : "s are" + } required for this ESLint configuration.`, + ), ); dependencies.logger.stdout.write(chalk.cyanBright(" ⚡")); dependencies.logger.stdout.write(`${EOL} `); diff --git a/src/reporting/reportConversionResults.test.ts b/src/reporting/reportConversionResults.test.ts index c5352e577..4da981b76 100644 --- a/src/reporting/reportConversionResults.test.ts +++ b/src/reporting/reportConversionResults.test.ts @@ -13,6 +13,7 @@ const basicExtends = [ describe("reportConversionResults", () => { it("logs a successful conversion without notices when there is one converted rule without notices", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ converted: new Map([ [ @@ -27,8 +28,6 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -38,6 +37,7 @@ describe("reportConversionResults", () => { it("logs a successful conversion with notices when there is one converted rule with notices", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ converted: new Map([ [ @@ -53,8 +53,6 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -76,6 +74,7 @@ describe("reportConversionResults", () => { it("logs successful conversions when there are multiple converted rules", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ converted: new Map([ [ @@ -100,8 +99,6 @@ describe("reportConversionResults", () => { extends: basicExtends, }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -127,13 +124,12 @@ describe("reportConversionResults", () => { it("logs a failed conversion when there is one failed conversion", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ failed: [{ getSummary: () => "It broke." }], extends: basicExtends, }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -147,13 +143,12 @@ describe("reportConversionResults", () => { it("logs failed conversions when there are multiple failed conversions", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ extends: basicExtends, failed: [{ getSummary: () => "It broke." }, { getSummary: () => "It really broke." }], }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -167,6 +162,7 @@ describe("reportConversionResults", () => { it("logs a missing rule when there is a missing rule", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ extends: basicExtends, missing: [ @@ -178,8 +174,6 @@ describe("reportConversionResults", () => { ], }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -199,6 +193,7 @@ describe("reportConversionResults", () => { it("logs missing rules when there are missing rules", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ extends: basicExtends, missing: [ @@ -215,8 +210,6 @@ describe("reportConversionResults", () => { ], }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); @@ -235,36 +228,38 @@ describe("reportConversionResults", () => { ); }); - it("logs missing plugins when there are missing plugins", async () => { + it("logs a Prettier recommendation when there are no extensions", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ - extends: basicExtends, - plugins: new Set(["plugin-one", "plugin-two"]), + extends: undefined, }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert - expectEqualWrites(); + expectEqualWrites( + logger.stdout.write, + `☠ Prettier plugins are missing from your configuration. ☠`, + ` We highly recommend running tslint-to-eslint-config --prettier to disable formatting ESLint rules.`, + ` See https://github/typescript-eslint/tslint-to-eslint-config/docs/FAQs.md#should-i-use-prettier.`, + ); }); - it("logs a Prettier recommendation when eslint-config-prettier isn't extended", async () => { + it("logs a Prettier recommendation when extends don't include eslint-config-prettier", async () => { // Arrange + const logger = createStubLogger(); const conversionResults = createEmptyConversionResults({ extends: [], }); - const logger = createStubLogger(); - // Act await reportConversionResults({ logger }, ".eslintrc.js", conversionResults); // Assert expectEqualWrites( - ``, + logger.stdout.write, `☠ Prettier plugins are missing from your configuration. ☠`, ` We highly recommend running tslint-to-eslint-config --prettier to disable formatting ESLint rules.`, ` See https://github/typescript-eslint/tslint-to-eslint-config/docs/FAQs.md#should-i-use-prettier.`, diff --git a/src/reporting/reportConversionResults.ts b/src/reporting/reportConversionResults.ts index df9e32378..7f63938e7 100644 --- a/src/reporting/reportConversionResults.ts +++ b/src/reporting/reportConversionResults.ts @@ -42,7 +42,7 @@ export const reportConversionResults = async ( ); } - if (!ruleConversionResults.extends?.includes("eslint-config-prettier")) { + if (!ruleConversionResults.extends?.join("").includes("eslint-config-prettier")) { logPrettierExtension(dependencies.logger); } }; From bc0aac36636eb61757d068a8bab98bdd534d45f7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 26 Apr 2020 23:00:18 -0400 Subject: [PATCH 3/3] Removed commented out imports --- src/reporting/packages/logMissingPackages.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/reporting/packages/logMissingPackages.test.ts b/src/reporting/packages/logMissingPackages.test.ts index 5a7474086..ac1a6fb72 100644 --- a/src/reporting/packages/logMissingPackages.test.ts +++ b/src/reporting/packages/logMissingPackages.test.ts @@ -1,8 +1,3 @@ -// import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs"; -// import { createEmptyConversionResults } from "../conversion/conversionResults.stubs"; -// import { PackageManager } from "./packages/packageManagers"; -// import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules"; - import { createStubLogger, expectEqualWrites } from "../../adapters/logger.stubs"; import { createEmptyConversionResults } from "../../conversion/conversionResults.stubs"; import { logMissingPackages } from "./logMissingPackages";