8000 fix(eslint-plugin): [prefer-function-type] apply existing comments to… · arminyahya/typescript-eslint@639df14 · GitHub
[go: up one dir, main page]
Skip to content

Commit 639df14

Browse files
arminyahyaarminyahya
committed
fix(eslint-plugin): [prefer-function-type] apply existing comments to the fixed code
1 parent 509a117 commit 639df14

File tree

2 files changed

+195
-58
lines changed

2 files changed

+195
-58
lines changed

‎packages/eslint-plugin/src/rules/prefer-function-type.ts

Lines changed: 85 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
AST_NODE_TYPES,
33
AST_TOKEN_TYPES,
4+
TSESLint,
45
TSESTree,
56
} from '@typescript-eslint/experimental-utils';
67
import * as util from '../util';
@@ -69,46 +70,6 @@ export default util.createRule({
6970
}
7071
}
7172

72-
/**
73-
* @param call The call signature causing the diagnostic
74-
* @param parent The parent of the call
75-
* @returns The suggestion to report
76-
*/
77-
function renderSuggestion(
78-
call:
79-
| TSESTree.TSCallSignatureDeclaration
80-
| TSESTree.TSConstructSignatureDeclaration,
81-
parent: TSESTree.Node,
82-
): string {
83-
const start = call.range[0];
84-
const colonPos = call.returnType!.range[0] - start;
85-
const text = sourceCode.getText().slice(start, call.range[1]);
86-
87-
let suggestion = `${text.slice(0, colonPos)} =>${text.slice(
88-
colonPos + 1,
89-
)}`;
90-
91-
const lastChar = suggestion.endsWith(';') ? ';' : '';
92-
if (lastChar) {
93-
suggestion = suggestion.slice(0, -1);
94-
}
95-
if (shouldWrapSuggestion(parent.parent)) {
96-
suggestion = `(${suggestion})`;
97-
}
98-
if (parent.type === AST_NODE_TYPES.TSInterfaceDeclaration) {
99-
if (typeof parent.typeParameters !== 'undefined') {
100-
return `type ${sourceCode
101-
.getText()
102-
.slice(
103-
parent.id.range[0],
104-
parent.typeParameters.range[1],
105-
)} = ${suggestion}${lastChar}`;
106-
}
107-
return `type ${parent.id.name} = ${suggestion}${lastChar}`;
108-
}
109-
return suggestion;
110-
}
111-
11273
/**
11374
* @param member The TypeElement being checked
11475
* @param node The parent of member being checked
@@ -140,30 +101,97 @@ export default util.createRule({
140101
});
141102
return;
142103
}
143-
const suggestion = renderSuggestion(member, node);
144-
const fixStart =
145-
node.type === AST_NODE_TYPES.TSTypeLiteral
146-
? node.range[0]
147-
: sourceCode
148-
.getTokens(node)
149-
.filter(
150-
token =>
151-
token.type === AST_TOKEN_TYPES.Keyword &&
152-
token.value === 'interface',
153-
)[0].range[0];
154104

105+
const fixable =
106+
node.parent &&
107+
node.parent.type === AST_NODE_TYPES.ExportDefaultDeclaration;
155108
context.report({
156109
node: member,
157110
messageId: 'functionTypeOverCallableType',
158111
data: {
159112
literalOrInterface: phrases[node.type],
160113
},
161-
fix(fixer) {
162-
return fixer.replaceTextRange(
163-
[fixStart, node.range[1]],
164-
suggestion,
165-
);
166-
},
114+
fix: fixable
115+
? null
116+
: (fixer): TSESLint.RuleFix[] => {
117+
const fixes: TSESLint.RuleFix[] = [];
118+
const start = member.range[0];
119+
const colonPos = member.returnType!.range[0] - start;
120+
const text = sourceCode.getText().slice(start, member.range[1]);
121+
const comments = sourceCode
122+
.getCommentsBefore(member)
123+
.concat(sourceCode.getCommentsAfter(member));
124+
let suggestion = `${text.slice(0, colonPos)} =>${text.slice(
125+
colonPos + 1,
126+
)}`;
127+
const lastChar = suggestion.endsWith(';') ? ';' : '';
128+
if (lastChar) {
129+
suggestion = suggestion.slice(0, -1);
130+
}
131+
if (shouldWrapSuggestion(node.parent)) {
132+
suggestion = `(${suggestion})`;
133+
}
134+
135+
if (node.type === AST_NODE_TYPES.TSInterfaceDeclaration) {
136+
if (typeof node.typeParameters !== 'undefined') {
137+
suggestion = `type ${sourceCode
138+
.getText()
139+
.slice(
140+
node.id.range[0],
141+
node.typeParameters.range[1],
142+
)} = ${suggestion}${lastChar}`;
143+
} else {
144+
suggestion = `type ${node.id.name} = ${suggestion}${lastChar}`;
145+
}
146+
}
147+
148+
const isParentExported =
149+
node.parent &&
150+
node.parent.type === AST_NODE_TYPES.ExportNamedDeclaration;
151+
152+
if (
153+
node.type === AST_NODE_TYPES.TSInterfaceDeclaration &&
154+
isParentExported
155+
) {
156+
const commentsText = comments.reduce((text, comment) => {
157+
return (
158+
text +
159+
(comment.type === AST_TOKEN_TYPES.Line
160+
? `//${comment.value}`
161+
: `/*${comment.value}*/`) +
162+
'\n'
163+
);
164+
}, '');
165+
// comments should move before export and not between export and interface declaration
166+
fixes.push(
167+
fixer.insertTextBefore(
168+
node.parent as TSESTree.Node | TSESTree.Token,
169+
commentsText,
170+
),
171+
);
172+
} else {
173+
comments.forEach(comment => {
174+
let commentText =
175+
comment.type === AST_TOKEN_TYPES.Line
176+
? `//${comment.value}`
177+
: `/*${comment.value}*/`;
178+
const isCommentOnTheSameLine =
179+
comment.loc.start.line === member.loc.start.line;
180+
if (!isCommentOnTheSameLine) {
181+
commentText += '\n';
182+
} else {
183+
commentText += ' ';
184+
}
185+
suggestion = commentText + suggestion;
186+
});
187+
}
188+
189+
const fixStart = node.range[0];
190+
fixes.push(
191+
fixer.replaceTextRange([fixStart, node.range[1]], suggestion),
192+
);
193+
return fixes;
194+
},
167195
});
168196
}
169197
}

‎packages/eslint-plugin/tests/rules/prefer-function-type.test.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,114 @@ interface Foo {
6565
type Foo = () => string;
6666
`,
6767
},
68+
// https://github.com/typescript-eslint/typescript-eslint/issues/3004
69+
{
70+
code: `
71+
export default interface Foo {
72+
/** comment */
73+
(): string;
74+
}
75+
`,
76+
errors: [
77+
{
78+
messageId: 'functionTypeOverCallableType',
79+
type: AST_NODE_TYPES.TSCallSignatureDeclaration,
80+
data: {
81+
literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration],
82+
},
83+
},
84+
],
85+
output: `
86+
export default interface Foo {
87+
/** comment */
88+
(): string;
89+
}
90+
`,
91+
},
92+
{
93+
code: `
94+
interface Foo {
95+
// comment
96+
(): string;
97+
}
98+
`,
99+
errors: [
100+
{
101+
messageId: 'functionTypeOverCallableType',
102+
type: AST_NODE_TYPES.TSCallSignatureDeclaration,
103+
data: {
104+
literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration],
105+
},
106+
},
107+
],
108+
output: `
109+
// comment
110+
type Foo = () => string;
111+
`,
112+
},
113+
{
114+
code: `
115+
export interface Foo {
116+
/** comment */
117+
(): string;
118+
}
119+
`,
120+
errors: [
121+
{
122+
messageId: 'functionTypeOverCallableType',
123+
type: AST_NODE_TYPES.TSCallSignatureDeclaration,
124+
data: {
125+
literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration],
126+
},
127+
},
128+
],
129+
output: `
130+
/** comment */
131+
export type Foo = () => string;
132+
`,
133+
},
134+
{
135+
code: `
136+
export interface Foo {
137+
// comment
138+
(): string;
139+
}
140+
`,
141+
errors: [
142+
{
143+
messageId: 'functionTypeOverCallableType',
144+
type: AST_NODE_TYPES.TSCallSignatureDeclaration,
145+
data: {
146+
literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration],
147+
},
148+
},
149+
],
150+
output: `
151+
// comment
152+
export type Foo = () => string;
153+
`,
154+
},
155+
{
156+
code: `
157+
function foo(bar: { /* comment */ (s: string): number } | undefined): number {
158+
return bar('hello');
159+
}
160+
`,
161+
errors: [
162+
{
163+
messageId: 'functionTypeOverCallableType',
164+
type: AST_NODE_TYPES.TSCallSignatureDeclaration,
165+
data: {
166+
literalOrInterface: phrases[AST_NODE_TYPES.TSTypeLiteral],
167+
},
168+
},
169+
],
170+
output: `
171+
function foo(bar: /* comment */ ((s: string) => number) | undefined): number {
172+
return bar('hello');
173+
}
174+
`,
175+
},
68176
{
69177
code: `
70178
type Foo = {
@@ -234,8 +342,8 @@ interface Foo {
234342
},
235343
{
236344
code: `
345+
// isn't actually valid ts but want to not give message saying it refers to Foo.
237346
interface Foo {
238-
// isn't actually valid ts but want to not give message saying it refers to Foo.
239347
(): {
240348
a: {
241349
nested: this;
@@ -257,6 +365,7 @@ interface Foo {
257365
},
258366
],
259367
output: noFormat`
368+
// isn't actually valid ts but want to not give message saying it refers to Foo.
260369
type Foo = () => {
261370
a: {
262371
nested: this;

0 commit comments

Comments
 (0)
0