Skip to content

Commit

Permalink
feat: enforce to use function declaration on top-level
Browse files Browse the repository at this point in the history
  • Loading branch information
antfu committed Mar 29, 2023
1 parent e17d2f8 commit 87d26fb
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 2 deletions.
2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -103,6 +103,8 @@ This config does NOT lint CSS. I personally use [UnoCSS](https://github.com/unoc

Sure, you can override the rules in your `.eslintrc` file.

<!-- eslint-skip -->

```jsonc
{
"extends": "@antfu",
Expand Down
2 changes: 1 addition & 1 deletion fixtures/vitesse/src/components/Footer.vue
@@ -1,7 +1,7 @@
<script setup lang="ts">
const { t, availableLocales, locale } = useI18n()
const toggleLocales = () => {
function toggleLocales() {
// change to some real logic
const locales = availableLocales
locale.value = locales[(locales.indexOf(locale.value) + 1) % locales.length]
Expand Down
2 changes: 1 addition & 1 deletion fixtures/vitesse/src/pages/index.vue
Expand Up @@ -3,7 +3,7 @@ const user = useUserStore()
const name = $ref(user.savedName)
const router = useRouter()
const go = () => {
function go() {
if (name)
router.push(`/hi/${encodeURIComponent(name)}`)
}
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-config-basic/index.js
Expand Up @@ -365,6 +365,7 @@ module.exports = {
// antfu
'antfu/if-newline': 'error',
'antfu/import-dedupe': 'error',
'antfu/top-level-function': 'error',
// 'antfu/prefer-inline-type-import': 'error',
},
}
2 changes: 2 additions & 0 deletions packages/eslint-plugin-antfu/src/index.ts
Expand Up @@ -2,12 +2,14 @@ import genericSpacing from './rules/generic-spacing'
import ifNewline from './rules/if-newline'
import importDedupe from './rules/import-dedupe'
import preferInlineTypeImport from './rules/prefer-inline-type-import'
import topLevelFunction from './rules/top-level-function'

export default {
rules: {
'if-newline': ifNewline,
'import-dedupe': importDedupe,
'prefer-inline-type-import': preferInlineTypeImport,
'generic-spacing': genericSpacing,
'top-level-function': topLevelFunction,
},
}
51 changes: 51 additions & 0 deletions packages/eslint-plugin-antfu/src/rules/top-level-function.test.ts
@@ -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 packages/eslint-plugin-antfu/src/rules/top-level-function.ts
@@ -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)
},
})
},
}
},
})

9 comments on commit 87d26fb

@felixranesberger
Copy link

@felixranesberger felixranesberger commented on 87d26fb Mar 29, 2023

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?

@mcfarljw
Copy link

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

export default createGlobalState(() => {
  return {}
})

@antfu
Copy link
Owner Author

@antfu antfu commented on 87d26fb Mar 29, 2023

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

@antfu
Copy link
Owner Author

@antfu antfu commented on 87d26fb Mar 29, 2023

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.

@mcfarljw
Copy link

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 to 0.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.

@artur-oliva
Copy link

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 rule

@lincenying
Copy link
Contributor

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 rule

'antfu/top-level-function': 'off',

@zanminkian
Copy link
Contributor

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.

const stuName = (stu) => `name: ${stu.name}`
const stuAge = (stu) => `age: ${stu.age}`
const stuGender = (stu) => `gender: ${stu.gender}`

@antfu Is it possible that don't format the one-line arrow function?

@antfu
Copy link
Owner Author

@antfu antfu commented on 87d26fb Apr 17, 2023

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

Please sign in to comment.