sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.18k stars 362 forks source link

Rule proposal: no inline callbacks before other parameters #2234

Open fregante opened 9 months ago

fregante commented 9 months ago

Description

Initially reported in https://github.com/sindresorhus/memoize/issues/97, it's bad practice to place arguments after functions because functions tend to be inlined and eclipse the parameters that follow them.

This could actually be two rules:

@sindresorhus doesn't like the latter so I'll focus on the first part 😅

Fail

element.addEventListener('click', () => {
    // long
}, {once: true})
const memoized = memoize(() => {
    // long
}, {
    cache: new WeakMap(),
})

Pass

element.addEventListener('click', () => {
    // long
}) // No follow-up argument
const memoized = memoize(() => {
    // long
}) // No follow-up argument
const handler = () => {
    // long
};
element.addEventListener('click', handler, {once: true})
const raw = () => {
    // long
};
const memoized = memoize(raw, {
    cache: new WeakMap(),
})

{maxStatements: 3}

element.addEventListener('click', () => short(), {once: true})
const memoized = memoize((x) => alert(x), {
    cache: new WeakMap(),
})

Additional info

This might conflict with https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-callback-reference.md in some cases (reduce) if maxStatements defaults to 0

const foo = array.reduce(callback, 0);
// ❌ no-array-callback-reference

👇

const foo = array.reduce((a, b) => a + b, 0);
// ❌ no-inline-function-sometimes
sindresorhus commented 9 months ago

How about allowing one-line functions?

fregante commented 9 months ago

What about

mem(async url => {
  const r = await fetch(url);
  return r.json();
}, {
  something
};

For the purpose of this rule, this still seems readable. That's why it should probably default to "3 statements" and warn that "the inline function is too long and impacts readability" for anything longer.