-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(eslint-plugin): [consistent-generic-constructors] ignore when constructor is typed array #10477
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
2e75e31
34f390c
845a74b
d3fd541
7613753
d331b1f
013fd02
f3c37b1
82e9dfb
e32fec8
91b65ae
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 |
---|---|---|
|
@@ -4,8 +4,13 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils'; | |
|
||
import { createRule, nullThrows, NullThrowsReasons } from '../util'; | ||
|
||
export type MessageIds = 'preferConstructor' | 'preferTypeAnnotation'; | ||
export type Options = ['constructor' | 'type-annotation']; | ||
type MessageIds = 'preferConstructor' | 'preferTypeAnnotation'; | ||
type Options = [ | ||
'constructor' | 'type-annotation', | ||
{ | ||
ignore?: string[]; | ||
}?, | ||
]; | ||
|
||
export default createRule<Options, MessageIds>({ | ||
name: 'consistent-generic-constructors', | ||
|
@@ -29,10 +34,24 @@ export default createRule<Options, MessageIds>({ | |
description: 'Which constructor call syntax to prefer.', | ||
enum: ['type-annotation', 'constructor'], | ||
}, | ||
{ | ||
type: 'object', | ||
additionalProperties: false, | ||
properties: { | ||
ignore: { | ||
type: 'array', | ||
description: | ||
'A list of constructor names to ignore when enforcing the rule.', | ||
items: { | ||
type: 'string', | ||
}, | ||
}, | ||
}, | ||
}, | ||
], | ||
}, | ||
defaultOptions: ['constructor'], | ||
create(context, [mode]) { | ||
defaultOptions: ['constructor', {}], | ||
create(context, [mode, options]) { | ||
return { | ||
'VariableDeclarator,PropertyDefinition,AccessorProperty,:matches(FunctionDeclaration,FunctionExpression) > AssignmentPattern'( | ||
node: | ||
|
@@ -77,7 +96,8 @@ export default createRule<Options, MessageIds>({ | |
lhs && | ||
(lhs.type !== AST_NODE_TYPES.TSTypeReference || | ||
lhs.typeName.type !== AST_NODE_TYPES.Identifier || | ||
lhs.typeName.name !== rhs.callee.name) | ||
lhs.typeName.name !== rhs.callee.name || | ||
options?.ignore?.includes(lhs.typeName.name)) | ||
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. Threading #10477 (comment):
It looks like we don't have consensus on this yet?
One concern with having default values in the default list is that it gets inconvenient to add to the list rather than replace it. If the default type includes, say, We're previously had options like @Josh-Cena I think what's confusing me here is:
|
||
) { | ||
return; | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
IMO if we do go with a list then we'd want to use TypeOrValueSpecifier. Otherwise it'll have the same issue as #10477 (comment) with coincidentally matching names.
But, I'm not sure this is the right step. I started a thread later in this file about it.