eslint-plugin-unicorn: no-useless-undefined - should not be fixable

Fair enough - I suppose there is a threshold of what an edge case is. Relevant xkcd:

I still think this rule is making too many assumptions about the runtime implementation of the function. It relies on the function being called not doing something strange, which to me is dangerous.

Fair enough that we can’t expect to succeed in never making runtime behaviour changes. But in this case, I think there’s a pretty reasonable chance that automatically deleting a parameter will have a real side-effect.

Which is related to another reason this shouldn’t auto-fix IMO: unlike most autofix rules like prettier/prettier, indent, no-semi, etc., if I write doSomething(undefined), it’s fairly likely that I did it on purpose, specifically because I didn’t want doSomething() for whatever reason. The rule is autofixing to the more obvious overload. The fact that I wrote doSomething(undefined) is a signal that doSomething is a weird function that behaves differently based on the number of args - at least the probability is much higher than for the average function call.

So while it’s an edge case for the set of all function calls that undefined is “useful”, when this rule is actually triggered, it’s much more likely to be significant. It’s a good thing that the linter is flagging unusual and potentially dangerous code, but it’s a bad thing that it’s blindly fixing it, potentially without the developer noticing. In my case, I updated this package in a medium sized repo, and there were a bunch of lint changes. Included were a small number of false positive fixes for this rule that got silently changed by yarn lint --fix. They might have slipped in unnoticed if there wasn’t proper test coverage. I would have preferred to handle them manually, since then I could have given proper thought to whether some refactoring/redesign was needed.

Anyway, I’ll shut up now - it’s not that big a deal 🙃

There are many functions that behave differently when they receive an explicit undefined. console.log was just a simple way of showing that. Here’s a contrived example of how this could break a program:

const logResultsAndExit = (...args: unknown[]) => {
  if (args.length === 0) {
    throw Error('missing args')
  }
  console.log(...args)
  process.exit()
}

// Before fix
logResultsAndExit(undefined) // works

// After fix
logResultsAndExit() // throws at runtime

A more likely example, with jest:

expect(someValue).toEqual(undefined)

The above works ok without the rule. With it, it’ll be fixed to expect(someValue).toEqual() which is a compiler error. If the fix becomes becomes a suggestion, I can look at it, and decide for myself it’s better to change it to expect(someValue).toBeUndefined() , but there’s no way the no-useless-undefined could know about that particular jest feature.

I accept that it’s usually not a good design, but 1) it’s not always in our control - maybe we didn’t write the function, we’re just using it and 2) that’s why there’s a lint rule for this. It’s ok to discourage accidental reliance on bad design, and even to suggest a fix, but it’s not ok to change the runtime behaviour with an auto-fix.