"This binary expression is never nullish" errors due to the way I format my code

[ChiriVulpes]

🔎 Search Terms

This binary expression is never nullish

🕗 Version & Regression Information

• (Pretty confident that) this changed in commit or PR Disallow truthiness/nullishness checks on syntax that never varies on it #59217

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.6.0-beta#code/KYOwrgtgBACghgJwC4HkBmAROBPKBvAWACgBIAOQEsBzACySQomCgF4oAmKAKi6gAYANMRJYA7iFYduvAIxDSAZTAgEFAM7M2nHh3kicDJpO28AzHqUgNSY9KgAWPRjBqA1rZ0BWecICCAG38sbENNKDEJAB8oS1UNKGjg0ISY5Ws9AP9KWnpGMOc3FOy6UIz-URw1SUykvJTM4tymeQBfYmIAE2AAY39EZm6AeysbKgRBsAAHAC4oACUewYQOgB54ZHRggShwCAAjYAQAbQBdU5TlLrQKEGAOgD4Abk6evoQB4bUbScRUTBxZus-sFnkRiEMRlBrFU2JdgNdbh1hAB6ZFQJAIXBjODgN5QH7IKCDNBQDo4KDXBBfYQAfhpUDGE0mRwJwJwJxRaKQNFAUG6PO67goJO5zGxuMQ+N+RJJZNw6igONwrJlO2oJTy2xxHSgwqhg3RmMVgTVOShP3Ealp9IAFEDNjgAHQNdVNZgAMilG3+uHpjKmR3tPudgUaoROUFmcIRdwAlJz0TyJPyekKRTyGQgcWA8SriaTyQqlV6bPm5aVFSAdXq1AaMbg4Ca5ea4JbrVA7b8HdgQ0EDHVPazu1A-eMA0Hgr3akwI1Gq-CbnGE2hG-49nBBeiDauC7g1BarO3-cyJ06AhVsGoTqCgA

💻 Code

const sets = undefined
	// try granular part of day first
	?? group[partOfDay]
	// then check if the granular part of day is any part of nighttime, and if so try all night spawns
	?? (PartOfDay.AllNighttime & partOfDay ? group[PartOfDay.AllNighttime] : undefined)
	// then check if the granular part of day is any part of daytime, and if so try all day spawns
	?? (PartOfDay.AllDaytime & partOfDay ? group[PartOfDay.AllDaytime] : undefined)
	// fallback to all day spawns
	?? group[PartOfDay.Always];

🙁 Actual behavior

My codebases have a number of instances of things similar to the above — undefined is used to put the actual potential results each on their own line, all formatted the same way. However this no longer works in TS 5.6.0 due to it throwing an error. I think this is an over-eager error message.

🙂 Expected behavior

I should be able to format the code how I want to still

Additional information about the issue

Other examples of code in my codebases broken by this

let plugs: PlugRaw[] = undefined
	?? (this.state ? refresh.plugs ?? [] : undefined)
	?? this.plugs?.slice()
	?? (!this.plugSetHash ? undefined : (await DestinyPlugSetDefinition.get(this.plugSetHash))?.reusablePlugItems)
	?? (this.plugSetHash ? undefined : (await DeepsightSocketExtendedDefinition.get(this.item?.definition.hash))?.sockets[this.index]?.rewardPlugItems)
	?? [];

Component.create()
	.classes.add(ItemTooltipSourceClasses.ActivityName)
	.text.set(undefined
		?? Display.name(lostSectorDisplay)
		?? Display.name(source.dropTable.displayProperties)
		?? Display.name(activity))
	.appendTo(activityComponent);

.setRefreshMethod(() => undefined
	?? (status === UiStatusType.Cut ? this.human?.getStatusLevel(StatusType.Bleeding) === BleedLevel.Minor : undefined)
	?? (status === StatusType.Bleeding ? this.human?.getStatusLevel(StatusType.Bleeding) === BleedLevel.Major : undefined)
	?? !!this.human?.hasStatus(status as StatusType))

Also, this still works for true && conditions and false || conditions — would be nice if it stayed consistent:

const inRange = true
	&& gatherTile.x - treasureRange <= treasure.x && gatherTile.x + treasureRange >= treasure.x
	&& gatherTile.y - treasureRange <= treasure.y && gatherTile.y + treasureRange >= treasure.y
	&& executor.z === map.position.z;

const needsAuthView = false
	|| (viewRequiredAuth === "required" && !ProfileManager.isAuthenticated())
	|| (viewRequiredAuth === "spy" && !ProfileManager.get());

[jcalz]

I wouldn't call this a formatting issue, since the line breaks aren't involved. Writing it all on the same line doesn't change the error.

Note that whether or not this is intentional behavior, the error message is bizarre. Right now it's flagging undefined with the error "This binary expression is never nullish. Are you missing parentheses?" 😕

[ChiriVulpes]

That's true. I guess I was a little hyperfocused on the formatting aspect because I wouldn't do this kind of thing a single line, since it's just noise in that case

[MartinJohns]

Note that whether or not this is intentional behavior, the error message is bizarre. Right now it's flagging undefined with the error "This binary expression is never nullish. Are you missing parentheses?" 😕

And you can't even work around it using a type assertion, which IMO would be a suitable solution to the OPs issue.

[RyanCavanaugh]

The error message needs to be fixed but I'd still consider this a correctly-issued error. This doesn't seem to be an idiomatic pattern and honestly, as a human, that code does look like it has a bug. It's not really distinguishable from other cases which absolutely do represent bugs. As a workaround you could write something like

const seeBelow = undefined;
const p = seeBelow
  ?? a
  ?? b
  ?? c;

since the check is entirely syntactic on undefined.

true and false don't get the "static truthiness" check applied since things like while (true) or do { } while (false) are reasonable to write.

[ChiriVulpes]

Since you've added Help Wanted and put it in the Backlog, does that mean it won't be fixed in 5.6.0 unless I PR to fix it? I can try, I suppose.

Regarding the comment:

In the one project I had to port over to 5.6.0-beta due to a teammate, I changed about 30 instances of code like this to use a new _ global I set up instead. I probably could've also done something like Undefined. Either way is in my opinion pretty terrible, but it was what I had to do here.

I have been using TypeScript and doing this for a number of years and would not be exaggerating to say there are hundreds of instances of this in my code. I would really rather not have to change all of them. It's probably at least a thousand if we're including the true && and false || variants, which I see as functionally the same thing. It's really bizarre to me that it would work for those but not for this. In practice they all work the same, even if they're classified differently — && uses the right if the left is truthy, || uses the right if the left is falsy, and ?? uses the right if the left is undefined or null.

In an ideal world, I would be able to put nothing on the first line and have ?? or || or && work fine without something on the left, like how TypeScript supports starting types with a leading | or & for consistent formatting of the components of the type on each line (clearly at some point your design you all have seen the benefit of that?) But yeah, that does not work in JavaScript and will probably never work in JavaScript, because teams writing code with an extraneous line like this is not particularly common even if it is arguably easier to read when you're used to it due to all relevant lines being formatted identically.

So since JavaScript will probably never support this, I've been doing this alternative. (And like all programming things, if you haven't been doing it you're not used to code like this so it looks confusing. I promise, for people doing this consistently, it's not!)

As far as this not being idiomatic — when exactly does something become idiomatic? SQL commonly has people write WHERE 1=1 or 1=0 so that every subsequent AND condition or OR condition line can be more quickly commented out or added/removed entirely. No, I don't normally see this in JavaScript, but I don't really know why? In the case of undefined ?? at least, ?? is a fairly new construct so it makes sense to me that people don't use that operator like that yet.

[snarbies]

I imagine the PR that would be accepted is a correction on the error message. Setting aside the specific error message, that there is an error under these circumstance is "working as intended." Branching on a known/constant condition is indicative of a mistake and should be called out. It wouldn't make sense to get rid of that useful functionality to suit a coding style.

[RyanCavanaugh]

In an ideal world, I would be able to put nothing on the first line and have ?? or || or && work fine without something on the left

ts-ignore seems suitable for this purpose? Since you wouldn't have a type error on the left side

As far as this not being idiomatic — when exactly does something become idiomatic?

"idiomatic" means "in common use". We ran this check on 800 repos and found zero instances of this pattern; everything the check did find was a bona fide bug. At some point it's worth the 0.1% false positive rate to find the other 99.9%.

[ChiriVulpes]

I've cloned the latest commit and I've been playing with predicateSemantics.ts.

const p01 = null ?? 1; // This expression is always nullish.
const p02 = null ?? null; // This expression is always nullish.

const p10 = cond ?? null ?? 1; // OK
const p11 = cond ?? null ?? null; // OK
const p12 = cond ?? (null ?? 1); // This expression is always nullish.
const p13 = cond ?? (null ?? null); // This expression is always nullish.

To me it seems like the actually useful error would be on p10 and p11 here, since that could be indicative of missing parentheses, but they're apparently fine right now? And meanwhile I can't see any reason why null ?? at the start of an expression would ever exist unintentionally, since it wouldn't be affected at all by being wrapped in parentheses and null is a literal

[ChiriVulpes]

This also doesn't error but probably should?

declare let opt: number | undefined;
declare let opt2: number | undefined;

const p19 = opt ?? (opt2 ? null : undefined) ?? 1; // OK

[ChiriVulpes]

Well, I have it working as I think it should locally, so I could make a PR. But I think we're still not on the same page so maybe this isn't helpful? Thoughts appreciated though

Note: Making the middle always-nullish and never-nullish operands error involved checking the right operand as well as the left operand. I think it makes sense that it must be done though? Because given a situation where the first left operand is undefined, that expression is just whatever the right operand is, so it should be subject to the same checks

Edit: I went ahead and just made the PR because I wanted to have done the whole process once in case I need the knowledge another time in the future

[Knagis]

I encountered this same in my project.

// OK
const a = false || obj["key1"] || obj["key2"];

// NOK
const b1 = undefined || obj["key1"] || obj["key2"];
const b2 = undefined ?? obj["key1"] ?? obj["key2"];

we had b - because at the time of writing it seemed better - since obj[] can return undefined, lets start with undefined.

It just seems slightly strange to see the special handling for false, but not null or undefined literals?

[RyanCavanaugh]

From the PR description:

The expressions false, true, 0, and 1 are excluded from this check, since code like while (true) {, while(1) {, do { } while (false), if (true || i_am_just_debugging) { are common and unlikely to represent programmer errors.