Skip to content

Recursively mark object literals as non-fresh when checking assignability to intersections#8605

Merged
sandersn merged 7 commits into
masterfrom
recursively-remove-object-literal-freshness
May 23, 2016
Merged

Recursively mark object literals as non-fresh when checking assignability to intersections#8605
sandersn merged 7 commits into
masterfrom
recursively-remove-object-literal-freshness

Conversation

@sandersn
Copy link
Copy Markdown
Member

Fixes #7649

sandersn added 2 commits May 13, 2016 15:56
`getRegularTypeOfObjectLiteral` marks an object literal as non-fresh so
that excess object-property errors will not be reported. It is needed so
that errors aren't reported when checking assignability to intersections
or unions, for example. Previously, the function was not recursive, so
nested object literals still erroneously gave the error. Now it's
recursive.
nested: {
d: 'yes',
e: 'no'
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another layer of nesting would be good to test as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. works.

@sandersn
Copy link
Copy Markdown
Member Author

sandersn commented May 19, 2016

@DanielRosenwasser can you take a look at the refactored code?

(I looked at #8694 which you linked. It seems like the opposite of #7649 -- the compiler over-checks here, it under-checks there.)

@sandersn
Copy link
Copy Markdown
Member Author

ping @DanielRosenwasser

Comment thread src/compiler/checker.ts Outdated
return symbol;
}

function updateTypeOfMembers(type: Type, update: (propertyType: Type) => Type) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transform or map might be a better word that doesn't imply mutation.

Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note in a comment that this doesn't work for signatures of any kind.

@sandersn
Copy link
Copy Markdown
Member Author

OK, I addressed the comments, but didn't try to avoid creating a fresh SymbolTable. I think you are right that it's too complex to be worth it.

Comment thread src/compiler/checker.ts Outdated
return members;
}

/** Mark an object literal as exempt from the excess properties check.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually mark an object as exempt, it creates a new non-fresh object that will be exempt if the old one wasn't.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start on next line

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@DanielRosenwasser
Copy link
Copy Markdown
Member

Fix comment, then 👍

@sandersn sandersn merged commit 1527499 into master May 23, 2016
@sandersn sandersn deleted the recursively-remove-object-literal-freshness branch May 23, 2016 22:24
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants