From 5de07d2bca3becf5a03b6b8001508778a9890e0b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 16:04:00 -0400 Subject: [PATCH 1/8] Moved file-related functions from runner.ts to src/files/ or src/utils.ts src/runner.ts is getting pretty overcrowded; it was getting hard to reason about where things belonged in it. --- src/files/finding.ts | 67 ++++++++++++++++++++ src/files/program.ts | 67 ++++++++++++++++++++ src/files/reading.ts | 43 +++++++++++++ src/runner.ts | 142 +++++++++---------------------------------- src/utils.ts | 4 ++ 5 files changed, 211 insertions(+), 112 deletions(-) create mode 100644 src/files/finding.ts create mode 100644 src/files/program.ts create mode 100644 src/files/reading.ts diff --git a/src/files/finding.ts b/src/files/finding.ts new file mode 100644 index 00000000000..4dd1bf49ffe --- /dev/null +++ b/src/files/finding.ts @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2019 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as fs from "fs"; +import * as glob from "glob"; +import { filter as createMinimatchFilter, Minimatch } from "minimatch"; +import * as path from "path"; + +import { flatMap, trimSingleQuotes } from "../utils"; +import { Logger } from '../runner'; + +export function filterFiles(files: string[], patterns: string[], include: boolean): string[] { + if (patterns.length === 0) { + return include ? [] : files; + } + const matcher = patterns.map(pattern => new Minimatch(pattern, { dot: !include })); // `glob` always enables `dot` for ignore patterns + return files.filter(file => include === matcher.some(pattern => pattern.match(file))); +} + +export function findTsconfig(project: string): string | undefined { + try { + const stats = fs.statSync(project); // throws if file does not exist + if (!stats.isDirectory()) { + return project; + } + const projectFile = path.join(project, "tsconfig.json"); + fs.accessSync(projectFile); // throws if file does not exist + return projectFile; + } catch (e) { + return undefined; + } +} + +export function resolveGlobs( + files: string[], + ignore: string[], + outputAbsolutePaths: boolean | undefined, + logger: Logger, +): string[] { + const results = flatMap(files, file => + glob.sync(trimSingleQuotes(file), { ignore, nodir: true }), + ); + // warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns + for (const file of filterFiles(files, ignore, false)) { + if (!glob.hasMagic(file) && !results.some(createMinimatchFilter(file))) { + logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 + } + } + const cwd = process.cwd(); + return results.map(file => + outputAbsolutePaths ? path.resolve(cwd, file) : path.relative(cwd, file), + ); +} diff --git a/src/files/program.ts b/src/files/program.ts new file mode 100644 index 00000000000..464479ae21b --- /dev/null +++ b/src/files/program.ts @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2019 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as fs from "fs"; +import * as glob from "glob"; +import { filter as createMinimatchFilter } from "minimatch"; +import * as path from "path"; +import * as ts from "typescript"; + +import { Linter } from "../linter"; +import { FatalError } from "../error"; +import { Options, Logger } from "../runner"; +import { trimSingleQuotes } from "../utils"; +import { resolveGlobs, findTsconfig, filterFiles } from "./finding"; + +export function resolveFilesAndProgram( + { files, project, exclude, outputAbsolutePaths }: Options, + logger: Logger, +): { files: string[]; program?: ts.Program } { + // remove single quotes which break matching on Windows when glob is passed in single quotes + exclude = exclude.map(trimSingleQuotes); + + if (project === undefined) { + return { files: resolveGlobs(files, exclude, outputAbsolutePaths, logger) }; + } + + const projectPath = findTsconfig(project); + if (projectPath === undefined) { + throw new FatalError(`Invalid option for project: ${project}`); + } + + exclude = exclude.map(pattern => path.resolve(pattern)); + const program = Linter.createProgram(projectPath); + let filesFound: string[]; + if (files.length === 0) { + filesFound = filterFiles(Linter.getFileNames(program), exclude, false); + } else { + files = files.map(f => path.resolve(f)); + filesFound = filterFiles(program.getSourceFiles().map(f => f.fileName), files, true); + filesFound = filterFiles(filesFound, exclude, false); + + // find non-glob files that have no matching file in the project and are not excluded by any exclude pattern + for (const file of filterFiles(files, exclude, false)) { + if (!glob.hasMagic(file) && !filesFound.some(createMinimatchFilter(file))) { + if (fs.existsSync(file)) { + throw new FatalError(`'${file}' is not included in project.`); + } + logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 + } + } + } + return { files: filesFound, program }; +} diff --git a/src/files/reading.ts b/src/files/reading.ts new file mode 100644 index 00000000000..88b17a1888f --- /dev/null +++ b/src/files/reading.ts @@ -0,0 +1,43 @@ +/** + * @license + * Copyright 2019 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as fs from "fs"; +import { Logger } from "../runner"; +import { FatalError } from "../error"; + +/** Read a file, but return undefined if it is an MPEG '.ts' file. */ +export async function tryReadFile(filename: string, logger: Logger): Promise { + if (!fs.existsSync(filename)) { + throw new FatalError(`Unable to open file: ${filename}`); + } + const buffer = Buffer.allocUnsafe(256); + const fd = fs.openSync(filename, "r"); + try { + fs.readSync(fd, buffer, 0, 256, 0); + if (buffer.readInt8(0) === 0x47 && buffer.readInt8(188) === 0x47) { + // MPEG transport streams use the '.ts' file extension. They use 0x47 as the frame + // separator, repeating every 188 bytes. It is unlikely to find that pattern in + // TypeScript source, so tslint ignores files with the specific pattern. + logger.error(`${filename}: ignoring MPEG transport stream\n`); + return undefined; + } + } finally { + fs.closeSync(fd); + } + + return fs.readFileSync(filename, "utf8"); +} diff --git a/src/runner.ts b/src/runner.ts index 2d4f3505d3c..cbd14eefbc5 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -18,8 +18,6 @@ // tslint:disable strict-boolean-expressions (TODO: Fix up options) import * as fs from "fs"; -import * as glob from "glob"; -import { filter as createMinimatchFilter, Minimatch } from "minimatch"; import * as path from "path"; import * as ts from "typescript"; @@ -30,9 +28,11 @@ import { JSON_CONFIG_FILENAME, } from "./configuration"; import { FatalError } from "./error"; +import { resolveFilesAndProgram } from './files/program'; +import { tryReadFile } from './files/reading'; import { LintResult } from "./index"; import { Linter } from "./linter"; -import { flatMap } from "./utils"; +import { trimSingleQuotes } from './utils'; export interface Options { /** @@ -85,6 +85,11 @@ export interface Options { */ outputAbsolutePaths?: boolean; + /** + * Outputs the configuration to be used instead of linting. + */ + printConfig?: string; + /** * tsconfig.json file. */ @@ -145,6 +150,10 @@ async function runWorker(options: Options, logger: Logger): Promise { return Status.Ok; } + if (options.printConfig) { + return runConfigPrinting(options, logger); + } + if (options.test) { const test = await import("./test"); const results = test.runTests( @@ -165,6 +174,24 @@ async function runWorker(options: Options, logger: Logger): Promise { return options.force || errorCount === 0 ? Status.Ok : Status.LintError; } +async function runConfigPrinting(options: Options, logger: Logger): Promise { + const { config, files } = options; + if (files.length !== 1) { + throw new FatalError(`--print-config must be run with exactly one file`); + } + if (config === undefined) { + throw new FatalError(`--print-config must be run with exactly one file`); + } + + const configuration = findConfiguration(config, files[1]).results; + if (configuration === undefined) { + throw new FatalError(`Could not find configuration for '${files[1]}`) + } + + logger.log(`${JSON.stringify(configuration, undefined, 4)}\n`); + return Status.Ok; +} + async function runLinter(options: Options, logger: Logger): Promise { const { files, program } = resolveFilesAndProgram(options, logger); // if type checking, run the type checker @@ -184,74 +211,6 @@ async function runLinter(options: Options, logger: Logger): Promise return doLinting(options, files, program, logger); } -function resolveFilesAndProgram( - { files, project, exclude, outputAbsolutePaths }: Options, - logger: Logger, -): { files: string[]; program?: ts.Program } { - // remove single quotes which break matching on Windows when glob is passed in single quotes - exclude = exclude.map(trimSingleQuotes); - - if (project === undefined) { - return { files: resolveGlobs(files, exclude, outputAbsolutePaths, logger) }; - } - - const projectPath = findTsconfig(project); - if (projectPath === undefined) { - throw new FatalError(`Invalid option for project: ${project}`); - } - - exclude = exclude.map(pattern => path.resolve(pattern)); - const program = Linter.createProgram(projectPath); - let filesFound: string[]; - if (files.length === 0) { - filesFound = filterFiles(Linter.getFileNames(program), exclude, false); - } else { - files = files.map(f => path.resolve(f)); - filesFound = filterFiles(program.getSourceFiles().map(f => f.fileName), files, true); - filesFound = filterFiles(filesFound, exclude, false); - - // find non-glob files that have no matching file in the project and are not excluded by any exclude pattern - for (const file of filterFiles(files, exclude, false)) { - if (!glob.hasMagic(file) && !filesFound.some(createMinimatchFilter(file))) { - if (fs.existsSync(file)) { - throw new FatalError(`'${file}' is not included in project.`); - } - logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 - } - } - } - return { files: filesFound, program }; -} - -function filterFiles(files: string[], patterns: string[], include: boolean): string[] { - if (patterns.length === 0) { - return include ? [] : files; - } - const matcher = patterns.map(pattern => new Minimatch(pattern, { dot: !include })); // `glob` always enables `dot` for ignore patterns - return files.filter(file => include === matcher.some(pattern => pattern.match(file))); -} - -function resolveGlobs( - files: string[], - ignore: string[], - outputAbsolutePaths: boolean | undefined, - logger: Logger, -): string[] { - const results = flatMap(files, file => - glob.sync(trimSingleQuotes(file), { ignore, nodir: true }), - ); - // warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns - for (const file of filterFiles(files, ignore, false)) { - if (!glob.hasMagic(file) && !results.some(createMinimatchFilter(file))) { - logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 - } - } - const cwd = process.cwd(); - return results.map(file => - outputAbsolutePaths ? path.resolve(cwd, file) : path.relative(cwd, file), - ); -} - async function doLinting( options: Options, files: string[], @@ -312,29 +271,6 @@ async function doLinting( return linter.getResult(); } -/** Read a file, but return undefined if it is an MPEG '.ts' file. */ -async function tryReadFile(filename: string, logger: Logger): Promise { - if (!fs.existsSync(filename)) { - throw new FatalError(`Unable to open file: ${filename}`); - } - const buffer = Buffer.allocUnsafe(256); - const fd = fs.openSync(filename, "r"); - try { - fs.readSync(fd, buffer, 0, 256, 0); - if (buffer.readInt8(0) === 0x47 && buffer.readInt8(188) === 0x47) { - // MPEG transport streams use the '.ts' file extension. They use 0x47 as the frame - // separator, repeating every 188 bytes. It is unlikely to find that pattern in - // TypeScript source, so tslint ignores files with the specific pattern. - logger.error(`${filename}: ignoring MPEG transport stream\n`); - return undefined; - } - } finally { - fs.closeSync(fd); - } - - return fs.readFileSync(filename, "utf8"); -} - function showDiagnostic( { file, start, category, messageText }: ts.Diagnostic, program: ts.Program, @@ -351,21 +287,3 @@ function showDiagnostic( } return `${message} ${ts.flattenDiagnosticMessageText(messageText, "\n")}`; } - -function trimSingleQuotes(str: string): string { - return str.replace(/^'|'$/g, ""); -} - -function findTsconfig(project: string): string | undefined { - try { - const stats = fs.statSync(project); // throws if file does not exist - if (!stats.isDirectory()) { - return project; - } - const projectFile = path.join(project, "tsconfig.json"); - fs.accessSync(projectFile); // throws if file does not exist - return projectFile; - } catch (e) { - return undefined; - } -} diff --git a/src/utils.ts b/src/utils.ts index 4ca158a47b1..b309c0fe869 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -235,6 +235,10 @@ export function detectBufferEncoding(buffer: Buffer, length = buffer.length): En return "utf8"; } +export function trimSingleQuotes(str: string): string { + return str.replace(/^'|'$/g, ""); +} + // converts Windows normalized paths (with backwards slash `\`) to paths used by TypeScript (with forward slash `/`) export function denormalizeWinPath(path: string): string { return path.replace(/\\/g, "/"); From eabf53864e6bef3eb03160370afb22f165edee3e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 16:04:39 -0400 Subject: [PATCH 2/8] Added --print-config option Relies on some slightly manual stringification of members of the config files. --- src/configuration.ts | 22 ++++++++++++++++++++++ src/runner.ts | 18 ++++++++++++------ src/tslintCli.ts | 10 +++++++++- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 712f8cc55eb..310754d69a9 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -660,3 +660,25 @@ export function isFileExcluded(filepath: string, configFile?: IConfigurationFile const fullPath = path.resolve(filepath); return configFile.linterOptions.exclude.some(pattern => new Minimatch(pattern).match(fullPath)); } + +export function stringifyConfiguration(configFile: IConfigurationFile) { + return JSON.stringify( + { + defaultSeverity: configFile.defaultSeverity, + extends: configFile.extends, + jsRules: convertRulesMapToObject(configFile.jsRules), + rules: convertRulesMapToObject(configFile.rules), + linterOptions: configFile.linterOptions, + rulesDirectory: configFile.rulesDirectory, + }, + undefined, + 4, + ); +} + +function convertRulesMapToObject(rules: Map>) { + return Array.from(rules).reduce( + (result, [key, value]) => Object.assign(result, { [key]: value }), + {}, + ); +} diff --git a/src/runner.ts b/src/runner.ts index cbd14eefbc5..b3e13945a8f 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -26,6 +26,8 @@ import { findConfiguration, isFileExcluded, JSON_CONFIG_FILENAME, + findConfigurationPath, + stringifyConfiguration, } from "./configuration"; import { FatalError } from "./error"; import { resolveFilesAndProgram } from './files/program'; @@ -88,7 +90,7 @@ export interface Options { /** * Outputs the configuration to be used instead of linting. */ - printConfig?: string; + printConfig?: boolean; /** * tsconfig.json file. @@ -175,20 +177,24 @@ async function runWorker(options: Options, logger: Logger): Promise { } async function runConfigPrinting(options: Options, logger: Logger): Promise { - const { config, files } = options; + const { files } = options; if (files.length !== 1) { throw new FatalError(`--print-config must be run with exactly one file`); } - if (config === undefined) { - throw new FatalError(`--print-config must be run with exactly one file`); + + const configurationPath = options.config === undefined + ? findConfigurationPath(null, files[0]) + : options.config; + if (configurationPath === undefined) { + throw new FatalError(`Could not find configuration path. Try passing a --config to your tslint.json.`); } - const configuration = findConfiguration(config, files[1]).results; + const configuration = findConfiguration(configurationPath, files[0]).results; if (configuration === undefined) { throw new FatalError(`Could not find configuration for '${files[1]}`) } - logger.log(`${JSON.stringify(configuration, undefined, 4)}\n`); + logger.log(`${stringifyConfiguration(configuration)}\n`); return Status.Ok; } diff --git a/src/tslintCli.ts b/src/tslintCli.ts index 9aaabf99e1c..fd71b02be9b 100644 --- a/src/tslintCli.ts +++ b/src/tslintCli.ts @@ -35,6 +35,7 @@ interface Argv { init?: boolean; out?: string; outputAbsolutePaths: boolean; + printConfig?: boolean; project?: string; rulesDir?: string; formattersDir: string; @@ -48,7 +49,7 @@ interface Argv { interface Option { short?: string; // Commander will camelCase option names. - name: keyof Argv | "rules-dir" | "formatters-dir" | "type-check"; + name: keyof Argv | "rules-dir" | "formatters-dir" | "print-config" | "type-check"; type: "string" | "boolean" | "array"; describe: string; // Short, used for usage message description: string; // Long, used for `--help` @@ -121,6 +122,12 @@ const options: Option[] = [ describe: "whether or not outputted file paths are absolute", description: "If true, all paths in the output will be absolute.", }, + { + name: "print-config", + type: "boolean", + describe: "idk", + description: "wat", + }, { short: "r", name: "rules-dir", @@ -296,6 +303,7 @@ run( init: argv.init, out: argv.out, outputAbsolutePaths: argv.outputAbsolutePaths, + printConfig: argv.printConfig, project: argv.project, quiet: argv.quiet, rulesDirectory: argv.rulesDir, From 23bce6c282179aecd7299e589cc8b9731ff0ff40 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 16:11:31 -0400 Subject: [PATCH 3/8] Added preliminary tests for exit code Haven't figured out yet how to test the real output :( --- test/executable/executableTests.ts | 29 +++++++++++++++++++++++++++++ test/files/print-config/a.ts | 1 + test/files/print-config/b.ts | 1 + 3 files changed, 31 insertions(+) create mode 100644 test/files/print-config/a.ts create mode 100644 test/files/print-config/b.ts diff --git a/test/executable/executableTests.ts b/test/executable/executableTests.ts index a0211ae4c3a..eb8dd66e601 100644 --- a/test/executable/executableTests.ts +++ b/test/executable/executableTests.ts @@ -435,6 +435,35 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) { }); }); + describe("--print-config flag", () => { + it("exits with code 1 if no files are provided", async () => { + const status = await execRunner({ + files: [], + printConfig: true, + }); + + assert.equal(status, Status.FatalError); + }); + + it("exits with code 0 if one file is provided", async () => { + const status = await execRunner({ + files: ["test/files/a.ts"], + printConfig: true, + }); + + assert.equal(status, Status.Ok); + }); + + it("exits with code 1 if multiple files are provided", async () => { + const status = await execRunner({ + files: ["test/files/a.ts", "test/files//b.ts"], + printConfig: true, + }); + + assert.equal(status, Status.FatalError); + }); + }); + describe("--project flag", () => { it("exits with code 0 if `tsconfig.json` is passed and it specifies files without errors", async () => { const status = await execRunner({ diff --git a/test/files/print-config/a.ts b/test/files/print-config/a.ts new file mode 100644 index 00000000000..7b2a3460115 --- /dev/null +++ b/test/files/print-config/a.ts @@ -0,0 +1 @@ +console.log("a"); diff --git a/test/files/print-config/b.ts b/test/files/print-config/b.ts new file mode 100644 index 00000000000..6d012e7f1f1 --- /dev/null +++ b/test/files/print-config/b.ts @@ -0,0 +1 @@ +console.log("b"); From 35d142d7bb9ac80c8e62133c54511a417b86a62b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 16:18:41 -0400 Subject: [PATCH 4/8] Updated docs --- docs/usage/cli/index.md | 1 + src/tslintCli.ts | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/usage/cli/index.md b/docs/usage/cli/index.md index d45089cb154..daf9304e0c6 100644 --- a/docs/usage/cli/index.md +++ b/docs/usage/cli/index.md @@ -41,6 +41,7 @@ Options: -i, --init generate a tslint.json config file in the current working directory -o, --out [out] output file --outputAbsolutePaths whether or not outputted file paths are absolute +--print-config print resolved configuration for a file -r, --rules-dir [rules-dir] rules directory -s, --formatters-dir [formatters-dir] formatters directory -t, --format [format] output format (prose, json, stylish, verbose, pmd, msbuild, checkstyle, vso, fileslist, codeFrame) diff --git a/src/tslintCli.ts b/src/tslintCli.ts index fd71b02be9b..0f6c52a36e1 100644 --- a/src/tslintCli.ts +++ b/src/tslintCli.ts @@ -125,8 +125,11 @@ const options: Option[] = [ { name: "print-config", type: "boolean", - describe: "idk", - description: "wat", + describe: "print resolved configuration for a file", + description: dedent` + When passed a single file name, prints the configuration that would + be used to lint that file. + No linting is performed and only config-related options are valid.`, }, { short: "r", From 40404cca101ee9b4c3d3b273a7fbeb4e31abbf06 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 16:22:01 -0400 Subject: [PATCH 5/8] Switched config stringification indentation to 2 Matches ESLint's. --- src/configuration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/configuration.ts b/src/configuration.ts index 310754d69a9..264692516de 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -672,7 +672,7 @@ export function stringifyConfiguration(configFile: IConfigurationFile) { rulesDirectory: configFile.rulesDirectory, }, undefined, - 4, + 2, ); } From 472c23fabcd9df4da830dd090c5c07fee54202ca Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 May 2019 17:16:38 -0400 Subject: [PATCH 6/8] Fixed lint complaints --- src/configuration.ts | 7 +++---- src/files/finding.ts | 2 +- src/files/program.ts | 7 ++++--- src/files/reading.ts | 3 ++- src/runner.ts | 21 +++++++++++---------- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 310754d69a9..00c0f6e5675 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -664,11 +664,10 @@ export function isFileExcluded(filepath: string, configFile?: IConfigurationFile export function stringifyConfiguration(configFile: IConfigurationFile) { return JSON.stringify( { - defaultSeverity: configFile.defaultSeverity, extends: configFile.extends, jsRules: convertRulesMapToObject(configFile.jsRules), - rules: convertRulesMapToObject(configFile.rules), linterOptions: configFile.linterOptions, + rules: convertRulesMapToObject(configFile.rules), rulesDirectory: configFile.rulesDirectory, }, undefined, @@ -677,8 +676,8 @@ export function stringifyConfiguration(configFile: IConfigurationFile) { } function convertRulesMapToObject(rules: Map>) { - return Array.from(rules).reduce( - (result, [key, value]) => Object.assign(result, { [key]: value }), + return Array.from(rules).reduce<{ [i: string]: Partial }>( + (result, [key, value]) => ({ ...result, [key]: value }), {}, ); } diff --git a/src/files/finding.ts b/src/files/finding.ts index 4dd1bf49ffe..6e40866f617 100644 --- a/src/files/finding.ts +++ b/src/files/finding.ts @@ -20,8 +20,8 @@ import * as glob from "glob"; import { filter as createMinimatchFilter, Minimatch } from "minimatch"; import * as path from "path"; +import { Logger } from "../runner"; import { flatMap, trimSingleQuotes } from "../utils"; -import { Logger } from '../runner'; export function filterFiles(files: string[], patterns: string[], include: boolean): string[] { if (patterns.length === 0) { diff --git a/src/files/program.ts b/src/files/program.ts index 464479ae21b..dcc4249685a 100644 --- a/src/files/program.ts +++ b/src/files/program.ts @@ -21,11 +21,12 @@ import { filter as createMinimatchFilter } from "minimatch"; import * as path from "path"; import * as ts from "typescript"; -import { Linter } from "../linter"; import { FatalError } from "../error"; -import { Options, Logger } from "../runner"; +import { Linter } from "../linter"; +import { Logger, Options } from "../runner"; import { trimSingleQuotes } from "../utils"; -import { resolveGlobs, findTsconfig, filterFiles } from "./finding"; + +import { filterFiles, findTsconfig, resolveGlobs } from "./finding"; export function resolveFilesAndProgram( { files, project, exclude, outputAbsolutePaths }: Options, diff --git a/src/files/reading.ts b/src/files/reading.ts index 88b17a1888f..a91bc23ca1a 100644 --- a/src/files/reading.ts +++ b/src/files/reading.ts @@ -16,8 +16,9 @@ */ import * as fs from "fs"; -import { Logger } from "../runner"; + import { FatalError } from "../error"; +import { Logger } from "../runner"; /** Read a file, but return undefined if it is an MPEG '.ts' file. */ export async function tryReadFile(filename: string, logger: Logger): Promise { diff --git a/src/runner.ts b/src/runner.ts index b3e13945a8f..951fd307216 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -24,17 +24,17 @@ import * as ts from "typescript"; import { DEFAULT_CONFIG, findConfiguration, + findConfigurationPath, isFileExcluded, JSON_CONFIG_FILENAME, - findConfigurationPath, stringifyConfiguration, } from "./configuration"; import { FatalError } from "./error"; -import { resolveFilesAndProgram } from './files/program'; -import { tryReadFile } from './files/reading'; +import { resolveFilesAndProgram } from "./files/program"; +import { tryReadFile } from "./files/reading"; import { LintResult } from "./index"; import { Linter } from "./linter"; -import { trimSingleQuotes } from './utils'; +import { trimSingleQuotes } from "./utils"; export interface Options { /** @@ -181,17 +181,18 @@ async function runConfigPrinting(options: Options, logger: Logger): Promise Date: Sat, 1 Jun 2019 08:30:18 -0400 Subject: [PATCH 7/8] Review feedback: merge files; add unit tests; rename run file --- src/files/finding.ts | 67 ----------- src/files/{program.ts => resolution.ts} | 47 +++++++- src/runner.ts | 6 +- test/configurationTests.ts | 153 ++++++++++++++++++++++++ 4 files changed, 200 insertions(+), 73 deletions(-) delete mode 100644 src/files/finding.ts rename src/files/{program.ts => resolution.ts} (58%) diff --git a/src/files/finding.ts b/src/files/finding.ts deleted file mode 100644 index 6e40866f617..00000000000 --- a/src/files/finding.ts +++ /dev/null @@ -1,67 +0,0 @@ -/** - * @license - * Copyright 2019 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as fs from "fs"; -import * as glob from "glob"; -import { filter as createMinimatchFilter, Minimatch } from "minimatch"; -import * as path from "path"; - -import { Logger } from "../runner"; -import { flatMap, trimSingleQuotes } from "../utils"; - -export function filterFiles(files: string[], patterns: string[], include: boolean): string[] { - if (patterns.length === 0) { - return include ? [] : files; - } - const matcher = patterns.map(pattern => new Minimatch(pattern, { dot: !include })); // `glob` always enables `dot` for ignore patterns - return files.filter(file => include === matcher.some(pattern => pattern.match(file))); -} - -export function findTsconfig(project: string): string | undefined { - try { - const stats = fs.statSync(project); // throws if file does not exist - if (!stats.isDirectory()) { - return project; - } - const projectFile = path.join(project, "tsconfig.json"); - fs.accessSync(projectFile); // throws if file does not exist - return projectFile; - } catch (e) { - return undefined; - } -} - -export function resolveGlobs( - files: string[], - ignore: string[], - outputAbsolutePaths: boolean | undefined, - logger: Logger, -): string[] { - const results = flatMap(files, file => - glob.sync(trimSingleQuotes(file), { ignore, nodir: true }), - ); - // warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns - for (const file of filterFiles(files, ignore, false)) { - if (!glob.hasMagic(file) && !results.some(createMinimatchFilter(file))) { - logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 - } - } - const cwd = process.cwd(); - return results.map(file => - outputAbsolutePaths ? path.resolve(cwd, file) : path.relative(cwd, file), - ); -} diff --git a/src/files/program.ts b/src/files/resolution.ts similarity index 58% rename from src/files/program.ts rename to src/files/resolution.ts index dcc4249685a..dc4a4dfed40 100644 --- a/src/files/program.ts +++ b/src/files/resolution.ts @@ -17,16 +17,57 @@ import * as fs from "fs"; import * as glob from "glob"; -import { filter as createMinimatchFilter } from "minimatch"; +import { filter as createMinimatchFilter, Minimatch } from "minimatch"; import * as path from "path"; import * as ts from "typescript"; import { FatalError } from "../error"; import { Linter } from "../linter"; import { Logger, Options } from "../runner"; -import { trimSingleQuotes } from "../utils"; +import { flatMap, trimSingleQuotes } from "../utils"; -import { filterFiles, findTsconfig, resolveGlobs } from "./finding"; +export function filterFiles(files: string[], patterns: string[], include: boolean): string[] { + if (patterns.length === 0) { + return include ? [] : files; + } + const matcher = patterns.map(pattern => new Minimatch(pattern, { dot: !include })); // `glob` always enables `dot` for ignore patterns + return files.filter(file => include === matcher.some(pattern => pattern.match(file))); +} + +export function findTsconfig(project: string): string | undefined { + try { + const stats = fs.statSync(project); // throws if file does not exist + if (!stats.isDirectory()) { + return project; + } + const projectFile = path.join(project, "tsconfig.json"); + fs.accessSync(projectFile); // throws if file does not exist + return projectFile; + } catch (e) { + return undefined; + } +} + +export function resolveGlobs( + files: string[], + ignore: string[], + outputAbsolutePaths: boolean | undefined, + logger: Logger, +): string[] { + const results = flatMap(files, file => + glob.sync(trimSingleQuotes(file), { ignore, nodir: true }), + ); + // warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns + for (const file of filterFiles(files, ignore, false)) { + if (!glob.hasMagic(file) && !results.some(createMinimatchFilter(file))) { + logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0 + } + } + const cwd = process.cwd(); + return results.map(file => + outputAbsolutePaths ? path.resolve(cwd, file) : path.relative(cwd, file), + ); +} export function resolveFilesAndProgram( { files, project, exclude, outputAbsolutePaths }: Options, diff --git a/src/runner.ts b/src/runner.ts index 951fd307216..0ccfd8da504 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -30,11 +30,11 @@ import { stringifyConfiguration, } from "./configuration"; import { FatalError } from "./error"; -import { resolveFilesAndProgram } from "./files/program"; import { tryReadFile } from "./files/reading"; import { LintResult } from "./index"; import { Linter } from "./linter"; import { trimSingleQuotes } from "./utils"; +import { resolveFilesAndProgram } from './files/resolution'; export interface Options { /** @@ -153,7 +153,7 @@ async function runWorker(options: Options, logger: Logger): Promise { } if (options.printConfig) { - return runConfigPrinting(options, logger); + return printConfiguration(options, logger); } if (options.test) { @@ -176,7 +176,7 @@ async function runWorker(options: Options, logger: Logger): Promise { return options.force || errorCount === 0 ? Status.Ok : Status.LintError; } -async function runConfigPrinting(options: Options, logger: Logger): Promise { +async function printConfiguration(options: Options, logger: Logger): Promise { const { files } = options; if (files.length !== 1) { throw new FatalError(`--print-config must be run with exactly one file`); diff --git a/test/configurationTests.ts b/test/configurationTests.ts index 2db73d0fa61..6fcf207082b 100644 --- a/test/configurationTests.ts +++ b/test/configurationTests.ts @@ -26,6 +26,7 @@ import { loadConfigurationFromPath, parseConfigFile, RawConfigFile, + stringifyConfiguration, } from "../src/configuration"; import { IOptions, RuleSeverity } from "../src/language/rule/rule"; @@ -681,6 +682,158 @@ describe("Configuration", () => { }); }); }); + + describe.only("stringifyConfiguration", () => { + const blankConfiguration: IConfigurationFile = { + extends: [], + jsRules: new Map(), + rules: new Map(), + rulesDirectory: [], + }; + + it("stringifies an empty configuration", () => { + const actual = stringifyConfiguration(blankConfiguration); + + assert.equal( + actual, + JSON.stringify( + { + extends: [], + jsRules: {}, + rules: {}, + rulesDirectory: [], + }, + undefined, + 2, + ), + ); + }); + + it("stringifies a configuration with jsRules", () => { + const configuration: IConfigurationFile = { + ...blankConfiguration, + jsRules: new Map([ + [ + "js-rule", + { + ruleArguments: ["sample", "argument"], + ruleName: "js-rule", + }, + ], + ]), + }; + + const actual = stringifyConfiguration(configuration); + + assert.equal( + actual, + JSON.stringify( + { + extends: [], + jsRules: { + "js-rule": { + ruleArguments: ["sample", "argument"], + ruleName: "js-rule" + } + }, + rules: {}, + rulesDirectory: [], + }, + undefined, + 2, + ), + ); + }); + + it("stringifies a configuration with linterOptions", () => { + const configuration: IConfigurationFile = { + ...blankConfiguration, + linterOptions: { + exclude: ["./sample/**/*.ts"], + format: "sample-format" + } + }; + + const actual = stringifyConfiguration(configuration); + + assert.equal( + actual, + JSON.stringify( + { + extends: [], + jsRules: {}, + linterOptions: { + exclude: ["./sample/**/*.ts"], + format: "sample-format", + }, + rules: {}, + rulesDirectory: [], + }, + undefined, + 2, + ), + ); + }); + + it("stringifies a configuration with rules", () => { + const configuration: IConfigurationFile = { + ...blankConfiguration, + rules: new Map([ + ["ts-rule", { + ruleArguments: ["sample", "argument"], + ruleName: "ts-rule", + }] + ]) + }; + + const actual = stringifyConfiguration(configuration); + + assert.equal( + actual, + JSON.stringify( + { + extends: [], + jsRules: {}, + rules: { + "ts-rule": { + ruleArguments: ["sample", "argument"], + ruleName: "ts-rule" + } + }, + rulesDirectory: [], + }, + undefined, + 2, + ), + ); + }); + + it("stringifies a configuration with rulesDirectory", () => { + const configuration: IConfigurationFile = { + ...blankConfiguration, + rulesDirectory: ["./directory/one", "./directory/two"] + }; + + const actual = stringifyConfiguration(configuration); + + assert.equal( + actual, + JSON.stringify( + { + extends: [], + jsRules: {}, + rules: {}, + rulesDirectory: [ + "./directory/one", + "./directory/two", + ], + }, + undefined, + 2, + ), + ); + }); + }); }); function getEmptyConfig(): IConfigurationFile { From d57909d229ff0e4060b5c46745ec0fb024b63406 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 1 Jun 2019 08:42:18 -0400 Subject: [PATCH 8/8] Lint, Prettier fixups --- src/runner.ts | 2 +- test/configurationTests.ts | 34 +++++++++++++++++----------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/runner.ts b/src/runner.ts index 0ccfd8da504..7e47595a226 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -31,10 +31,10 @@ import { } from "./configuration"; import { FatalError } from "./error"; import { tryReadFile } from "./files/reading"; +import { resolveFilesAndProgram } from "./files/resolution"; import { LintResult } from "./index"; import { Linter } from "./linter"; import { trimSingleQuotes } from "./utils"; -import { resolveFilesAndProgram } from './files/resolution'; export interface Options { /** diff --git a/test/configurationTests.ts b/test/configurationTests.ts index 6fcf207082b..3305a510023 100644 --- a/test/configurationTests.ts +++ b/test/configurationTests.ts @@ -683,7 +683,7 @@ describe("Configuration", () => { }); }); - describe.only("stringifyConfiguration", () => { + describe("stringifyConfiguration", () => { const blankConfiguration: IConfigurationFile = { extends: [], jsRules: new Map(), @@ -733,8 +733,8 @@ describe("Configuration", () => { jsRules: { "js-rule": { ruleArguments: ["sample", "argument"], - ruleName: "js-rule" - } + ruleName: "js-rule", + }, }, rules: {}, rulesDirectory: [], @@ -750,8 +750,8 @@ describe("Configuration", () => { ...blankConfiguration, linterOptions: { exclude: ["./sample/**/*.ts"], - format: "sample-format" - } + format: "sample-format", + }, }; const actual = stringifyConfiguration(configuration); @@ -779,11 +779,14 @@ describe("Configuration", () => { const configuration: IConfigurationFile = { ...blankConfiguration, rules: new Map([ - ["ts-rule", { - ruleArguments: ["sample", "argument"], - ruleName: "ts-rule", - }] - ]) + [ + "ts-rule", + { + ruleArguments: ["sample", "argument"], + ruleName: "ts-rule", + }, + ], + ]), }; const actual = stringifyConfiguration(configuration); @@ -797,8 +800,8 @@ describe("Configuration", () => { rules: { "ts-rule": { ruleArguments: ["sample", "argument"], - ruleName: "ts-rule" - } + ruleName: "ts-rule", + }, }, rulesDirectory: [], }, @@ -811,7 +814,7 @@ describe("Configuration", () => { it("stringifies a configuration with rulesDirectory", () => { const configuration: IConfigurationFile = { ...blankConfiguration, - rulesDirectory: ["./directory/one", "./directory/two"] + rulesDirectory: ["./directory/one", "./directory/two"], }; const actual = stringifyConfiguration(configuration); @@ -823,10 +826,7 @@ describe("Configuration", () => { extends: [], jsRules: {}, rules: {}, - rulesDirectory: [ - "./directory/one", - "./directory/two", - ], + rulesDirectory: ["./directory/one", "./directory/two"], }, undefined, 2,