Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 15 commits into from

Conversation

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]>
@nodejs-github-bot 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
@addaleax
Copy link
Member

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]>
@BridgeAR BridgeAR marked this pull request as ready for review January 22, 2020 11:21
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 25, 2020

MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
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
Copy link
Contributor

landed in ea3d4e8...67ec97a

BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants