Enhancement: [restrict-plus-operands] Bring in options from restrict-template-expressions

[JoshuaKGoldberg]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/restrict-plus-operands

Description

restrict-plus-operands and restrict-template-expressions don't tackle the exact same JavaScript behaviors (+ operands and template expressions don't concatenate/toString()/toValue() quite the same). But they both generally tackle the specific common case of code trying to join a string with a non-string:

'' + 0;
'' + {};
// _roughly_ equivalent to:
`${0}`;
`${{}}`;

As a user, I've been inconvenienced by restrict-plus-operands complaining on concatenating two primitive values: e.g. number and string. Sometimes I want to be able to join primitives together in my logs!

Proposal: can we add the following options that already exist in restrict-template-expressions?

• allowAny: already exists!
• allowNumber
• allowBoolean
• allowNullish
• allowRegExp

Fail

'' + {}

Pass

'' + 0

Additional Info

I think we've seen a bigger demand for them in restrict-template-expressions because of https://eslint.org/docs/latest/rules/prefer-template. Linted projects don't concatenate strings with + as much as they used to.

[JoshuaKGoldberg]

@bradzacher bringing in #6161 (comment) on allowNullish:

The nullish case will create TS errors for numbers, and is only valid in strings. In the string case it also means you'll be concatenating with 'null' | 'undefined' - which doesn't seem desirable in the vast majority of code.

I don't think that this option really makes sense in the context of the rule.

...and #6161 (comment) on allowBoolean:

Similar to the nullish case the boolean case will create TS errors for numbers, and is only valid when adding with strings. In the string case it also means you'll be concatenating with 'true' | 'false' - which doesn't seem desirable in the vast majority of code.

I don't think that this option really makes sense in the context of the rule.

I've anecdotally seen developers intentionally do both those string concatenations, such as for console logs when debugging. If we want to enable this in the recommended preset I think we'd need to make it so users aren't getting warnings from their working-as-intended code. We can turn these options off in strict. WDYT?

[bradzacher]

I've anecdotally seen developers intentionally do both those string concatenations, such as for console logs when debugging

IMO lint rules exist to ensure your code is ready for production.
Which means that if you do console.log('debug value' + boolean) - that code should definitely cause a lint error!

If the default/recommended config ignored that then you could very easily do like alert('You selected' + boolean) in production code - which would be bad and then lead the user to have to manually reconfigure the lint rule to turn off that check.

If we want to enable this in the recommended preset

restrict-plus-operands is currently a recommended rule and I haven't heard of people being upset that it doesn't allow booleans at all.
Have you had reports of people getting annoyed by it?

[JoshuaKGoldberg]

Have you had reports of people getting annoyed by it?

Yes, I've been annoyed by it. It's happened a few times that I've wanted to print out something that includes, say, a boolean or regular expression in a message in a CLI and been bit by this rule. But I can't remember where those are now.

---

You know, in my mind I had real world evidence from other users supporting this issue. But I can't find it anywhere. I suppose it's fine if we just close it until a real user requests the need. Thanks for the discussion! 😄

[JoshuaKGoldberg]

Aha @bradzacher I just found some examples. I tried enabling on the TypeScript repo and found about 200 lint complaints. A few examples of common code styles there are ...

• Verbose logging: log("getCompletionData: Get previous token: " + (timestamp() - start)); [completions.ts]
• Key generation: "" + (nameType as StringLiteralType | NumberLiteralType).value [checker.ts]
• assert.equal(lac.line + 1, lp.line, "Line number mismatch " + (lac.line + 1) + " " + lp.line + " " + i); [unittests ... versionCache.ts]

The unfortunate predicament many repos are in, I think, is that they intentionally want to violate this rule if and only if the types are known to safely create developer-readable strings. Without the more permissive opt-in options, their choices are to either disable the rule, use very many inline comments, or modify the code to be more verbose.

[bradzacher]

I wonder if there is a better way to do this sort of thing instead of using concatenation at the callsite?

For example instead of APIs like log(arg: string) and manually concatenating things together to a single argument, perhaps instead more like console log like log(...args: (string | number | boolean)[])

log('Foo bar' + baz + ' ' + bam);

log('Foo bar', baz, bam);

Eg an implementation like

function log(...args: (string | number | boolean)[]) {
  debugger(args.join(' '));
} 

There's opportunities to improve the codebase and have more safety in general instead of doing a shotgun, super unsafe option like allowStringAndNumber.

I definitely understand why you would want to turn on such an option - because it's easier to ignore the cases instead of changing the codebase - but IMO "fix the API and disable the existing cases to be fixed later" is a better approach.

IDK. Options live forever, but migrations are temporary. I'm always very averse to adding options instead of guiding teams to just do better.

[JoshuaKGoldberg]

better way to do this sort of thing instead of using concatenation at the callsite

Sure - I think we're in agreement that the concatenation pattern isn't great. I like the log function you're writing. Unfortunately I think we're not going to be able to win the battle of convincing people to use it, and our choice is between giving them an option with angry docs or them not using the rule at all.

[bradzacher]

I'm unhappy with, but won't block the addition of an option - but I think that we need to clearly document the caveat that it severely weakens the rule to the point that all it bans is string + nullish.

I think we should document alternatives and better patterns to use to help guide users towards better code because better code means you can have a stricter rule and a better codebase.