-
-
Notifications
You must be signed in to change notification settings - Fork 478
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: enforce to use function declaration on top-level
- Loading branch information
Showing
7 changed files
with
139 additions
and
2 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
51 changes: 51 additions & 0 deletions
51
packages/eslint-plugin-antfu/src/rules/top-level-function.test.ts
This file contains 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,51 @@ | ||
import { RuleTester } from '@typescript-eslint/utils/dist/ts-eslint' | ||
import { it } from 'vitest' | ||
import rule, { RULE_NAME } from './top-level-function' | ||
|
||
const valids = [ | ||
'function foo() {}', | ||
// allow arrow function inside function | ||
'function foo() { const bar = () => {} }', | ||
// allow arrow function when type is specified | ||
'const Foo: Bar = () => {}', | ||
// allow let/var | ||
'let foo = () => {}', | ||
// allow arrow function in as | ||
'const foo = (() => {}) as any', | ||
// allow iife | ||
';(() => {})()', | ||
] | ||
|
||
const invalids = [ | ||
[ | ||
'const foo = (as: string, bar: number) => { return as + bar }', | ||
'function foo (as: string, bar: number) { return as + bar }', | ||
], | ||
[ | ||
'const foo = <K, T extends Boolean>(as: string, bar: number): Omit<T, K> => as + bar', | ||
'function foo <K, T extends Boolean>(as: string, bar: number): Omit<T, K> { return as + bar }', | ||
], | ||
[ | ||
'export const foo = () => {}', | ||
'export function foo () {}', | ||
], | ||
[ | ||
'export const foo = () => ({})', | ||
'export function foo () { return {} }', | ||
], | ||
] | ||
|
||
it('runs', () => { | ||
const ruleTester: RuleTester = new RuleTester({ | ||
parser: require.resolve('@typescript-eslint/parser'), | ||
}) | ||
|
||
ruleTester.run(RULE_NAME, rule, { | ||
valid: valids, | ||
invalid: invalids.map(i => ({ | ||
code: i[0], | ||
output: i[1], | ||
errors: [{ messageId: 'topLevelFunctionDeclaration' }], | ||
})), | ||
}) | ||
}) |
81 changes: 81 additions & 0 deletions
81
packages/eslint-plugin-antfu/src/rules/top-level-function.ts
This file contains 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,81 @@ | ||
import { createEslintRule } from '../utils' | ||
|
||
export const RULE_NAME = 'top-level-function' | ||
export type MessageIds = 'topLevelFunctionDeclaration' | ||
export type Options = [] | ||
|
||
export default createEslintRule<Options, MessageIds>({ | ||
name: RULE_NAME, | ||
meta: { | ||
type: 'problem', | ||
docs: { | ||
description: 'Enforce top-level functions to be declared with function keyword', | ||
recommended: 'error', | ||
}, | ||
fixable: 'code', | ||
schema: [], | ||
messages: { | ||
topLevelFunctionDeclaration: 'Top-level functions should be declared with function keyword', | ||
}, | ||
}, | ||
defaultOptions: [], | ||
create: (context) => { | ||
return { | ||
VariableDeclaration(node) { | ||
if (node.parent.type !== 'Program' && node.parent.type !== 'ExportNamedDeclaration') | ||
return | ||
|
||
if (node.declarations.length !== 1) | ||
return | ||
if (node.kind !== 'const') | ||
return | ||
if (node.declare) | ||
return | ||
|
||
const declaration = node.declarations[0] | ||
|
||
if (declaration.init?.type !== 'ArrowFunctionExpression') | ||
return | ||
if (declaration.id?.type !== 'Identifier') | ||
return | ||
if (declaration.id.typeAnnotation) | ||
return | ||
|
||
const arrowFn = declaration.init | ||
const body = declaration.init.body | ||
const id = declaration.id | ||
|
||
context.report({ | ||
node, | ||
loc: { | ||
start: node.loc.start, | ||
end: node.loc.end, | ||
}, | ||
messageId: 'topLevelFunctionDeclaration', | ||
fix(fixer) { | ||
const code = context.getSourceCode().text | ||
const textName = code.slice(id.range[0], id.range[1]) | ||
const textArgs = arrowFn.params.length | ||
? code.slice(arrowFn.params[0].range[0], arrowFn.params[arrowFn.params.length - 1].range[1]) | ||
: '' | ||
const textBody = body.type === 'BlockStatement' | ||
? code.slice(body.range[0], body.range[1]) | ||
: `{ return ${code.slice(body.range[0], body.range[1])} }` | ||
const textGeneric = arrowFn.typeParameters | ||
? code.slice(arrowFn.typeParameters.range[0], arrowFn.typeParameters.range[1]) | ||
: '' | ||
const textTypeReturn = arrowFn.returnType | ||
? code.slice(arrowFn.returnType.range[0], arrowFn.returnType.range[1]) | ||
: '' | ||
const text = `function ${textName} ${textGeneric}(${textArgs})${textTypeReturn} ${textBody}` | ||
// console.log({ | ||
// input: code.slice(node.range[0], node.range[1]), | ||
// output: text, | ||
// }) | ||
return fixer.replaceTextRange([node.range[0], node.range[1]], text) | ||
}, | ||
}) | ||
}, | ||
} | ||
}, | ||
}) |
87d26fb
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.
Hi Anthony, thanks for providing this ESLint configuration!
What is your reason to enforce top-level functions, instead of arrow functions?
Is it because of the
this
binding?87d26fb
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.
This change breaks a lot of things in my current codebase so I had to disable it. For example:
https://vueuse.org/shared/createGlobalState/#createglobalstate
87d26fb
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.
@mcfarljw I am happy to improve it by including more cases if you could provide more info. The one you sent seems to work fine: 3fa8617
87d26fb
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.
@felixranesberger It's just out of personal preference. I might raise the awareness again this is my personal opinionated config. Feel free to disable/config/fork to fit your needs.
87d26fb
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.
@antfu thanks for responding! It looks like this was just a fluke, after updating from
0.37.0
to0.38.0
VSCode needed to be fully reloaded on my machine otherwise I just get this error at the top of most of my files.87d26fb
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.
What is the syntax to disable the
top-level-function
rule87d26fb
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.
'antfu/top-level-function': 'off',
87d26fb
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.
I agree most of the configs except this rule. For example, one-line arrow function is convenient in some case. But enforcing this rule will make the code verbose.
@antfu Is it possible that don't format the one-line arrow function?
87d26fb
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.
Sure, we could filter out one-liner, PR welcome