From 913de4ce14448c9c16173def3344757a766d6124 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 6 Jun 2019 12:55:26 +0900 Subject: [PATCH 1/4] fix(eslint-plugin): modify to don't use 'isTypescriptFile' in rules --- .../rules/explicit-function-return-type.ts | 10 +- .../rules/explicit-member-accessibility.ts | 110 +++++++++--------- .../explicit-function-return-type.test.ts | 8 -- .../explicit-member-accessibility.test.ts | 23 ---- 4 files changed, 56 insertions(+), 95 deletions(-) diff --git a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts index 876f42517d61..83a07ba96f13 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -211,12 +211,10 @@ export default util.createRule({ return; } - if (util.isTypeScriptFile(context.getFilename())) { - context.report({ - node, - messageId: 'missingReturnType', - }); - } + context.report({ + node, + messageId: 'missingReturnType', + }); } /** diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index 4fea64e99ebc..bb6af6333da5 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -119,30 +119,28 @@ export default util.createRule({ return; } - if (util.isTypeScriptFile(context.getFilename())) { - // const methodName = util.getNameFromPropertyName(methodDefinition.key); - const methodName = util.getNameFromClassMember( + // const methodName = util.getNameFromPropertyName(methodDefinition.key); + const methodName = util.getNameFromClassMember( + methodDefinition, + sourceCode, + ); + if ( + check === 'no-public' && + methodDefinition.accessibility === 'public' + ) { + reportIssue( + 'unwantedPublicAccessibility', + nodeType, methodDefinition, - sourceCode, + methodName, + ); + } else if (check === 'explicit' && !methodDefinition.accessibility) { + reportIssue( + 'missingAccessibility', + nodeType, + methodDefinition, + methodName, ); - if ( - check === 'no-public' && - methodDefinition.accessibility === 'public' - ) { - reportIssue( - 'unwantedPublicAccessibility', - nodeType, - methodDefinition, - methodName, - ); - } else if (check === 'explicit' && !methodDefinition.accessibility) { - reportIssue( - 'missingAccessibility', - nodeType, - methodDefinition, - methodName, - ); - } } } @@ -155,26 +153,24 @@ export default util.createRule({ ): void { const nodeType = 'class property'; - if (util.isTypeScriptFile(context.getFilename())) { - const propertyName = util.getNameFromPropertyName(classProperty.key); - if ( - propCheck === 'no-public' && - classProperty.accessibility === 'public' - ) { - reportIssue( - 'unwantedPublicAccessibility', - nodeType, - classProperty, - propertyName, - ); - } else if (propCheck === 'explicit' && !classProperty.accessibility) { - reportIssue( - 'missingAccessibility', - nodeType, - classProperty, - propertyName, - ); - } + const propertyName = util.getNameFromPropertyName(classProperty.key); + if ( + propCheck === 'no-public' && + classProperty.accessibility === 'public' + ) { + reportIssue( + 'unwantedPublicAccessibility', + nodeType, + classProperty, + propertyName, + ); + } else if (propCheck === 'explicit' && !classProperty.accessibility) { + reportIssue( + 'missingAccessibility', + nodeType, + classProperty, + propertyName, + ); } } @@ -186,24 +182,22 @@ export default util.createRule({ node: TSESTree.TSParameterProperty, ) { const nodeType = 'parameter property'; - if (util.isTypeScriptFile(context.getFilename())) { - // HAS to be an identifier or assignment or TSC will throw - if ( - node.parameter.type !== AST_NODE_TYPES.Identifier && - node.parameter.type !== AST_NODE_TYPES.AssignmentPattern - ) { - return; - } + // HAS to be an identifier or assignment or TSC will throw + if ( + node.parameter.type !== AST_NODE_TYPES.Identifier && + node.parameter.type !== AST_NODE_TYPES.AssignmentPattern + ) { + return; + } - const nodeName = - node.parameter.type === AST_NODE_TYPES.Identifier - ? node.parameter.name - : // has to be an Identifier or TSC will throw an error - (node.parameter.left as TSESTree.Identifier).name; + const nodeName = + node.parameter.type === AST_NODE_TYPES.Identifier + ? node.parameter.name + : // has to be an Identifier or TSC will throw an error + (node.parameter.left as TSESTree.Identifier).name; - if (paramPropCheck === 'no-public' && node.accessibility === 'public') { - reportIssue('unwantedPublicAccessibility', nodeType, node, nodeName); - } + if (paramPropCheck === 'no-public' && node.accessibility === 'public') { + reportIssue('unwantedPublicAccessibility', nodeType, node, nodeName); } } diff --git a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts index 1b9209b7abf0..77fef025d6e2 100644 --- a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts @@ -42,14 +42,6 @@ class Test { return; } arrow = (): string => 'arrow'; -} - `, - }, - { - filename: 'test.js', - code: ` -function test() { - return; } `, }, diff --git a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts index 51a94a19de20..a7af5ac6d4e9 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -26,16 +26,6 @@ class Test { protected name: string protected foo?: string public "foo-bar"?: string -} - `, - }, - { - filename: 'test.js', - code: ` -class Test { - getX () { - return 1; - } } `, }, @@ -149,19 +139,6 @@ class Test { }, ], }, - { - filename: 'test.js', - code: ` -class Test { - constructor(public x: number){} -} - `, - options: [ - { - accessibility: 'no-public', - }, - ], - }, ], invalid: [ { From 4569e6cca364d642aadf62d1a62e1431e4cddbb2 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 6 Jun 2019 12:57:25 +0900 Subject: [PATCH 2/4] fix(eslint-plugin): remove 'isTypescriptFile' from util --- packages/eslint-plugin/src/util/misc.ts | 7 ----- packages/eslint-plugin/tests/util.test.ts | 36 ----------------------- 2 files changed, 43 deletions(-) diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index 2a9ba2a1c934..3c88d766fe99 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -8,13 +8,6 @@ import { TSESTree, } from '@typescript-eslint/experimental-utils'; -/** - * Check if the context file name is *.ts or *.tsx - */ -export function isTypeScriptFile(fileName: string) { - return /\.tsx?$/i.test(fileName || ''); -} - /** * Check if the context file name is *.d.ts or *.d.tsx */ diff --git a/packages/eslint-plugin/tests/util.test.ts b/packages/eslint-plugin/tests/util.test.ts index 59c820d28b9c..449cfc9b1adb 100644 --- a/packages/eslint-plugin/tests/util.test.ts +++ b/packages/eslint-plugin/tests/util.test.ts @@ -2,42 +2,6 @@ import assert from 'assert'; import * as util from '../src/util'; -describe('isTypescript', () => { - it('returns false for non-typescript files', () => { - const invalid = [ - 'test.js', - 'test.jsx', - 'README.md', - 'test.d.js', - 'test.ts.js', - 'test.ts.map', - 'test.ts-js', - 'ts', - ]; - - invalid.forEach(f => { - assert.strictEqual(util.isTypeScriptFile(f), false); - }); - }); - - it('returns true for typescript files', () => { - const valid = [ - 'test.ts', - 'test.tsx', - 'test.TS', - 'test.TSX', - 'test.d.ts', - 'test.d.tsx', - 'test.D.TS', - 'test.D.TSX', - ]; - - valid.forEach(f => { - assert.strictEqual(util.isTypeScriptFile(f), true); - }); - }); -}); - describe('isDefinitionFile', () => { it('returns false for non-definition files', () => { const invalid = [ From 595013be8fc9cafa7fe74c2c9ab92bc94090b219 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 6 Jun 2019 15:11:08 +0900 Subject: [PATCH 3/4] fix(eslint-plugin): add test for parameter properties --- .../tests/rules/explicit-member-accessibility.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts index a7af5ac6d4e9..abf6ca2b25ec 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -32,6 +32,14 @@ class Test { { filename: 'test.ts', code: ` +class Test { + public constructor({x, y}: {x: number; y: number;}) {} +} + `, + }, + { + filename: 'test.ts', + code: ` class Test { protected name: string protected foo?: string From 601159dcea0c21848a885780520a9be20a3347b4 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 20 Jun 2019 13:22:25 +0900 Subject: [PATCH 4/4] fix(eslint-plugin): remove unnecessary comment --- .../eslint-plugin/src/rules/explicit-member-accessibility.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index bb6af6333da5..8d2b590ffad8 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -119,7 +119,6 @@ export default util.createRule({ return; } - // const methodName = util.getNameFromPropertyName(methodDefinition.key); const methodName = util.getNameFromClassMember( methodDefinition, sourceCode,