Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): [no-useless-default-assignment] add rule#11720
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
ulrichstark wants to merge20 commits intotypescript-eslint:mainChoose a base branch
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
+623 −0
Open
Changes fromall commits
Commits
Show all changes
20 commits Select commitHold shift + click to select a range
a97e52e feat(eslint-plugin): [no-useless-default-assignment] add rule
ulrichstark4a8ef6b support function parameters without type annotation
ulrichstark7637497 fix false positives
ulrichstark7a7b334 fixing more false positives
ulrichstark527b367 enable rule in repo to dogfood
ulrichstark010c166 cleanup code and increase code coverage
ulrichstarkff7bd0b cleanup code and add column numbers to tests
ulrichstark8d29d31 add tests to increase test coverage
ulrichstarkdf51d1c remove option allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing
ulrichstark0526062 remove comments at each test case
ulrichstark90d121a remove rule from rulesWithComplexOptionHeadings
ulrichstark9b55fb2 update snapshot
ulrichstark422a704 Update packages/eslint-plugin/docs/rules/no-useless-default-assignmen…
ulrichstark16ff0d6 Update packages/eslint-plugin/docs/rules/no-useless-default-assignmen…
ulrichstark4c40bfb add rule to strict config
ulrichstarkdef0e8a Merge branch 'main' into feat(eslint-plugin)---no-useless-default-ass…
ulrichstark6bef7fa turn suggestion into fix
ulrichstark299249e add more test cases and make them pass
ulrichstark242883a make example code easier to understand and add tuple example
ulrichstark7d7a636 increase test coverage
ulrichstarkFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
1 change: 1 addition & 0 deletionseslint.config.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletionspackages/eslint-plugin/docs/rules/no-useless-default-assignment.mdx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| --- | ||
| description: 'Disallow default values that will never be used.' | ||
| --- | ||
| import Tabs from '@theme/Tabs'; | ||
| import TabItem from '@theme/TabItem'; | ||
| > 🛑 This file is source code, not the primary documentation location! 🛑 | ||
| > | ||
| > See **https://typescript-eslint.io/rules/no-useless-default-assignment** for documentation. | ||
| [Default parameters](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters) and [default values](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#default_value) are only used if the parameter or property is `undefined`. | ||
| That can happen when a value is missing, or when one is provided and set to `undefined`. | ||
| If a non-`undefined` value is guaranteed to be provided, then there is no need to define a default. | ||
| ## Examples | ||
| <Tabs> | ||
| <TabItem value="❌ Incorrect"> | ||
| ```ts | ||
| function Bar({ foo = '' }: { foo: string }) { | ||
| return foo; | ||
| } | ||
| const { foo = '' } = { foo: 'bar' }; | ||
| const [foo = ''] = ['bar']; | ||
| [1, 2, 3].map((a = 42) => a + 1); | ||
| ``` | ||
| </TabItem> | ||
| <TabItem value="✅ Correct"> | ||
| ```ts | ||
| function Bar({ foo = '' }: { foo?: string }) { | ||
| return foo; | ||
| } | ||
| const { foo = '' } = { foo: undefined }; | ||
| const [foo = ''] = [undefined]; | ||
| [1, 2, 3, undefined].map((a = 42) => a + 1); | ||
| ``` | ||
| </TabItem> | ||
| </Tabs> | ||
| ## When Not To Use It | ||
| If your codebase is still onboarding to TypeScript and/or has many existing `any`s or areas of loosely typed code, it may be difficult to enable this rule. | ||
| You might consider using ESLint disable comments for those specific situations instead of completely disabling this rule. |
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/eslintrc/all.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/eslintrc/disable-type-checked.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/eslintrc/strict-type-checked-only.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/eslintrc/strict-type-checked.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/flat/all.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/flat/disable-type-checked.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/flat/strict-type-checked-only.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletionspackages/eslint-plugin/src/configs/flat/strict-type-checked.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletionspackages/eslint-plugin/src/rules/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
252 changes: 252 additions & 0 deletionspackages/eslint-plugin/src/rules/no-useless-default-assignment.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,252 @@ | ||
| import type { TSESTree } from '@typescript-eslint/utils'; | ||
| import { AST_NODE_TYPES } from '@typescript-eslint/utils'; | ||
| import * as tsutils from 'ts-api-utils'; | ||
| import * as ts from 'typescript'; | ||
| import { | ||
| createRule, | ||
| getParserServices, | ||
| isTypeAnyType, | ||
| isTypeFlagSet, | ||
| isTypeUnknownType, | ||
| } from '../util'; | ||
| type MessageId = 'uselessDefaultAssignment'; | ||
| export default createRule<[], MessageId>({ | ||
| name: 'no-useless-default-assignment', | ||
| meta: { | ||
| type: 'suggestion', | ||
| docs: { | ||
| description: 'Disallow default values that will never be used', | ||
| recommended: 'strict', | ||
| requiresTypeChecking: true, | ||
ulrichstark marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| }, | ||
| fixable: 'code', | ||
| messages: { | ||
| uselessDefaultAssignment: | ||
| 'Default value is useless because the {{ type }} is not optional.', | ||
| }, | ||
| schema: [ | ||
| { | ||
| type: 'object', | ||
| additionalProperties: false, | ||
| properties: {}, | ||
| }, | ||
| ], | ||
| }, | ||
| defaultOptions: [], | ||
| create(context) { | ||
| const services = getParserServices(context); | ||
| const checker = services.program.getTypeChecker(); | ||
| function canBeUndefined(type: ts.Type): boolean { | ||
| // any and unknown can be undefined | ||
| if (isTypeAnyType(type) || isTypeUnknownType(type)) { | ||
| return true; | ||
| } | ||
| // Check if any part of the union includes undefined | ||
| return tsutils | ||
| .unionConstituents(type) | ||
| .some(part => isTypeFlagSet(part, ts.TypeFlags.Undefined)); | ||
| } | ||
| function getPropertyType( | ||
| objectType: ts.Type, | ||
| propertyName: string, | ||
| ): ts.Type | null { | ||
| const symbol = objectType.getProperty(propertyName); | ||
| if (!symbol) { | ||
| return null; | ||
| } | ||
| return checker.getTypeOfSymbol(symbol); | ||
| } | ||
| function getArrayElementType( | ||
| arrayType: ts.Type, | ||
| elementIndex: number, | ||
| ): ts.Type | null { | ||
| if (checker.isTupleType(arrayType)) { | ||
| const tupleArgs = checker.getTypeArguments(arrayType); | ||
| if (elementIndex < tupleArgs.length) { | ||
| return tupleArgs[elementIndex]; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
| function isCallbackFunction( | ||
| functionNode: | ||
| | TSESTree.ArrowFunctionExpression | ||
| | TSESTree.FunctionExpression, | ||
| ): boolean { | ||
| const parentType = functionNode.parent.type; | ||
| return ( | ||
| parentType !== AST_NODE_TYPES.MethodDefinition && | ||
| parentType !== AST_NODE_TYPES.VariableDeclarator && | ||
| parentType !== AST_NODE_TYPES.Property && | ||
| parentType !== AST_NODE_TYPES.ExpressionStatement && | ||
| parentType !== AST_NODE_TYPES.ReturnStatement | ||
| ); | ||
| } | ||
| function checkAssignmentPattern(node: TSESTree.AssignmentPattern): void { | ||
| const parent = node.parent; | ||
| // Handle callback parameters (like array.map((a = 42) => ...)) | ||
| if ( | ||
| parent.type === AST_NODE_TYPES.ArrowFunctionExpression || | ||
| parent.type === AST_NODE_TYPES.FunctionExpression | ||
| ) { | ||
| const paramIndex = parent.params.indexOf(node); | ||
| if (paramIndex !== -1) { | ||
| // Only check if this is actually a callback, not a regular function | ||
| if (!isCallbackFunction(parent)) { | ||
| return; | ||
| } | ||
| const tsFunc = services.esTreeNodeToTSNodeMap.get(parent); | ||
| if (ts.isFunctionLike(tsFunc)) { | ||
| const signature = checker.getSignatureFromDeclaration(tsFunc); | ||
| if (signature) { | ||
| const params = signature.getParameters(); | ||
| if (paramIndex < params.length) { | ||
| const paramType = checker.getTypeOfSymbol(params[paramIndex]); | ||
| if (!canBeUndefined(paramType)) { | ||
| reportUselessDefault(node, 'parameter'); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return; | ||
| } | ||
| // Handle destructuring patterns | ||
| if (parent.type === AST_NODE_TYPES.Property) { | ||
| // This is a property in an object destructuring pattern | ||
| const objectPattern = parent.parent as TSESTree.ObjectPattern; | ||
| const sourceType = getSourceTypeForPattern(objectPattern); | ||
| if (!sourceType) { | ||
| return; | ||
| } | ||
| const propertyName = getPropertyName(parent.key); | ||
| if (!propertyName) { | ||
| return; | ||
| } | ||
| const propertyType = getPropertyType(sourceType, propertyName); | ||
| if (!propertyType) { | ||
| return; | ||
| } | ||
| if (!canBeUndefined(propertyType)) { | ||
| reportUselessDefault(node, 'property'); | ||
| } | ||
| } else if (parent.type === AST_NODE_TYPES.ArrayPattern) { | ||
| // This is an element in an array destructuring pattern | ||
| const sourceType = getSourceTypeForPattern(parent); | ||
| if (!sourceType) { | ||
| return; | ||
| } | ||
| const elementIndex = parent.elements.indexOf(node); | ||
| if (elementIndex === -1) { | ||
| return; | ||
| } | ||
| const elementType = getArrayElementType(sourceType, elementIndex); | ||
| if (!elementType) { | ||
| return; | ||
| } | ||
| if (!canBeUndefined(elementType)) { | ||
| reportUselessDefault(node, 'property'); | ||
| } | ||
| } | ||
| } | ||
| function getSourceTypeForPattern( | ||
| pattern: TSESTree.ArrayPattern | TSESTree.ObjectPattern, | ||
| ): ts.Type | null { | ||
| let currentNode: TSESTree.Node = pattern; | ||
| // Walk up through nested patterns | ||
| while ( | ||
| currentNode.parent.type === AST_NODE_TYPES.AssignmentPattern || | ||
| currentNode.parent.type === AST_NODE_TYPES.Property || | ||
| currentNode.parent.type === AST_NODE_TYPES.ObjectPattern || | ||
| currentNode.parent.type === AST_NODE_TYPES.ArrayPattern | ||
| ) { | ||
| currentNode = currentNode.parent; | ||
| } | ||
| const parent = currentNode.parent; | ||
| // Handle variable declarator | ||
| if (parent.type === AST_NODE_TYPES.VariableDeclarator && parent.init) { | ||
| const tsNode = services.esTreeNodeToTSNodeMap.get(parent.init); | ||
| return checker.getTypeAtLocation(tsNode); | ||
| } | ||
| // Handle function parameter | ||
| if ( | ||
| parent.type === AST_NODE_TYPES.FunctionExpression || | ||
| parent.type === AST_NODE_TYPES.ArrowFunctionExpression || | ||
| parent.type === AST_NODE_TYPES.FunctionDeclaration | ||
| ) { | ||
| const paramIndex = parent.params.indexOf( | ||
| currentNode as TSESTree.Parameter, | ||
| ); | ||
| const tsFunc = services.esTreeNodeToTSNodeMap.get(parent); | ||
| const signature = checker.getSignatureFromDeclaration(tsFunc); | ||
| if (!signature) { | ||
| return null; | ||
| } | ||
| const params = signature.getParameters(); | ||
| return checker.getTypeOfSymbol(params[paramIndex]); | ||
| } | ||
| return null; | ||
| } | ||
| function getPropertyName( | ||
| key: TSESTree.Expression | TSESTree.PrivateIdentifier, | ||
| ): string | null { | ||
| switch (key.type) { | ||
| case AST_NODE_TYPES.Identifier: | ||
| return key.name; | ||
| case AST_NODE_TYPES.Literal: | ||
| return String(key.value); | ||
| default: | ||
| return null; | ||
| } | ||
| } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. [Non-Actionable] I'm surprised a utility doesn't already exist for this - but I couldn't find one.
Not requesting changes, just noting - maybe I missed one? | ||
| function reportUselessDefault( | ||
| node: TSESTree.AssignmentPattern, | ||
| type: 'parameter' | 'property', | ||
| ): void { | ||
| context.report({ | ||
| node: node.right, | ||
| messageId: 'uselessDefaultAssignment', | ||
| data: { type }, | ||
| fix(fixer) { | ||
| // Remove from before the = to the end of the default value | ||
| // Find the start position (including whitespace before =) | ||
| const start = node.left.range[1]; | ||
| const end = node.range[1]; | ||
| return fixer.removeRange([start, end]); | ||
| }, | ||
| }); | ||
| } | ||
| return { | ||
| AssignmentPattern: checkAssignmentPattern, | ||
| }; | ||
| }, | ||
| }); | ||
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.