Skip to content

Housekeeping: Fix equalOwnProperties #26794

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

Merged
merged 2 commits into from
Aug 31, 2018

Conversation

s0
Copy link
Contributor

@s0 s0 commented Aug 30, 2018

equalOwnProperties() would incorrectly report two map-like objects as equal in the case where a property defined in left was not defined in right and whose value was considered "equal" to undefined by the equalityComparer.

This bug was found by an alert on LGTM.com

I've added unit tests that check around this functionality, which fail before the fix, and succeed after the fix commit.

Before 2c41d8b:

> jake runtests tests=compilerCore

...

Test failure:
correctly identifies undefined vs hasOwnProperty                [▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬․․․․․․․․․․․․․․․․․․․․․․․․․]
Test failure:
truthiness                                                      [▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬․․․․․․․․․․․․]
Test failure:
all equal                                                       [▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬]

  2 passing (2s)
  3 failing

  1) compilerCore
       equalOwnProperties
         correctly identifies undefined vs hasOwnProperty:
     Error: missing right property
      at Function.assert.isFalse (src/harness/harness.ts:10:78)
      at Context.<anonymous> (src/testRunner/unittests/compilerCore.ts:16:24)

  2) compilerCore
       equalOwnProperties
         truthiness:
     Error: missing right falsey property
      at Function.assert.isFalse (src/harness/harness.ts:10:78)
      at Context.<anonymous> (src/testRunner/unittests/compilerCore.ts:23:24)

  3) compilerCore
       equalOwnProperties
         all equal:
     Error: missing right property
      at Function.assert.isFalse (src/harness/harness.ts:10:78)
      at Context.<anonymous> (src/testRunner/unittests/compilerCore.ts:28:24)

After 2c41d8b:

> jake runtests tests=compilerCore

...

    compilerCore                                                [▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬]

  5 passing (245ms)

s0 added 2 commits August 30, 2018 14:08
equalOwnProperties would incorrectly report two map-like objects as equal in
the case where a property defined in `left` was not defined in `right` and
whose value was considered "equal" to undefined by the equalityComparer.

This bug was found by an alert on LGTM.com
@msftclas
Copy link

msftclas commented Aug 30, 2018

CLA assistant check
All CLA requirements met.

@RyanCavanaugh
Copy link
Member

Someone should write a static type checker for JavaScript 🤔

@RyanCavanaugh RyanCavanaugh merged commit cbdfc01 into microsoft:master Aug 31, 2018
@RyanCavanaugh
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants