[v12.x] multiple util and assert backports #31431

BridgeAR · 2020-01-20T22:58:22Z

This backports the following PRs:

util: fix .format() not always calling toString when it should be #30343
util: never trigger proxy traps while inspecting objects #30767
util: inspect prototype properties if showHidden is truthy #30768
util: add Set and Map size to the inspect output #30225 (Partially. This was requested by @addaleax)
util: add (typed) array length to the default output #31027
src: accept single argument in getProxyDetails #30858
util: improve prototype inspection using inspect() and showHidden #31113
assert: provide extra context in case of failed error validation checks #28263 (Partially. Only non semver-major parts. This made it easier to backport assert: implement assert.match() #30929)
assert: implement assert.match() #30929

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

This makes sure that `util.format('%s', object)` will always call a user defined `toString` function. It was formerly not the case when the object had the function declared on the super class. At the same time this also makes sure that getters won't be triggered accessing the `constructor` property. PR-URL: nodejs#30343 Fixes: nodejs#30333 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>

This adds a couple of benchmarks to check different options and code paths. PR-URL: nodejs#30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure we do not retrieve the handler in case it's not required. This improves the performance a tiny bit for these cases. PR-URL: nodejs#30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

PR-URL: nodejs#30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure that the regular expression matches all built-in objects properly. So far a couple where missed. PR-URL: nodejs#30768 Fixes: nodejs#30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This is only active if the `showHidden` option is truthy. The implementation is a trade-off between accuracy and performance. This will miss properties such as properties added to built-in data types. The goal is mainly to visualize prototype getters and setters such as: class Foo { ownProperty = true get bar() { return 'Hello world!' } } const a = new Foo() The `bar` property is a non-enumerable property on the prototype while `ownProperty` will be set directly on the created instance. The output is similar to the one of Chromium when inspecting objects closer. The output from Firefox is difficult to compare, since it's always a structured interactive output and was therefore not taken into account. PR-URL: nodejs#30768 Fixes: nodejs#30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

addaleax · 2020-01-20T23:02:08Z

util: add Set and map size to inspect output

I don’t think that’s a commit that should go into an LTS release line. The rest seems fine at a glance.

This removes the special handling to inspect iterable objects with a null prototype. It is now handled together with the regular prototype. PR-URL: nodejs#30225 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Align the inspect output with the one used in the Chrome dev tools. A recent survey outlined that most users prefer to see the number of set and map entries. This should count as well for array sizes. The size is only added to regular arrays in case the constructor is not the default constructor. Typed arrays always indicate their size. PR-URL: nodejs#31027 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>

This makes sure this function stays backwards compatible in case it's accessed through the binding directly. Refs: nodejs#29947 (comment) PR-URL: nodejs#30858 Refs: nodejs#30767 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

The fast path for the prototype inspection had a bug that caused some prototype properties to be skipped that should in fact be inspected. PR-URL: nodejs#31113 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This refactors some code for less duplication. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This updates two outdated examples to the current implementation. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This makes sure that `AssertionError` links to the correct place in the assert documentation. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This adds a new functionality to the assertion module: a dedicated check for regular expressions. So far it's possible to use `assert.ok(regexp.test(string))`. This is not ideal though when it comes to the error message, since it's not possible to know how either of the input values look like. It's just known that the assertion failed. This allows to pass through the regular expression and the input string. The string is then matched against the regular expression and reports a expressive error message in case of a failure. PR-URL: nodejs#30929 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>

nodejs-github-bot · 2020-01-22T12:19:43Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28560/

nodejs-github-bot · 2020-01-25T09:50:57Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28636/
Retry windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/33605/

This makes sure that `util.format('%s', object)` will always call a user defined `toString` function. It was formerly not the case when the object had the function declared on the super class. At the same time this also makes sure that getters won't be triggered accessing the `constructor` property. Backport-PR-URL: #31431 PR-URL: #30343 Fixes: #30333 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>

This adds a couple of benchmarks to check different options and code paths. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure we do not retrieve the handler in case it's not required. This improves the performance a tiny bit for these cases. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure that the regular expression matches all built-in objects properly. So far a couple where missed. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This is only active if the `showHidden` option is truthy. The implementation is a trade-off between accuracy and performance. This will miss properties such as properties added to built-in data types. The goal is mainly to visualize prototype getters and setters such as: class Foo { ownProperty = true get bar() { return 'Hello world!' } } const a = new Foo() The `bar` property is a non-enumerable property on the prototype while `ownProperty` will be set directly on the created instance. The output is similar to the one of Chromium when inspecting objects closer. The output from Firefox is difficult to compare, since it's always a structured interactive output and was therefore not taken into account. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This removes the special handling to inspect iterable objects with a null prototype. It is now handled together with the regular prototype. Backport-PR-URL: #31431 PR-URL: #30225 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Align the inspect output with the one used in the Chrome dev tools. A recent survey outlined that most users prefer to see the number of set and map entries. This should count as well for array sizes. The size is only added to regular arrays in case the constructor is not the default constructor. Typed arrays always indicate their size. Backport-PR-URL: #31431 PR-URL: #31027 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>

This makes sure this function stays backwards compatible in case it's accessed through the binding directly. Refs: #29947 (comment) Backport-PR-URL: #31431 PR-URL: #30858 Refs: #30767 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

The fast path for the prototype inspection had a bug that caused some prototype properties to be skipped that should in fact be inspected. Backport-PR-URL: #31431 PR-URL: #31113 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This refactors some code for less duplication. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This updates two outdated examples to the current implementation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This makes sure that `AssertionError` links to the correct place in the assert documentation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This adds a new functionality to the assertion module: a dedicated check for regular expressions. So far it's possible to use `assert.ok(regexp.test(string))`. This is not ideal though when it comes to the error message, since it's not possible to know how either of the input values look like. It's just known that the assertion failed. This allows to pass through the regular expression and the input string. The string is then matched against the regular expression and reports a expressive error message in case of a failure. Backport-PR-URL: #31431 PR-URL: #30929 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>

MylesBorins · 2020-01-30T23:18:47Z

landed in ea3d4e8...67ec97a

This makes sure that `util.format('%s', object)` will always call a user defined `toString` function. It was formerly not the case when the object had the function declared on the super class. At the same time this also makes sure that getters won't be triggered accessing the `constructor` property. Backport-PR-URL: #31431 PR-URL: #30343 Fixes: #30333 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>

This adds a couple of benchmarks to check different options and code paths. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure we do not retrieve the handler in case it's not required. This improves the performance a tiny bit for these cases. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure that the regular expression matches all built-in objects properly. So far a couple where missed. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This is only active if the `showHidden` option is truthy. The implementation is a trade-off between accuracy and performance. This will miss properties such as properties added to built-in data types. The goal is mainly to visualize prototype getters and setters such as: class Foo { ownProperty = true get bar() { return 'Hello world!' } } const a = new Foo() The `bar` property is a non-enumerable property on the prototype while `ownProperty` will be set directly on the created instance. The output is similar to the one of Chromium when inspecting objects closer. The output from Firefox is difficult to compare, since it's always a structured interactive output and was therefore not taken into account. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This removes the special handling to inspect iterable objects with a null prototype. It is now handled together with the regular prototype. Backport-PR-URL: #31431 PR-URL: #30225 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Align the inspect output with the one used in the Chrome dev tools. A recent survey outlined that most users prefer to see the number of set and map entries. This should count as well for array sizes. The size is only added to regular arrays in case the constructor is not the default constructor. Typed arrays always indicate their size. Backport-PR-URL: #31431 PR-URL: #31027 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>

This makes sure this function stays backwards compatible in case it's accessed through the binding directly. Refs: #29947 (comment) Backport-PR-URL: #31431 PR-URL: #30858 Refs: #30767 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

The fast path for the prototype inspection had a bug that caused some prototype properties to be skipped that should in fact be inspected. Backport-PR-URL: #31431 PR-URL: #31113 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This refactors some code for less duplication. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This updates two outdated examples to the current implementation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This makes sure that `AssertionError` links to the correct place in the assert documentation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

This adds a new functionality to the assertion module: a dedicated check for regular expressions. So far it's possible to use `assert.ok(regexp.test(string))`. This is not ideal though when it comes to the error message, since it's not possible to know how either of the input values look like. It's just known that the assertion failed. This allows to pass through the regular expression and the input string. The string is then matched against the regular expression and reports a expressive error message in case of a failure. Backport-PR-URL: #31431 PR-URL: #30929 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>

BridgeAR added 6 commits January 20, 2020 23:03

util: never trigger any proxy traps using format()

62ed6a2

PR-URL: nodejs#30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>

util: fix built-in detection

7d19aba

This makes sure that the regular expression matches all built-in objects properly. So far a couple where missed. PR-URL: nodejs#30768 Fixes: nodejs#30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

nodejs-github-bot added assert Issues and PRs related to the assert subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module. v12.x labels Jan 20, 2020

BridgeAR force-pushed the backport-30343-to-12 branch from 9022c3c to 972cfd7 Compare January 21, 2020 08:22

BridgeAR added 4 commits January 22, 2020 09:32

BridgeAR force-pushed the backport-30343-to-12 branch from 972cfd7 to 508f855 Compare January 22, 2020 08:48

BridgeAR added 5 commits January 22, 2020 12:20

assert: DRY .throws code

ad12514

This refactors some code for less duplication. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

doc: update assert.throws() examples

8b82c85

This updates two outdated examples to the current implementation. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>

BridgeAR marked this pull request as ready for review January 22, 2020 11:21

BridgeAR force-pushed the backport-30343-to-12 branch from 508f855 to e2970ab Compare January 22, 2020 11:21

BethGriggs mentioned this pull request Jan 29, 2020

git-node: add --backport flag to land nodejs/node-core-utils#383

Merged

MylesBorins closed this Jan 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v12.x] multiple util and assert backports #31431

[v12.x] multiple util and assert backports #31431

BridgeAR commented Jan 20, 2020 •

edited

Loading

addaleax commented Jan 20, 2020

nodejs-github-bot commented Jan 22, 2020

nodejs-github-bot commented Jan 25, 2020 •

edited by MylesBorins

Loading

MylesBorins commented Jan 30, 2020

[v12.x] multiple util and assert backports #31431

[v12.x] multiple util and assert backports #31431

Conversation

BridgeAR commented Jan 20, 2020 • edited Loading

Checklist

addaleax commented Jan 20, 2020

nodejs-github-bot commented Jan 22, 2020

nodejs-github-bot commented Jan 25, 2020 • edited by MylesBorins Loading

MylesBorins commented Jan 30, 2020

BridgeAR commented Jan 20, 2020 •

edited

Loading

nodejs-github-bot commented Jan 25, 2020 •

edited by MylesBorins

Loading