Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(eslint-plugin): [consistent-type-definitions] correct fix for export default #3899

Merged
merged 1 commit into from Sep 20, 2021
Merged

fix(eslint-plugin): [consistent-type-definitions] correct fix for export default #3899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/eslint-plugin/src/rules/consistent-type-definitions.ts
View file
Open in desktop
Expand Up @@ -122,6 +122,19 @@ export default util.createRule({
});
}

if (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on this fix, I noticed that the option is checked after having already found a certain node (and in some other files as well), for example:

typescript-eslint/packages/eslint-plugin/src/rules/consistent-type-definitions.ts

Lines 50 to 54 in 1e7e6d8

return {
"TSTypeAliasDeclaration[typeAnnotation.type='TSTypeLiteral']"(
node: TSESTree.TSTypeAliasDeclaration,
): void {
if (option === 'interface') {

Other cases I found (click to expand)

typescript-eslint/packages/eslint-plugin/src/rules/method-signature-style.ts

Lines 120 to 123 in b1d4449

TSMethodSignature(methodNode): void {
if (mode === 'method') {
return;
}

typescript-eslint/packages/eslint-plugin/src/rules/restrict-plus-operands.ts

Lines 136 to 137 in 1c1b572

"AssignmentExpression[operator='+=']"(node): void {
if (checkCompoundAssignments) {

typescript-eslint/packages/eslint-plugin/src/rules/sort-type-union-intersection-members.ts

Lines 242 to 244 in 25ea953

return {
TSIntersectionType(node): void {
if (checkIntersections === true) {

typescript-eslint/packages/eslint-plugin/src/rules/typedef.ts

Lines 180 to 183 in 1c1b572

'FunctionDeclaration, FunctionExpression'(
node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression,
): void {
if (options[OptionKeys.Parameter]) {

typescript-eslint/packages/eslint-plugin/src/rules/explicit-function-return-type.ts

Lines 71 to 76 in 1c1b572

return {
'ArrowFunctionExpression, FunctionExpression'(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
): void {
if (
options.allowConciseArrowFunctionExpressionsStartingWithVoid &&

But, IMHO this could be improved by either moving the conditional up then wrapping the return { or, in some cases do ...(optionX && { node ... }), and so avoiding looking for nodes which doesn't corresponds unnecessarily. Does it makes sense for you? If so, I can send a PR refactor for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to accept a PR if you want!

node.parent?.type ===
AST_NODE_TYPES.ExportDefaultDeclaration
) {
fixes.push(
fixer.removeRange([node.parent.range[0], node.range[0]]),
fixer.insertTextAfter(
node.body,
`\nexport default ${node.id.name}`,
),
);
}

return fixes;
},
});
Expand Down
26 changes: 25 additions & 1 deletion packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts
View file
Open in desktop
@@ -1,5 +1,5 @@
import rule from '../../src/rules/consistent-type-definitions';
import { RuleTester, noFormat } from '../RuleTester';
import { noFormat, RuleTester } from '../RuleTester';

const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
Expand Down Expand Up @@ -281,5 +281,29 @@ declare global {
},
],
},
{
// https://github.com/typescript-eslint/typescript-eslint/issues/3894
code: `
export default interface Test {
bar(): string;
foo(): number;
}
`,
output: noFormat`
type Test = {
bar(): string;
foo(): number;
}
export default Test
`,
options: ['type'],
errors: [
{
messageId: 'typeOverInterface',
line: 2,
column: 26,
},
],
},
],
});