Skip to content

Housekeeping: Fix equalOwnProperties#26794

Merged
RyanCavanaugh merged 2 commits into
microsoft:masterfrom
s0:fix-equalownproperties
Aug 31, 2018
Merged

Housekeeping: Fix equalOwnProperties#26794
RyanCavanaugh merged 2 commits into
microsoft:masterfrom
s0:fix-equalownproperties

Conversation

@s0
Copy link
Copy Markdown
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
@s0
Add test for ts.equalOwnProperties
65fa012
@s0
Fix equalOwnProperties
2c41d8b 
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
Copy Markdown

msftclas commented Aug 30, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

@RyanCavanaugh
Copy link
Copy Markdown
Member

RyanCavanaugh commented Aug 30, 2018

Someone should write a static type checker for JavaScript 🤔

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

RyanCavanaugh commented Aug 31, 2018

Thanks!

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaugh RyanCavanaugh RyanCavanaugh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@s0 @msftclas @RyanCavanaugh