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

Remove notion of predicates as types, move predicates back to signatures #6744

Merged
merged 15 commits into from
Feb 12, 2016

Conversation

DanielRosenwasser
Copy link
Member

Fixes #6311.

This is an alternative approach to #6328.

Offline discussions have determined that there was not enough review around the idea of type predicates on accessors and properties. This moves them back to signatures, keeps this-based predicates, and continues to infer from type predicates.

@mhegazy

@DanielRosenwasser
Copy link
Member Author

Ping.

@@ -7092,6 +7082,7 @@ namespace ts {
case SyntaxKind.Identifier:
case SyntaxKind.PropertyAccessExpression:
case SyntaxKind.QualifiedName:
// TODO (drosen): Why a qualified name?
Copy link
Member

Choose a reason for hiding this comment

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

It may not be needed - I believe it was just for completeness, since EntityName (which is what the function on the following line takes) is Identifier | QualifiedName - provided I'm remembering that right.

It may very well be that the grammar prohibits a QualifiedName from being passed to this check.

@DanielRosenwasser
Copy link
Member Author

Ping.

@sandersn
Copy link
Member

👍

@vladima
Copy link
Contributor

vladima commented Feb 12, 2016

LGTM, except I'll probably move changing of error spans for accessor to another PR. This is definitely useful but brings to much noise in tests into this PR

if (source.kind !== target.kind) {
if (reportErrors) {
errorReporter(Diagnostics.A_this_based_type_guard_is_not_compatible_with_a_parameter_based_type_guard);
errorReporter(Diagnostics.Type_predicate_0_is_not_assignable_to_1, typePredicateToString(source), typePredicateToString(target));
Copy link
Contributor

Choose a reason for hiding this comment

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

not specific to this PR, but error messages refer to this sometimes as "type guards" and others as "type predicate"

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point I'll file an issue

@mhegazy
Copy link
Contributor

mhegazy commented Feb 12, 2016

+1 for Vlads comment.
Also can you add a couple of tests one for declaration emit for valid predicate values and another for declaration emit with using a private name.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 12, 2016

otherwise looks good 👍

@mhegazy
Copy link
Contributor

mhegazy commented Feb 12, 2016

👍

DanielRosenwasser added a commit that referenced this pull request Feb 12, 2016
Remove notion of predicates as types, move predicates back to signatures
@DanielRosenwasser DanielRosenwasser merged commit 5a76a1d into release-1.8 Feb 12, 2016
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.

6 participants