-
Notifications
You must be signed in to change notification settings - Fork 36k
Add eslint rule to prevent bracket syntax for property access #274420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||||||||||||||||||||||||||
| /*--------------------------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||||||||||||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||||||||||||||||||||||||||||||||||
| *--------------------------------------------------------------------------------------------*/ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import * as eslint from 'eslint'; | ||||||||||||||||||||||||||||||||||
| import { TSESTree } from '@typescript-eslint/utils'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Disallow bracket notation for accessing properties that are valid identifiers, | ||||||||||||||||||||||||||||||||||
| * especially private members (starting with underscore). Bracket notation should | ||||||||||||||||||||||||||||||||||
| * only be used for properties with special characters or computed property names. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Bad: obj['_privateMember'] | ||||||||||||||||||||||||||||||||||
| * Bad: obj['normalProperty'] | ||||||||||||||||||||||||||||||||||
| * Good: obj._privateMember // TypeScript will catch private access | ||||||||||||||||||||||||||||||||||
| * Good: obj.normalProperty | ||||||||||||||||||||||||||||||||||
| * Good: obj['property-with-dashes'] | ||||||||||||||||||||||||||||||||||
| * Good: obj[computedKey] | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| export = new class NoBracketNotationForIdentifiers implements eslint.Rule.RuleModule { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| readonly meta: eslint.Rule.RuleMetaData = { | ||||||||||||||||||||||||||||||||||
| type: 'problem', | ||||||||||||||||||||||||||||||||||
| docs: { | ||||||||||||||||||||||||||||||||||
| description: 'Disallow bracket notation for accessing properties that are valid identifiers' | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| messages: { | ||||||||||||||||||||||||||||||||||
| noBracketNotation: 'Use dot notation instead of bracket notation for property \'{{property}}\'. Bracket notation bypasses TypeScript\'s type checking and access modifiers.' | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| schema: [], | ||||||||||||||||||||||||||||||||||
| fixable: 'code' | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Check if a string is a valid JavaScript identifier | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| function isValidIdentifier(str: string): boolean { | ||||||||||||||||||||||||||||||||||
| // Check if it's a valid JavaScript identifier | ||||||||||||||||||||||||||||||||||
| // Must start with letter, underscore, or dollar sign | ||||||||||||||||||||||||||||||||||
| // Can contain letters, digits, underscores, or dollar signs | ||||||||||||||||||||||||||||||||||
| return /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(str); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| MemberExpression(node: any) { | ||||||||||||||||||||||||||||||||||
| const memberExpr = node as TSESTree.MemberExpression; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Only check computed member expressions (bracket notation) | ||||||||||||||||||||||||||||||||||
| if (!memberExpr.computed) { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Only check string literals in brackets | ||||||||||||||||||||||||||||||||||
| if (memberExpr.property.type !== 'Literal' || typeof memberExpr.property.value !== 'string') { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const propertyName = memberExpr.property.value; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // If it's a valid identifier, report it | ||||||||||||||||||||||||||||||||||
| if (isValidIdentifier(propertyName)) { | ||||||||||||||||||||||||||||||||||
| context.report({ | ||||||||||||||||||||||||||||||||||
| node: memberExpr.property, | ||||||||||||||||||||||||||||||||||
| messageId: 'noBracketNotation', | ||||||||||||||||||||||||||||||||||
| data: { | ||||||||||||||||||||||||||||||||||
| property: propertyName | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| fix(fixer) { | ||||||||||||||||||||||||||||||||||
| // Convert obj['property'] to obj.property | ||||||||||||||||||||||||||||||||||
| // We need to replace the ['property'] part with .property | ||||||||||||||||||||||||||||||||||
| const sourceCode = context.getSourceCode(); | ||||||||||||||||||||||||||||||||||
| const objectText = sourceCode.getText(memberExpr.object as unknown as eslint.Rule.Node); | ||||||||||||||||||||||||||||||||||
| const dotNotation = `${objectText}.${propertyName}`; | ||||||||||||||||||||||||||||||||||
| return fixer.replaceText(node, dotNotation); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+73
to
+77
|
||||||||||||||||||||||||||||||||||
| // We need to replace the ['property'] part with .property | |
| const sourceCode = context.getSourceCode(); | |
| const objectText = sourceCode.getText(memberExpr.object as unknown as eslint.Rule.Node); | |
| const dotNotation = `${objectText}.${propertyName}`; | |
| return fixer.replaceText(node, dotNotation); | |
| // Only replace the ['property'] part with .property, preserving whitespace/comments | |
| const sourceCode = context.getSourceCode(); | |
| const leftBracket = sourceCode.getTokenBefore(memberExpr.property, token => token.value === '['); | |
| const rightBracket = sourceCode.getTokenAfter(memberExpr.property, token => token.value === ']'); | |
| if (!leftBracket || !rightBracket) { | |
| return null; | |
| } | |
| return fixer.replaceTextRange( | |
| [leftBracket.range[0], rightBracket.range[1]], | |
| '.' + propertyName | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regular expression doesn't account for reserved JavaScript keywords. Properties like 'class', 'const', 'function', etc. are valid in bracket notation but cannot be used with dot notation. Add a check to exclude reserved keywords:
const reservedWords = new Set(['break', 'case', 'catch', 'class', 'const', 'continue', 'debugger', 'default', 'delete', 'do', 'else', 'export', 'extends', 'finally', 'for', 'function', 'if', 'import', 'in', 'instanceof', 'new', 'return', 'super', 'switch', 'this', 'throw', 'try', 'typeof', 'var', 'void', 'while', 'with', 'yield', 'let', 'static', 'enum', 'await', 'implements', 'interface', 'package', 'private', 'protected', 'public']); return /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(str) && !reservedWords.has(str);