diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index fcc422386b4..c560ebb6761 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -102,7 +102,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | -| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | | | +| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | :wrench: | | | [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | | | [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index ebfbae27329..7c49f0d6a31 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -1,6 +1,8 @@ import { AST_NODE_TYPES, TSESTree, + AST_TOKEN_TYPES, + TSESLint, } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; @@ -38,6 +40,7 @@ export default util.createRule({ // too opinionated to be recommended recommended: false, }, + fixable: 'code', messages: { missingAccessibility: 'Missing accessibility modifier on {{type}} {{name}}.', @@ -91,6 +94,7 @@ export default util.createRule({ nodeType: string, node: TSESTree.Node, nodeName: string, + fix: TSESLint.ReportFixFunction | null = null, ): void { context.report({ node: node, @@ -99,6 +103,7 @@ export default util.createRule({ type: nodeType, name: nodeName, }, + fix: fix, }); } @@ -140,6 +145,7 @@ export default util.createRule({ nodeType, methodDefinition, methodName, + getUnwantedPublicAccessibilityFixer(methodDefinition), ); } else if (check === 'explicit' && !methodDefinition.accessibility) { reportIssue( @@ -151,6 +157,47 @@ export default util.createRule({ } } + /** + * Creates a fixer that removes a "public" keyword with following spaces + */ + function getUnwantedPublicAccessibilityFixer( + node: + | TSESTree.MethodDefinition + | TSESTree.ClassProperty + | TSESTree.TSParameterProperty, + ): TSESLint.ReportFixFunction { + return function(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { + const tokens = sourceCode.getTokens(node); + let rangeToRemove: TSESLint.AST.Range; + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i]; + if ( + token.type === AST_TOKEN_TYPES.Keyword && + token.value === 'public' + ) { + const commensAfterPublicKeyword = sourceCode.getCommentsAfter( + token, + ); + if (commensAfterPublicKeyword.length) { + // public /* Hi there! */ static foo() + // ^^^^^^^ + rangeToRemove = [ + token.range[0], + commensAfterPublicKeyword[0].range[0], + ]; + break; + } else { + // public static foo() + // ^^^^^^^ + rangeToRemove = [token.range[0], tokens[i + 1].range[0]]; + break; + } + } + } + return fixer.removeRange(rangeToRemove!); + }; + } + /** * Checks if property has an accessibility modifier. * @param classProperty The node representing a ClassProperty. @@ -170,6 +217,7 @@ export default util.createRule({ nodeType, classProperty, propertyName, + getUnwantedPublicAccessibilityFixer(classProperty), ); } else if (propCheck === 'explicit' && !classProperty.accessibility) { reportIssue( @@ -217,6 +265,7 @@ export default util.createRule({ nodeType, node, nodeName, + getUnwantedPublicAccessibilityFixer(node), ); } break; 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 b5477c062a1..fe0a09dd4c1 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -344,6 +344,11 @@ export class XXXX { line: 3, }, ], + output: ` +export class XXXX { + public constructor(readonly value: string) {} +} + `, }, { filename: 'test.ts', @@ -354,6 +359,11 @@ export class WithParameterProperty { `, options: [{ accessibility: 'explicit' }], errors: [{ messageId: 'missingAccessibility' }], + output: ` +export class WithParameterProperty { + public constructor(readonly value: string) {} +} + `, }, { filename: 'test.ts', @@ -372,6 +382,11 @@ export class XXXX { }, ], errors: [{ messageId: 'missingAccessibility' }], + output: ` +export class XXXX { + public constructor(readonly samosa: string) {} +} + `, }, { filename: 'test.ts', @@ -387,6 +402,11 @@ class Test { }, ], errors: [{ messageId: 'missingAccessibility' }], + output: ` +class Test { + public constructor(readonly foo: string) {} +} + `, }, { filename: 'test.ts', @@ -409,6 +429,14 @@ class Test { column: 3, }, ], + output: ` +class Test { + x: number + public getX () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -431,6 +459,14 @@ class Test { column: 3, }, ], + output: ` +class Test { + private x: number + getX () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -462,6 +498,14 @@ class Test { column: 3, }, ], + output: ` +class Test { + x?: number + getX? () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -486,6 +530,15 @@ class Test { column: 3, }, ], + output: ` +class Test { + protected name: string + protected foo?: string + getX () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -510,6 +563,15 @@ class Test { column: 3, }, ], + output: ` +class Test { + protected name: string + foo?: string + getX () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -534,6 +596,14 @@ class Test { }, ], options: [{ accessibility: 'no-public' }], + output: ` +class Test { + x: number + getX () { + return this.x + } +} + `, }, { filename: 'test.ts', @@ -564,6 +634,20 @@ class Test { }, ], options: [{ overrides: { constructors: 'no-public' } }], + output: ` +class Test { + private x: number; + constructor (x: number) { + this.x = x; + } + get internalValue() { + return this.x; + } + set internalValue(value: number) { + this.x = value; + } +} + `, }, { filename: 'test.ts', @@ -598,6 +682,20 @@ class Test { column: 3, }, ], + output: ` +class Test { + private x: number; + constructor (x: number) { + this.x = x; + } + get internalValue() { + return this.x; + } + set internalValue(value: number) { + this.x = value; + } +} + `, }, { filename: 'test.ts', @@ -621,6 +719,14 @@ class Test { overrides: { parameterProperties: 'no-public' }, }, ], + output: ` +class Test { + constructor(public x: number){} + public foo(): string { + return 'foo'; + } +} + `, }, { filename: 'test.ts', @@ -636,6 +742,11 @@ class Test { column: 3, }, ], + output: ` +class Test { + constructor(public x: number){} +} + `, }, { filename: 'test.ts', @@ -657,6 +768,11 @@ class Test { column: 15, }, ], + output: ` +class Test { + constructor(readonly x: number){} +} + `, }, { filename: 'test.ts', @@ -674,6 +790,7 @@ class Test { column: 14, }, ], + output: 'class Test { x = 2 }', }, { filename: 'test.ts', @@ -694,6 +811,10 @@ class Test { column: 9, }, ], + output: `class Test { + x = 2 + private x = 2 + }`, }, { code: 'class Test { constructor(public ...x: any[]) { }}', @@ -705,6 +826,224 @@ class Test { column: 14, }, ], + output: 'class Test { constructor(public ...x: any[]) { }}', + }, + { + filename: 'test.ts', + code: ` +class Test { + @public + public /*public*/constructor(private foo: string) {} +} + `, + options: [ + { + accessibility: 'no-public', + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 3, + }, + ], + output: ` +class Test { + @public + /*public*/constructor(private foo: string) {} +} + `, + }, + { + filename: 'test.ts', + code: ` +class Test { + @public + public foo() {} +} + `, + options: [ + { + accessibility: 'no-public', + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 3, + }, + ], + output: ` +class Test { + @public + foo() {} +} + `, + }, + + { + filename: 'test.ts', + code: ` +class Test { + @public + public foo; +} + `, + options: [ + { + accessibility: 'no-public', + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 3, + }, + ], + output: ` +class Test { + @public + foo; +} + `, + }, + { + filename: 'test.ts', + code: ` +class Test { + public foo = ""; +} + `, + options: [ + { + accessibility: 'no-public', + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 3, + }, + ], + output: ` +class Test { + foo = ""; +} + `, + }, + + { + filename: 'test.ts', + code: ` +class Test { + contructor(public/* Hi there */ readonly foo) +} + `, + options: [ + { + accessibility: 'no-public', + overrides: { parameterProperties: 'no-public' }, + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 14, + }, + ], + output: ` +class Test { + contructor(/* Hi there */ readonly foo) +} + `, + }, + { + filename: 'test.ts', + code: ` +class Test { + contructor(public readonly foo: string) +} + `, + options: [ + { + accessibility: 'no-public', + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 3, + column: 14, + }, + ], + output: ` +class Test { + contructor(readonly foo: string) +} + `, + }, + { + filename: 'test.ts', + code: + 'class EnsureWhiteSPaceSpan { public constructor() {}}', + options: [ + { + accessibility: 'no-public', + overrides: { parameterProperties: 'no-public' }, + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 1, + column: 30, + }, + ], + output: 'class EnsureWhiteSPaceSpan { constructor() {}}', + }, + { + filename: 'test.ts', + code: + 'class EnsureWhiteSPaceSpan { public /* */ constructor() {}}', + options: [ + { + accessibility: 'no-public', + overrides: { parameterProperties: 'no-public' }, + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 1, + column: 30, + }, + ], + output: + 'class EnsureWhiteSPaceSpan { /* */ constructor() {}}', + }, + { + filename: 'test.ts', + code: + 'class EnsureWhiteSPaceSpan { public /* */ constructor() {}}', + options: [ + { + accessibility: 'no-public', + overrides: { parameterProperties: 'no-public' }, + }, + ], + errors: [ + { + messageId: 'unwantedPublicAccessibility', + line: 1, + column: 30, + }, + ], + output: 'class EnsureWhiteSPaceSpan { /* */ constructor() {}}', }, ], });