From 206c94bddc2d0ed428f41eb954966e8f7ba4c97f Mon Sep 17 00:00:00 2001 From: Armano Date: Wed, 8 Jan 2020 22:26:45 +0100 Subject: [PATCH 1/2] fix(typescript-estree): parsing of deeply nested new files in new folder (#1412) Co-authored-by: Brad Zacher --- .../src/create-program/createWatchProgram.ts | 7 +++--- .../tests/lib/persistentParse.ts | 23 +++++++++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/typescript-estree/src/create-program/createWatchProgram.ts b/packages/typescript-estree/src/create-program/createWatchProgram.ts index 72b5535c5783..5d975f7d6d14 100644 --- a/packages/typescript-estree/src/create-program/createWatchProgram.ts +++ b/packages/typescript-estree/src/create-program/createWatchProgram.ts @@ -394,9 +394,10 @@ function maybeInvalidateProgram( current = next; const folderWatchCallbacks = folderWatchCallbackTrackingMap.get(current); if (folderWatchCallbacks) { - folderWatchCallbacks.forEach(cb => - cb(currentDir, ts.FileWatcherEventKind.Changed), - ); + folderWatchCallbacks.forEach(cb => { + cb(currentDir, ts.FileWatcherEventKind.Changed); + cb(current!, ts.FileWatcherEventKind.Changed); + }); hasCallback = true; break; } diff --git a/packages/typescript-estree/tests/lib/persistentParse.ts b/packages/typescript-estree/tests/lib/persistentParse.ts index 8be08c9c4572..bdeed2f55389 100644 --- a/packages/typescript-estree/tests/lib/persistentParse.ts +++ b/packages/typescript-estree/tests/lib/persistentParse.ts @@ -7,6 +7,7 @@ const CONTENTS = { foo: 'console.log("foo")', bar: 'console.log("bar")', 'baz/bar': 'console.log("baz bar")', + 'bat/baz/bar': 'console.log("bat/baz/bar")', }; const tmpDirs = new Set(); @@ -22,7 +23,7 @@ afterEach(() => { function writeTSConfig(dirName: string, config: Record): void { fs.writeFileSync(path.join(dirName, 'tsconfig.json'), JSON.stringify(config)); } -function writeFile(dirName: string, file: 'foo' | 'bar' | 'baz/bar'): void { +function writeFile(dirName: string, file: keyof typeof CONTENTS): void { fs.writeFileSync(path.join(dirName, 'src', `${file}.ts`), CONTENTS[file]); } function renameFile(dirName: string, src: 'bar', dest: 'baz/bar'): void { @@ -53,7 +54,7 @@ function setup(tsconfig: Record, writeBar = true): string { return tmpDir.name; } -function parseFile(filename: 'foo' | 'bar' | 'baz/bar', tmpDir: string): void { +function parseFile(filename: keyof typeof CONTENTS, tmpDir: string): void { parseAndGenerateServices(CONTENTS.foo, { project: './tsconfig.json', tsconfigRootDir: tmpDir, @@ -112,6 +113,24 @@ function baseTests( expect(() => parseFile(bazSlashBar, PROJECT_DIR)).not.toThrow(); }); + it('allows parsing of deeply nested new files in new folder', () => { + const PROJECT_DIR = setup(tsConfigIncludeAll); + + expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow(); + + // Create deep folder structure after first parse (this is important step) + // context: https://github.com/typescript-eslint/typescript-eslint/issues/1394 + fs.mkdirSync(path.join(PROJECT_DIR, 'src', 'bat')); + fs.mkdirSync(path.join(PROJECT_DIR, 'src', 'bat', 'baz')); + + const bazSlashBar = path.join('bat', 'baz', 'bar') as 'bat/baz/bar'; + + // write a new file and attempt to parse it + writeFile(PROJECT_DIR, bazSlashBar); + + expect(() => parseFile(bazSlashBar, PROJECT_DIR)).not.toThrow(); + }); + it('allows renaming of files', () => { const PROJECT_DIR = setup(tsConfigIncludeAll, true); const bazSlashBar = path.join('baz', 'bar') as 'baz/bar'; From 852fc3143cd287c396562fb72b6f6b97ad730281 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 8 Jan 2020 14:30:01 -0800 Subject: [PATCH 2/2] fix(eslint-plugin): [no-magic-numbers] handle UnaryExpression for enums (#1415) --- .../src/rules/no-magic-numbers.ts | 245 +++++++++--------- .../tests/rules/no-magic-numbers.test.ts | 74 ++++-- 2 files changed, 169 insertions(+), 150 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-magic-numbers.ts b/packages/eslint-plugin/src/rules/no-magic-numbers.ts index b4791614cda2..6576ad4b49f8 100644 --- a/packages/eslint-plugin/src/rules/no-magic-numbers.ts +++ b/packages/eslint-plugin/src/rules/no-magic-numbers.ts @@ -55,130 +55,6 @@ export default util.createRule({ create(context, [options]) { const rules = baseRule.create(context); - /** - * Returns whether the node is number literal - * @param node the node literal being evaluated - * @returns true if the node is a number literal - */ - function isNumber(node: TSESTree.Literal): boolean { - return typeof node.value === 'number'; - } - - /** - * Checks if the node grandparent is a Typescript type alias declaration - * @param node the node to be validated. - * @returns true if the node grandparent is a Typescript type alias declaration - * @private - */ - function isGrandparentTSTypeAliasDeclaration(node: TSESTree.Node): boolean { - return node.parent && node.parent.parent - ? node.parent.parent.type === AST_NODE_TYPES.TSTypeAliasDeclaration - : false; - } - - /** - * Checks if the node grandparent is a Typescript union type and its parent is a type alias declaration - * @param node the node to be validated. - * @returns true if the node grandparent is a Typescript union type and its parent is a type alias declaration - * @private - */ - function isGrandparentTSUnionType(node: TSESTree.Node): boolean { - if ( - node.parent && - node.parent.parent && - node.parent.parent.type === AST_NODE_TYPES.TSUnionType - ) { - return isGrandparentTSTypeAliasDeclaration(node.parent); - } - - return false; - } - - /** - * Checks if the node parent is a Typescript enum member - * @param node the node to be validated. - * @returns true if the node parent is a Typescript enum member - * @private - */ - function isParentTSEnumDeclaration(node: TSESTree.Node): boolean { - return ( - typeof node.parent !== 'undefined' && - node.parent.type === AST_NODE_TYPES.TSEnumMember - ); - } - - /** - * Checks if the node parent is a Typescript literal type - * @param node the node to be validated. - * @returns true if the node parent is a Typescript literal type - * @private - */ - function isParentTSLiteralType(node: TSESTree.Node): boolean { - return node.parent - ? node.parent.type === AST_NODE_TYPES.TSLiteralType - : false; - } - - /** - * Checks if the node is a valid TypeScript numeric literal type. - * @param node the node to be validated. - * @returns true if the node is a TypeScript numeric literal type. - * @private - */ - function isTSNumericLiteralType(node: TSESTree.Node): boolean { - // For negative numbers, update the parent node - if ( - node.parent && - node.parent.type === AST_NODE_TYPES.UnaryExpression && - node.parent.operator === '-' - ) { - node = node.parent; - } - - // If the parent node is not a TSLiteralType, early return - if (!isParentTSLiteralType(node)) { - return false; - } - - // If the grandparent is a TSTypeAliasDeclaration, ignore - if (isGrandparentTSTypeAliasDeclaration(node)) { - return true; - } - - // If the grandparent is a TSUnionType and it's parent is a TSTypeAliasDeclaration, ignore - if (isGrandparentTSUnionType(node)) { - return true; - } - - return false; - } - - /** - * Checks if the node parent is a readonly class property - * @param node the node to be validated. - * @returns true if the node parent is a readonly class property - * @private - */ - function isParentTSReadonlyClassProperty(node: TSESTree.Node): boolean { - if ( - node.parent && - node.parent.type === AST_NODE_TYPES.UnaryExpression && - ['-', '+'].includes(node.parent.operator) - ) { - node = node.parent; - } - - if ( - node.parent && - node.parent.type === AST_NODE_TYPES.ClassProperty && - node.parent.readonly - ) { - return true; - } - - return false; - } - return { Literal(node): void { // Check if the node is a TypeScript enum declaration @@ -189,14 +65,17 @@ export default util.createRule({ // Check TypeScript specific nodes for Numeric Literal if ( options.ignoreNumericLiteralTypes && - isNumber(node) && + typeof node.value === 'number' && isTSNumericLiteralType(node) ) { return; } // Check if the node is a readonly class property - if (isNumber(node) && isParentTSReadonlyClassProperty(node)) { + if ( + typeof node.value === 'number' && + isParentTSReadonlyClassProperty(node) + ) { if (options.ignoreReadonlyClassProperties) { return; } @@ -207,8 +86,10 @@ export default util.createRule({ let raw = node.raw; if ( - node.parent && - node.parent.type === AST_NODE_TYPES.UnaryExpression + node.parent?.type === AST_NODE_TYPES.UnaryExpression && + // the base rule only shows the operator for negative numbers + // https://github.com/eslint/eslint/blob/9dfc8501fb1956c90dc11e6377b4cb38a6bea65d/lib/rules/no-magic-numbers.js#L126 + node.parent.operator === '-' ) { fullNumberNode = node.parent; raw = `${node.parent.operator}${node.raw}`; @@ -229,3 +110,111 @@ export default util.createRule({ }; }, }); + +/** + * Gets the true parent of the literal, handling prefixed numbers (-1 / +1) + */ +function getLiteralParent(node: TSESTree.Literal): TSESTree.Node | undefined { + if ( + node.parent?.type === AST_NODE_TYPES.UnaryExpression && + ['-', '+'].includes(node.parent.operator) + ) { + return node.parent.parent; + } + + return node.parent; +} + +/** + * Checks if the node grandparent is a Typescript type alias declaration + * @param node the node to be validated. + * @returns true if the node grandparent is a Typescript type alias declaration + * @private + */ +function isGrandparentTSTypeAliasDeclaration(node: TSESTree.Node): boolean { + return node.parent?.parent?.type === AST_NODE_TYPES.TSTypeAliasDeclaration; +} + +/** + * Checks if the node grandparent is a Typescript union type and its parent is a type alias declaration + * @param node the node to be validated. + * @returns true if the node grandparent is a Typescript union type and its parent is a type alias declaration + * @private + */ +function isGrandparentTSUnionType(node: TSESTree.Node): boolean { + if (node.parent?.parent?.type === AST_NODE_TYPES.TSUnionType) { + return isGrandparentTSTypeAliasDeclaration(node.parent); + } + + return false; +} + +/** + * Checks if the node parent is a Typescript enum member + * @param node the node to be validated. + * @returns true if the node parent is a Typescript enum member + * @private + */ +function isParentTSEnumDeclaration(node: TSESTree.Literal): boolean { + const parent = getLiteralParent(node); + return parent?.type === AST_NODE_TYPES.TSEnumMember; +} + +/** + * Checks if the node parent is a Typescript literal type + * @param node the node to be validated. + * @returns true if the node parent is a Typescript literal type + * @private + */ +function isParentTSLiteralType(node: TSESTree.Node): boolean { + return node.parent?.type === AST_NODE_TYPES.TSLiteralType; +} + +/** + * Checks if the node is a valid TypeScript numeric literal type. + * @param node the node to be validated. + * @returns true if the node is a TypeScript numeric literal type. + * @private + */ +function isTSNumericLiteralType(node: TSESTree.Node): boolean { + // For negative numbers, use the parent node + if ( + node.parent?.type === AST_NODE_TYPES.UnaryExpression && + node.parent.operator === '-' + ) { + node = node.parent; + } + + // If the parent node is not a TSLiteralType, early return + if (!isParentTSLiteralType(node)) { + return false; + } + + // If the grandparent is a TSTypeAliasDeclaration, ignore + if (isGrandparentTSTypeAliasDeclaration(node)) { + return true; + } + + // If the grandparent is a TSUnionType and it's parent is a TSTypeAliasDeclaration, ignore + if (isGrandparentTSUnionType(node)) { + return true; + } + + return false; +} + +/** + * Checks if the node parent is a readonly class property + * @param node the node to be validated. + * @returns true if the node parent is a readonly class property + * @private + */ +function isParentTSReadonlyClassProperty(node: TSESTree.Literal): boolean { + const parent = getLiteralParent(node); + + if (parent?.type === AST_NODE_TYPES.ClassProperty && parent.readonly) { + return true; + } + + return false; +} diff --git a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts index f65610ea3f4b..9c8bdeae10a8 100644 --- a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts +++ b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts @@ -34,11 +34,14 @@ ruleTester.run('no-magic-numbers', rule, { options: [{ ignoreNumericLiteralTypes: true }], }, { - code: 'enum foo { SECOND = 1000 }', - options: [{ ignoreEnums: true }], - }, - { - code: 'enum foo { SECOND = 1000, NUM = "0123456789" }', + code: ` + enum foo { + SECOND = 1000, + NUM = "0123456789", + NEG = -1, + POS = +1, + } + `, options: [{ ignoreEnums: true }], }, { @@ -152,7 +155,14 @@ class Foo { ], }, { - code: 'enum foo { SECOND = 1000 }', + code: ` +enum foo { + SECOND = 1000, + NUM = "0123456789", + NEG = -1, + POS = +1, +} + `, options: [{ ignoreEnums: false }], errors: [ { @@ -160,22 +170,24 @@ class Foo { data: { raw: '1000', }, - line: 1, - column: 21, + line: 3, + column: 12, }, - ], - }, - { - code: 'enum foo { SECOND = 1000, NUM = "0123456789" }', - options: [{ ignoreEnums: false }], - errors: [ { messageId: 'noMagic', data: { - raw: '1000', + raw: '-1', }, - line: 1, - column: 21, + line: 5, + column: 9, + }, + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 6, + column: 10, }, ], }, @@ -184,43 +196,61 @@ class Foo { class Foo { readonly A = 1; readonly B = 2; - public static readonly C = 1; - static readonly D = 1; - readonly E = -1; - readonly F = +1; + public static readonly C = 3; + static readonly D = 4; + readonly E = -5; + readonly F = +6; } `, options: [{ ignoreReadonlyClassProperties: false }], errors: [ { messageId: 'noMagic', + data: { + raw: '1', + }, line: 3, column: 16, }, { messageId: 'noMagic', + data: { + raw: '2', + }, line: 4, column: 16, }, { messageId: 'noMagic', + data: { + raw: '3', + }, line: 5, column: 30, }, { messageId: 'noMagic', + data: { + raw: '4', + }, line: 6, column: 23, }, { messageId: 'noMagic', + data: { + raw: '-5', + }, line: 7, column: 16, }, { messageId: 'noMagic', + data: { + raw: '6', + }, line: 8, - column: 16, + column: 17, }, ], },