From 9d3c8afa7e35bc23c298d6afcf5294233ce2b055 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sat, 6 Apr 2024 15:17:31 -0600 Subject: [PATCH 1/6] [explicit-member-accessibility] Refine report locations. --- .../rules/explicit-member-accessibility.ts | 162 +++++++++++++----- .../explicit-member-accessibility.test.ts | 122 +++++++++++-- 2 files changed, 231 insertions(+), 53 deletions(-) diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index 83591c7fa71d..79514ba8d67d 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -152,18 +152,19 @@ export default createRule({ check === 'no-public' && methodDefinition.accessibility === 'public' ) { + const publicKeyword = findPublicKeyword(methodDefinition); context.report({ - node: methodDefinition, + loc: rangeToLoc(context.sourceCode, publicKeyword.range), messageId: 'unwantedPublicAccessibility', data: { type: nodeType, name: methodName, }, - fix: getUnwantedPublicAccessibilityFixer(methodDefinition), + fix: fixer => fixer.removeRange(publicKeyword.rangeToRemove), }); } else if (check === 'explicit' && !methodDefinition.accessibility) { context.report({ - node: methodDefinition, + loc: getMissingAccessibilityReportLoc(methodDefinition), messageId: 'missingAccessibility', data: { type: nodeType, @@ -175,49 +176,117 @@ export default createRule({ } /** - * Creates a fixer that removes a "public" keyword with following spaces + * Returns an object containing a range that corresponds to the "public" + * keyword for a node, and the range that would need to be removed to + * remove the "public" keyword (including associated whitespace). */ - function getUnwantedPublicAccessibilityFixer( + function findPublicKeyword( node: | TSESTree.MethodDefinition | TSESTree.PropertyDefinition | TSESTree.TSAbstractMethodDefinition | TSESTree.TSAbstractPropertyDefinition | TSESTree.TSParameterProperty, - ): TSESLint.ReportFixFunction { - return function (fixer: TSESLint.RuleFixer): TSESLint.RuleFix { - const tokens = context.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 = - context.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; - } + ): { range: TSESLint.AST.Range; rangeToRemove: TSESLint.AST.Range } { + const tokens = context.sourceCode.getTokens(node); + let rangeToRemove!: TSESLint.AST.Range; + let keywordRange!: 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' + ) { + keywordRange = structuredClone(token.range); + const commensAfterPublicKeyword = + context.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); + } + return { range: keywordRange, rangeToRemove }; + } + + /** + * For missing accessibility modifiers, we want to report any keywords + * out in front of the key, and the key itself, but not anything afterwards, + * i.e. parens, type annotations, method bodies, or `?`. + */ + function getMissingAccessibilityReportLoc( + node: + | TSESTree.MethodDefinition + | TSESTree.PropertyDefinition + | TSESTree.TSAbstractPropertyDefinition, + ): TSESTree.SourceLocation { + let start: TSESTree.Position; + + if (node.decorators.length === 0) { + start = structuredClone(node.loc.start); + } else { + const lastDecorator = node.decorators[node.decorators.length - 1]; + const nextToken = nullThrows( + context.sourceCode.getTokenAfter(lastDecorator), + NullThrowsReasons.MissingToken('token', 'last decorator'), + ); + start = structuredClone(nextToken.loc.start); + } + + return { + start, + end: structuredClone(node.key.loc.end), + }; + } + + /** + * For missing accessibility modifiers, we want to report any keywords + * out in front of the key, and the key itself, but not anything afterwards, + * i.e. parens, type annotations, method bodies, or `?`. + */ + function getMissingAccessibilityReportLocForParameterProperty( + node: TSESTree.TSParameterProperty, + nodeName: string, + ): TSESTree.SourceLocation { + // Parameter properties have a weirdly different AST structure + // than other class members. + + let start: TSESTree.Position; + + if (node.decorators.length === 0) { + start = structuredClone(node.loc.start); + } else { + const lastDecorator = node.decorators[node.decorators.length - 1]; + const nextToken = nullThrows( + context.sourceCode.getTokenAfter(lastDecorator), + NullThrowsReasons.MissingToken('token', 'last decorator'), + ); + start = structuredClone(nextToken.loc.start); + } + + const end = rangeToLoc(context.sourceCode, [ + node.parameter.range[0], + node.parameter.range[0] + nodeName.length, + ]).end; + + return { + start, + end, }; } /** - * Creates a fixer that adds a "public" keyword with following spaces + * Creates a fixer that adds an accessibility modifier keyword */ function getMissingAccessibilitySuggestions( node: @@ -284,21 +353,22 @@ export default createRule({ propCheck === 'no-public' && propertyDefinition.accessibility === 'public' ) { + const publicKeywordRange = findPublicKeyword(propertyDefinition); context.report({ - node: propertyDefinition, + loc: rangeToLoc(context.sourceCode, publicKeywordRange.range), messageId: 'unwantedPublicAccessibility', data: { type: nodeType, name: propertyName, }, - fix: getUnwantedPublicAccessibilityFixer(propertyDefinition), + fix: fixer => fixer.removeRange(publicKeywordRange.rangeToRemove), }); } else if ( propCheck === 'explicit' && !propertyDefinition.accessibility ) { context.report({ - node: propertyDefinition, + loc: getMissingAccessibilityReportLoc(propertyDefinition), messageId: 'missingAccessibility', data: { type: nodeType, @@ -335,7 +405,10 @@ export default createRule({ case 'explicit': { if (!node.accessibility) { context.report({ - node, + loc: getMissingAccessibilityReportLocForParameterProperty( + node, + nodeName, + ), messageId: 'missingAccessibility', data: { type: nodeType, @@ -348,14 +421,15 @@ export default createRule({ } case 'no-public': { if (node.accessibility === 'public' && node.readonly) { + const publicKeyword = findPublicKeyword(node); context.report({ - node, + loc: rangeToLoc(context.sourceCode, publicKeyword.range), messageId: 'unwantedPublicAccessibility', data: { type: nodeType, name: nodeName, }, - fix: getUnwantedPublicAccessibilityFixer(node), + fix: fixer => fixer.removeRange(publicKeyword.rangeToRemove), }); } break; @@ -372,3 +446,13 @@ export default createRule({ }; }, }); + +function rangeToLoc( + sourceCode: TSESLint.SourceCode, + range: TSESLint.AST.Range, +): TSESTree.SourceLocation { + return { + start: sourceCode.getLocFromIndex(range[0]), + end: sourceCode.getLocFromIndex(range[1]), + }; +} 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 d682c711fcce..36f4cee1f7c5 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -339,8 +339,10 @@ export class XXXX { errors: [ { messageId: 'missingAccessibility', - column: 22, line: 3, + column: 22, + endLine: 3, + endColumn: 36, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -383,6 +385,10 @@ export class WithParameterProperty { errors: [ { messageId: 'missingAccessibility', + line: 3, + column: 22, + endLine: 3, + endColumn: 36, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -433,6 +439,10 @@ export class XXXX { errors: [ { messageId: 'missingAccessibility', + line: 3, + column: 22, + endLine: 3, + endColumn: 37, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -480,6 +490,10 @@ class Test { errors: [ { messageId: 'missingAccessibility', + line: 3, + column: 22, + endLine: 3, + endColumn: 34, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -530,6 +544,8 @@ class Test { }, line: 3, column: 3, + endLine: 3, + endColumn: 4, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -592,6 +608,8 @@ class Test { }, line: 4, column: 3, + endLine: 4, + endColumn: 7, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -651,6 +669,8 @@ class Test { }, line: 3, column: 3, + endLine: 3, + endColumn: 4, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -698,6 +718,8 @@ class Test { }, line: 4, column: 3, + endLine: 4, + endColumn: 7, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -759,6 +781,8 @@ class Test { }, line: 5, column: 3, + endLine: 5, + endColumn: 9, }, ], output: ` @@ -791,6 +815,8 @@ class Test { }, line: 4, column: 3, + endLine: 4, + endColumn: 9, }, ], output: ` @@ -817,11 +843,15 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, { messageId: 'unwantedPublicAccessibility', line: 4, column: 3, + endLine: 4, + endColumn: 9, }, ], options: [{ accessibility: 'no-public' }], @@ -854,6 +884,8 @@ class Test { messageId: 'missingAccessibility', line: 7, column: 3, + endLine: 7, + endColumn: 20, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -915,6 +947,8 @@ class Test { messageId: 'missingAccessibility', line: 10, column: 3, + endLine: 10, + endColumn: 20, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -995,6 +1029,8 @@ class Test { messageId: 'missingAccessibility', line: 4, column: 3, + endLine: 4, + endColumn: 14, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1056,6 +1092,8 @@ class Test { messageId: 'missingAccessibility', line: 7, column: 3, + endLine: 7, + endColumn: 20, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1117,6 +1155,8 @@ class Test { messageId: 'missingAccessibility', line: 10, column: 3, + endLine: 10, + endColumn: 20, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1190,6 +1230,8 @@ class Test { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 14, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1247,6 +1289,8 @@ class Test { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 14, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1296,6 +1340,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 15, + endLine: 3, + endColumn: 21, }, ], output: ` @@ -1321,6 +1367,8 @@ class Test { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 4, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1371,6 +1419,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1383,7 +1433,7 @@ class Test { { code: ` class Test { - constructor(public ...x: any[]) {} + constructor(public x: any[]) {} } `, options: [{ accessibility: 'explicit' }], @@ -1392,13 +1442,15 @@ class Test { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 14, suggestions: [ { messageId: 'addExplicitAccessibility', data: { type: 'public' }, output: ` class Test { - public constructor(public ...x: any[]) {} + public constructor(public x: any[]) {} } `, }, @@ -1407,7 +1459,7 @@ class Test { data: { type: 'private' }, output: ` class Test { - private constructor(public ...x: any[]) {} + private constructor(public x: any[]) {} } `, }, @@ -1416,7 +1468,7 @@ class Test { data: { type: 'protected' }, output: ` class Test { - protected constructor(public ...x: any[]) {} + protected constructor(public x: any[]) {} } `, }, @@ -1440,6 +1492,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1463,8 +1517,10 @@ class Test { errors: [ { messageId: 'unwantedPublicAccessibility', - line: 3, + line: 4, column: 3, + endLine: 4, + endColumn: 9, }, ], output: ` @@ -1474,7 +1530,6 @@ class Test { } `, }, - { code: ` class Test { @@ -1490,8 +1545,10 @@ class Test { errors: [ { messageId: 'unwantedPublicAccessibility', - line: 3, + line: 4, column: 3, + endLine: 4, + endColumn: 9, }, ], output: ` @@ -1517,6 +1574,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1525,7 +1584,6 @@ class Test { } `, }, - { code: noFormat` class Test { @@ -1543,6 +1601,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 15, + endLine: 3, + endColumn: 21, }, ], output: ` @@ -1567,6 +1627,8 @@ class Test { messageId: 'unwantedPublicAccessibility', line: 3, column: 15, + endLine: 3, + endColumn: 21, }, ], output: ` @@ -1592,6 +1654,8 @@ class EnsureWhiteSPaceSpan { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1617,6 +1681,8 @@ class EnsureWhiteSPaceSpan { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1645,6 +1711,8 @@ class Test { }, line: 3, column: 3, + endLine: 3, + endColumn: 9, }, { messageId: 'unwantedPublicAccessibility', @@ -1654,6 +1722,8 @@ class Test { }, line: 4, column: 3, + endLine: 4, + endColumn: 9, }, { messageId: 'unwantedPublicAccessibility', @@ -1663,6 +1733,8 @@ class Test { }, line: 5, column: 3, + endLine: 5, + endColumn: 9, }, { messageId: 'unwantedPublicAccessibility', @@ -1672,6 +1744,8 @@ class Test { }, line: 6, column: 3, + endLine: 6, + endColumn: 9, }, ], output: ` @@ -1695,6 +1769,8 @@ abstract class SomeClass { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 18, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1744,6 +1820,8 @@ abstract class SomeClass { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1765,6 +1843,8 @@ abstract class SomeClass { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 13, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1814,6 +1894,8 @@ abstract class SomeClass { messageId: 'unwantedPublicAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 9, }, ], output: ` @@ -1845,6 +1927,8 @@ class DecoratedClass { messageId: 'missingAccessibility', line: 3, column: 3, + endLine: 3, + endColumn: 14, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1914,7 +1998,9 @@ class DecoratedClass { { messageId: 'missingAccessibility', line: 3, - column: 15, + column: 27, + endLine: 3, + endColumn: 39, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -1984,7 +2070,9 @@ class DecoratedClass { { messageId: 'missingAccessibility', line: 4, - column: 3, + column: 15, + endLine: 4, + endColumn: 16, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -2054,7 +2142,9 @@ class DecoratedClass { { messageId: 'missingAccessibility', line: 5, - column: 3, + column: 15, + endLine: 5, + endColumn: 19, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -2123,8 +2213,10 @@ class DecoratedClass { }, { messageId: 'missingAccessibility', - line: 8, + line: 10, column: 3, + endLine: 10, + endColumn: 8, suggestions: [ { messageId: 'addExplicitAccessibility', @@ -2194,7 +2286,9 @@ class DecoratedClass { { messageId: 'missingAccessibility', line: 13, - column: 3, + column: 15, + endLine: 13, + endColumn: 20, suggestions: [ { messageId: 'addExplicitAccessibility', From 82d5a2e5347c029c9a00899e84929c4d44dcd2e0 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 9 Apr 2024 12:25:39 -0500 Subject: [PATCH 2/6] fix snapshot --- .../explicit-member-accessibility.shot | 65 +++++-------------- 1 file changed, 17 insertions(+), 48 deletions(-) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/explicit-member-accessibility.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/explicit-member-accessibility.shot index ee066bed4686..ad4cf9847d1d 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/explicit-member-accessibility.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/explicit-member-accessibility.shot @@ -5,37 +5,26 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { constructor(name) { - ~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on method definition constructor. + ~~~~~~~~~~~ Missing accessibility modifier on method definition constructor. // No accessibility modifier -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ this.animalName = name; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ animalName: string; // No accessibility modifier - ~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on class property animalName. + ~~~~~~~~~~ Missing accessibility modifier on class property animalName. get name(): string { - ~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on get property accessor name. + ~~~~~~~~ Missing accessibility modifier on get property accessor name. // No accessibility modifier -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ return this.animalName; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ set name(value: string) { - ~~~~~~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on set property accessor name. + ~~~~~~~~ Missing accessibility modifier on set property accessor name. // No accessibility modifier -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ this.animalName = value; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ walk() { - ~~~~~~~~ Missing accessibility modifier on method definition walk. + ~~~~ Missing accessibility modifier on method definition walk. // method -~~~~~~~~~~~~~ } -~~~ } " `; @@ -53,21 +42,15 @@ class Animal { } private animalName: string; // Property get name(): string { - ~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on get property accessor name. + ~~~~~~~~ Missing accessibility modifier on get property accessor name. // get accessor -~~~~~~~~~~~~~~~~~~~ return this.animalName; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ set name(value: string) { - ~~~~~~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on set property accessor name. + ~~~~~~~~ Missing accessibility modifier on set property accessor name. // set accessor -~~~~~~~~~~~~~~~~~~~ this.animalName = value; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ public walk() { // method } @@ -80,43 +63,29 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { public constructor( - ~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on method definition constructor. + ~~~~~~ Public accessibility modifier on method definition constructor. public breed, -~~~~~~~~~~~~~~~~~ name, -~~~~~~~~~ ) { -~~~~~ // Parameter property and constructor -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ this.animalName = name; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ public animalName: string; // Property - ~~~~~~~~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on class property animalName. + ~~~~~~ Public accessibility modifier on class property animalName. public get name(): string { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on get property accessor name. + ~~~~~~ Public accessibility modifier on get property accessor name. // get accessor -~~~~~~~~~~~~~~~~~~~ return this.animalName; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ public set name(value: string) { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on set property accessor name. + ~~~~~~ Public accessibility modifier on set property accessor name. // set accessor -~~~~~~~~~~~~~~~~~~~ this.animalName = value; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~~~ public walk() { - ~~~~~~~~~~~~~~~ Public accessibility modifier on method definition walk. + ~~~~~~ Public accessibility modifier on method definition walk. // method -~~~~~~~~~~~~~ } -~~~ } " `; @@ -153,7 +122,7 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { public constructor(protected animalName) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on method definition constructor. + ~~~~~~ Public accessibility modifier on method definition constructor. public get name() { return this.animalName; } @@ -185,7 +154,7 @@ class Animal { this.animalName = value; } legs: number; - ~~~~~~~~~~~~~ Missing accessibility modifier on class property legs. + ~~~~ Missing accessibility modifier on class property legs. private hasFleas: boolean; } " @@ -213,7 +182,7 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { constructor(readonly animalName: string) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on parameter property animalName. + ~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on parameter property animalName. } " `; @@ -240,7 +209,7 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { constructor(public readonly animalName: string) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Public accessibility modifier on parameter property animalName. + ~~~~~~ Public accessibility modifier on parameter property animalName. } " `; @@ -259,7 +228,7 @@ exports[`Validating rule docs explicit-member-accessibility.mdx code examples ES class Animal { constructor(protected animalName) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Missing accessibility modifier on method definition constructor. + ~~~~~~~~~~~ Missing accessibility modifier on method definition constructor. public get name() { return this.animalName; } From ada6422896f02d365b6ec3896b65c6f8b649c1e7 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 9 Apr 2024 15:23:24 -0500 Subject: [PATCH 3/6] fix types --- .../eslint-plugin/src/rules/explicit-member-accessibility.ts | 5 ++++- 1 file changed, 4 insertions(+), 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 79514ba8d67d..694dda7ec53b 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -117,7 +117,9 @@ export default createRule({ * @param methodDefinition The node representing a MethodDefinition. */ function checkMethodAccessibilityModifier( - methodDefinition: TSESTree.MethodDefinition, + methodDefinition: + | TSESTree.MethodDefinition + | TSESTree.TSAbstractMethodDefinition, ): void { if (methodDefinition.key.type === AST_NODE_TYPES.PrivateIdentifier) { return; @@ -227,6 +229,7 @@ export default createRule({ function getMissingAccessibilityReportLoc( node: | TSESTree.MethodDefinition + | TSESTree.TSAbstractMethodDefinition | TSESTree.PropertyDefinition | TSESTree.TSAbstractPropertyDefinition, ): TSESTree.SourceLocation { From 89717bda473f4cfe800dcd4b1ed85fedf2360e7f Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 9 Apr 2024 18:14:19 -0600 Subject: [PATCH 4/6] computed names --- .../rules/explicit-member-accessibility.ts | 23 ++++++++-- .../explicit-member-accessibility.test.ts | 46 +++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index 694dda7ec53b..6c3a9d307aea 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -236,19 +236,34 @@ export default createRule({ let start: TSESTree.Position; if (node.decorators.length === 0) { - start = structuredClone(node.loc.start); + start = node.loc.start; } else { const lastDecorator = node.decorators[node.decorators.length - 1]; const nextToken = nullThrows( context.sourceCode.getTokenAfter(lastDecorator), NullThrowsReasons.MissingToken('token', 'last decorator'), ); - start = structuredClone(nextToken.loc.start); + start = nextToken.loc.start; + } + + let end: TSESTree.Position; + + if (!node.computed) { + end = node.key.loc.end; + } else { + const closingBracket = nullThrows( + context.sourceCode.getTokenAfter( + node.key, + token => token.value === ']', + ), + NullThrowsReasons.MissingToken(']', node.type), + ); + end = closingBracket.loc.end; } return { - start, - end: structuredClone(node.key.loc.end), + start: structuredClone(start), + end: structuredClone(end), }; } 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 36f4cee1f7c5..df4acf49b4d9 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -2350,6 +2350,52 @@ class DecoratedClass { @foo @bar() protected set z(@foo @bar() value: x) { this.x = x; } +} + `, + }, + ], + }, + ], + }, + { + code: ` +abstract class SomeClass { + abstract ['computed-method-name'](): string; +} + `, + options: [{ accessibility: 'explicit' }], + errors: [ + { + messageId: 'missingAccessibility', + line: 3, + column: 3, + endLine: 3, + endColumn: 36, + suggestions: [ + { + messageId: 'addExplicitAccessibility', + data: { type: 'public' }, + output: ` +abstract class SomeClass { + public abstract ['computed-method-name'](): string; +} + `, + }, + { + messageId: 'addExplicitAccessibility', + data: { type: 'private' }, + output: ` +abstract class SomeClass { + private abstract ['computed-method-name'](): string; +} + `, + }, + { + messageId: 'addExplicitAccessibility', + data: { type: 'protected' }, + output: ` +abstract class SomeClass { + protected abstract ['computed-method-name'](): string; } `, }, From f2de27aff5fbe69038316474518ff872b7c0a2ab Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 9 Apr 2024 18:20:08 -0600 Subject: [PATCH 5/6] simplify slightly --- .../eslint-plugin/src/rules/explicit-member-accessibility.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index 6c3a9d307aea..a0e11f5cb69e 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -292,10 +292,9 @@ export default createRule({ start = structuredClone(nextToken.loc.start); } - const end = rangeToLoc(context.sourceCode, [ - node.parameter.range[0], + const end = context.sourceCode.getLocFromIndex( node.parameter.range[0] + nodeName.length, - ]).end; + ); return { start, From 602de2a418eed3155250f6544b072d076a16cf6c Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 2 May 2024 22:44:42 -0600 Subject: [PATCH 6/6] missing output --- .../tests/rules/explicit-member-accessibility.test.ts | 1 + 1 file changed, 1 insertion(+) 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 53e6b4a76f0b..4b47ebe7826a 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -2379,6 +2379,7 @@ abstract class SomeClass { abstract ['computed-method-name'](): string; } `, + output: null, options: [{ accessibility: 'explicit' }], errors: [ {