From d495d9d258693c1c8e9c03a7c2c86d11f0278897 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 13 Nov 2019 14:31:33 -0800 Subject: [PATCH 1/4] fix(typescript-estree): correctly account f trailing slash in includes --- .../src/create-program/createWatchProgram.ts | 3 +- .../src/create-program/shared.ts | 17 ++-- .../tests/lib/persistentParse.ts | 78 +++++++++++++++++-- 3 files changed, 83 insertions(+), 15 deletions(-) diff --git a/packages/typescript-estree/src/create-program/createWatchProgram.ts b/packages/typescript-estree/src/create-program/createWatchProgram.ts index 9223107986af..8776bed53737 100644 --- a/packages/typescript-estree/src/create-program/createWatchProgram.ts +++ b/packages/typescript-estree/src/create-program/createWatchProgram.ts @@ -246,8 +246,7 @@ function createWatchProgram( watchCompilerHost.readFile = (filePathIn, encoding): string | undefined => { const filePath = getCanonicalFileName(filePathIn); const fileContent = - path.normalize(filePath) === - path.normalize(currentLintOperationState.filePath) + filePath === currentLintOperationState.filePath ? currentLintOperationState.code : oldReadFile(filePath, encoding); if (fileContent) { diff --git a/packages/typescript-estree/src/create-program/shared.ts b/packages/typescript-estree/src/create-program/shared.ts index 6509997094f9..a19bef274139 100644 --- a/packages/typescript-estree/src/create-program/shared.ts +++ b/packages/typescript-estree/src/create-program/shared.ts @@ -20,14 +20,21 @@ const DEFAULT_COMPILER_OPTIONS: ts.CompilerOptions = { // This narrows the type so we can be sure we're passing canonical names in the correct places type CanonicalPath = string & { __brand: unknown }; + // typescript doesn't provide a ts.sys implementation for browser environments const useCaseSensitiveFileNames = ts.sys !== undefined ? ts.sys.useCaseSensitiveFileNames : true; -const getCanonicalFileName = useCaseSensitiveFileNames - ? (filePath: string): CanonicalPath => - path.normalize(filePath) as CanonicalPath - : (filePath: string): CanonicalPath => - path.normalize(filePath).toLowerCase() as CanonicalPath; +const correctPathCasing = useCaseSensitiveFileNames + ? (filePath: string): string => filePath + : (filePath: string): string => filePath.toLowerCase(); + +function getCanonicalFileName(filePath: string): CanonicalPath { + let normalized = path.normalize(filePath); + if (normalized.endsWith('/')) { + normalized = normalized.substr(0, normalized.length - 1); + } + return correctPathCasing(normalized) as CanonicalPath; +} function getTsconfigPath(tsconfigPath: string, extra: Extra): CanonicalPath { return getCanonicalFileName( diff --git a/packages/typescript-estree/tests/lib/persistentParse.ts b/packages/typescript-estree/tests/lib/persistentParse.ts index e1cb8b9f9b76..a1bf457cd2cd 100644 --- a/packages/typescript-estree/tests/lib/persistentParse.ts +++ b/packages/typescript-estree/tests/lib/persistentParse.ts @@ -3,14 +3,6 @@ import path from 'path'; import tmp from 'tmp'; import { clearCaches, parseAndGenerateServices } from '../../src/parser'; -const tsConfigExcludeBar = { - include: ['src'], - exclude: ['./src/bar.ts'], -}; -const tsConfigIncludeAll = { - include: ['src'], - exclude: [], -}; const CONTENTS = { foo: 'console.log("foo")', bar: 'console.log("bar")', @@ -70,6 +62,15 @@ function parseFile(filename: 'foo' | 'bar' | 'baz/bar', tmpDir: string): void { } describe('persistent lint session', () => { + const tsConfigExcludeBar = { + include: ['src'], + exclude: ['./src/bar.ts'], + }; + const tsConfigIncludeAll = { + include: ['src'], + exclude: [], + }; + it('parses both files successfully when included', () => { const PROJECT_DIR = setup(tsConfigIncludeAll); @@ -176,3 +177,64 @@ describe('persistent lint session', () => { // TODO - support the complex monorepo case with a tsconfig with no include/exclude }); + +/* +If the includes ends in a slash, typescript will ask for watchers ending in a slash. +These tests ensure the normalisation code works as expected in this case. +*/ +describe('includes ending in a slash', () => { + const tsConfigExcludeBar = { + include: ['src/'], + exclude: ['./src/bar.ts'], + }; + const tsConfigIncludeAll = { + include: ['src/'], + exclude: [], + }; + + it('parses both files successfully when included', () => { + const PROJECT_DIR = setup(tsConfigIncludeAll); + + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + expect(() => parseFile('bar', PROJECT_DIR)).not.toThrow(); + }); + + it('parses included files, and throws on excluded files', () => { + const PROJECT_DIR = setup(tsConfigExcludeBar); + + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + expect(() => parseFile('bar', PROJECT_DIR)).toThrow(); + }); + + it('allows parsing of new files', () => { + const PROJECT_DIR = setup(tsConfigIncludeAll, false); + + // parse once to: assert the config as correct, and to make sure the program is setup + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + // bar should throw because it doesn't exist yet + expect(() => parseFile('bar', PROJECT_DIR)).toThrow(); + + // write a new file and attempt to parse it + writeFile(PROJECT_DIR, 'bar'); + + // both files should parse fine now + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + expect(() => parseFile('bar', PROJECT_DIR)).not.toThrow(); + }); + + it('allows parsing of deeply nested new files', () => { + const PROJECT_DIR = setup(tsConfigIncludeAll, false); + + // parse once to: assert the config as correct, and to make sure the program is setup + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + // bar should throw because it doesn't exist yet + expect(() => parseFile('baz/bar', PROJECT_DIR)).toThrow(); + + // write a new file and attempt to parse it + writeFile(PROJECT_DIR, 'baz/bar'); + + // both files should parse fine now + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + expect(() => parseFile('baz/bar', PROJECT_DIR)).not.toThrow(); + }); +}); From 4f80af6a0a810b34d3cbcb1ff9d97e177924c5c8 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 13 Nov 2019 14:45:42 -0800 Subject: [PATCH 2/4] chore: refactor to share all tests for better coverage --- .../tests/lib/persistentParse.ts | 99 ++++++------------- 1 file changed, 32 insertions(+), 67 deletions(-) diff --git a/packages/typescript-estree/tests/lib/persistentParse.ts b/packages/typescript-estree/tests/lib/persistentParse.ts index a1bf457cd2cd..6098869b7ff2 100644 --- a/packages/typescript-estree/tests/lib/persistentParse.ts +++ b/packages/typescript-estree/tests/lib/persistentParse.ts @@ -61,16 +61,10 @@ function parseFile(filename: 'foo' | 'bar' | 'baz/bar', tmpDir: string): void { }); } -describe('persistent lint session', () => { - const tsConfigExcludeBar = { - include: ['src'], - exclude: ['./src/bar.ts'], - }; - const tsConfigIncludeAll = { - include: ['src'], - exclude: [], - }; - +function baseTests( + tsConfigExcludeBar: Record, + tsConfigIncludeAll: Record, +): void { it('parses both files successfully when included', () => { const PROJECT_DIR = setup(tsConfigIncludeAll); @@ -176,65 +170,36 @@ describe('persistent lint session', () => { }); // TODO - support the complex monorepo case with a tsconfig with no include/exclude -}); - -/* -If the includes ends in a slash, typescript will ask for watchers ending in a slash. -These tests ensure the normalisation code works as expected in this case. -*/ -describe('includes ending in a slash', () => { - const tsConfigExcludeBar = { - include: ['src/'], - exclude: ['./src/bar.ts'], - }; - const tsConfigIncludeAll = { - include: ['src/'], - exclude: [], - }; - - it('parses both files successfully when included', () => { - const PROJECT_DIR = setup(tsConfigIncludeAll); - - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - expect(() => parseFile('bar', PROJECT_DIR)).not.toThrow(); - }); - - it('parses included files, and throws on excluded files', () => { - const PROJECT_DIR = setup(tsConfigExcludeBar); - - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - expect(() => parseFile('bar', PROJECT_DIR)).toThrow(); - }); - - it('allows parsing of new files', () => { - const PROJECT_DIR = setup(tsConfigIncludeAll, false); - - // parse once to: assert the config as correct, and to make sure the program is setup - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - // bar should throw because it doesn't exist yet - expect(() => parseFile('bar', PROJECT_DIR)).toThrow(); - - // write a new file and attempt to parse it - writeFile(PROJECT_DIR, 'bar'); +} - // both files should parse fine now - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - expect(() => parseFile('bar', PROJECT_DIR)).not.toThrow(); +describe('persistent parse', () => { + describe('includes not ending in a slash', () => { + const tsConfigExcludeBar = { + include: ['src'], + exclude: ['./src/bar.ts'], + }; + const tsConfigIncludeAll = { + include: ['src'], + exclude: [], + }; + + baseTests(tsConfigExcludeBar, tsConfigIncludeAll); }); - it('allows parsing of deeply nested new files', () => { - const PROJECT_DIR = setup(tsConfigIncludeAll, false); - - // parse once to: assert the config as correct, and to make sure the program is setup - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - // bar should throw because it doesn't exist yet - expect(() => parseFile('baz/bar', PROJECT_DIR)).toThrow(); - - // write a new file and attempt to parse it - writeFile(PROJECT_DIR, 'baz/bar'); - - // both files should parse fine now - expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); - expect(() => parseFile('baz/bar', PROJECT_DIR)).not.toThrow(); + /* + If the includes ends in a slash, typescript will ask for watchers ending in a slash. + These tests ensure the normalisation code works as expected in this case. + */ + describe('includes ending in a slash', () => { + const tsConfigExcludeBar = { + include: ['src/'], + exclude: ['./src/bar.ts'], + }; + const tsConfigIncludeAll = { + include: ['src/'], + exclude: [], + }; + + baseTests(tsConfigExcludeBar, tsConfigIncludeAll); }); }); From 38f277610e6205c0d4cf40bb93ca6698c1cec369 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 14 Nov 2019 09:38:05 -0800 Subject: [PATCH 3/4] chore: remove unnecessary normalize --- .../typescript-estree/src/create-program/createWatchProgram.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/typescript-estree/src/create-program/createWatchProgram.ts b/packages/typescript-estree/src/create-program/createWatchProgram.ts index 8776bed53737..38bd209ff3f2 100644 --- a/packages/typescript-estree/src/create-program/createWatchProgram.ts +++ b/packages/typescript-estree/src/create-program/createWatchProgram.ts @@ -1,6 +1,5 @@ import debug from 'debug'; import fs from 'fs'; -import path from 'path'; import * as ts from 'typescript'; // leave this as * as ts so people using util package don't need syntheticDefaultImports import { Extra } from '../parser-options'; import { WatchCompilerHostOfConfigFile } from './WatchCompilerHostOfConfigFile'; @@ -67,7 +66,7 @@ function saveWatchCallback( fileName: string, callback: ts.FileWatcherCallback, ): ts.FileWatcher => { - const normalizedFileName = getCanonicalFileName(path.normalize(fileName)); + const normalizedFileName = getCanonicalFileName(fileName); const watchers = ((): Set => { let watchers = trackingMap.get(normalizedFileName); if (!watchers) { From bb6b7f415b47a2b0dc673f9263e6cbc38da88d79 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 14 Nov 2019 17:20:40 -0800 Subject: [PATCH 4/4] test: add test for no includes --- .../typescript-estree/tests/lib/persistentParse.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/typescript-estree/tests/lib/persistentParse.ts b/packages/typescript-estree/tests/lib/persistentParse.ts index 6098869b7ff2..502245977bdc 100644 --- a/packages/typescript-estree/tests/lib/persistentParse.ts +++ b/packages/typescript-estree/tests/lib/persistentParse.ts @@ -202,4 +202,16 @@ describe('persistent parse', () => { baseTests(tsConfigExcludeBar, tsConfigIncludeAll); }); + + /* + If there is no includes, then typescript will ask for a slightly different set of watchers. + */ + describe('tsconfig with no includes / files', () => { + const tsConfigExcludeBar = { + exclude: ['./src/bar.ts'], + }; + const tsConfigIncludeAll = {}; + + baseTests(tsConfigExcludeBar, tsConfigIncludeAll); + }); });