fix: both %i and %d should parse floats to int according to Formatter docs

[joshuapare]

Fixes #48238

According to the docs at https://console.spec.whatwg.org/#formatter, both %d and %i should parse to an integer.

Matched the parsing method of %d to the same as %i, and corrected tests with improper assertions given this information.

[kamilogorek]

Instead of fixing %d, you can modify the switch statement to match both variants, as they are now exactly the same:

case 100: // 100 ('d') and 105 are ('i') should be handled in the exact same way
case 105: {
  // ...
}

This change also requires an update to util.inspect docs table - https://github.com/nodejs/node/blob/main/doc/api/util.md#utilformatformat-args

[kvakil]

Marking this as semver-major, since it changes the output of console.log & util.format.

[MoLow]

NIT: in chrome console.log('%d', '-0.5') outputs 0

[joshuapare]

Sorry I'm not familiar with that acronym 😅 though this is a good point of difference between both integer formats in server and in browser (confirmed in Firefox as well).

Should this be corrected (likely not in this unit of work) or just noted?

[kamilogorek]

NIT = Nitpick (something that doesn't affect the overall PR and can, but doesn't necessarily have to be updated in order for it to be mergeable) :)

[ljharb]

Given that node and chrome are v8, that seems more important to align with than firefox here?

[joshuapare]

Agreed 100%, was just noting that Firefox behaved the same as Chrome in this situation with the sign flip, so it's not specific to just Chrome/V8

[targos]

@nodejs/util

[ljharb]

This seems like it's both a breaking change, and also one that makes node strictly less useful solely to conform with browsers. Why is this desirable for a core module? util.format isn't a web API.

[OguzhanUmutlu]

@ljharb It is the expected behavior of JavaScript.

[ljharb]

@OguzhanUmutlu how so? "the web" isn't the same as "javascript", and while "%i" definitely tells me "integer" (in many languages), "%d" tells me "decimal", which is decidedly not integers (across many languages as well).

[joshuapare]

@ljharb while I see where you are coming from, I would like to note that the official description of utils.format in the NodeJS docs is:

A printf-like format string.

%d is standard for a decimal signed integer across most other printf implementations, including C/C++. Given this, along with the current behavior of the major browsers, I would think that this is a valid point of consistency to achieve given that it neither matches the behavior of the V8 engine, the browser, or historical behavior of printf.

In addition, I'm not sure how this deviation of %d is useful - if you need floating point precision, you'd want the first position after the decimal even if it was zero, in which case you would use %f. The only difference left (at least from what I can tell, which I could very well be wrong) is Infinity being formatted as NaN instead of 'Infinity', which I wouldn't think is a valid reason to deviate.

However, this is not my call to make, so I'm more than happy to revise based on the feedback from the maintainers.

[aduh95]

"%d" tells me "decimal", which is decidedly not integers (across many languages as well).

%d means "integer in base 10" in every languages I've ever used 🤔 (and base 10 is decimal of course). IMO it makes sense to align.

[BridgeAR]

I am a bit torn what to do here. The suggestion is definitely valid, as it aligns with the "normal" %d behavior. In Node.js it has just always worked as "decimal", and some code might rely upon the current behavior - especially debug logs. I can't think of a situation where changing the behavior now would improve a users experience and therefore, I wonder if it makes sense to keep the current behavior as legacy oversight.

[aduh95]

This would need a rebase and a fix for the the linter failure.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[avivkeller]

Hey, I've marked this PR as stalled, as it is currently unable to land. @aduh95 gave suggestions on how to make this landable, but they are so-far unaddressed.

[BridgeAR]

I thought about it more and this could definitely cause difficult to detect problems for users to change this. Imagine sensor data is formatted using util.format() and now the decimal part would just be ignored. It is pretty much impossible to detect the changed behavior (integers would still be legit) but it would break the users expected behavior.
For new users, it's easy to switch to %i for integers, as they will immediately see that %d is not used like that (as it is documented).
Overall, I don't think it's worth the risk of breaking applications/causing data loss for that.