From 78ad0f4c82a08245818edbac07cb1222edfa7f22 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Tue, 6 Oct 2015 10:03:27 -0700 Subject: [PATCH 1/4] Re-enable failing fourslash tests --- tests/cases/fourslash/commentsOverloads.ts | 22 ++++++------- .../completionListInstanceProtectedMembers.ts | 20 ++++++------ ...completionListInstanceProtectedMembers2.ts | 18 +++++------ .../completionListProtectedMembers.ts | 32 +++++++++---------- 4 files changed, 46 insertions(+), 46 deletions(-) diff --git a/tests/cases/fourslash/commentsOverloads.ts b/tests/cases/fourslash/commentsOverloads.ts index d32c35b0508..1100b55dca8 100644 --- a/tests/cases/fourslash/commentsOverloads.ts +++ b/tests/cases/fourslash/commentsOverloads.ts @@ -482,8 +482,8 @@ verify.quickInfoIs("(method) c.prop1(a: number): number (+1 overload)", ""); goTo.marker('46'); verify.currentSignatureHelpDocCommentIs(""); verify.currentParameterHelpArgumentDocCommentIs(""); -//goTo.marker('46q'); -//verify.quickInfoIs("(method) c.prop1(b: string): number (+1 overload)", ""); +goTo.marker('46q'); +verify.quickInfoIs("(method) c.prop1(b: string): number (+1 overload)", ""); goTo.marker('47'); verify.currentSignatureHelpDocCommentIs("prop2 1"); @@ -494,8 +494,8 @@ verify.quickInfoIs("(method) c.prop2(a: number): number (+1 overload)", "prop2 1 goTo.marker('48'); verify.currentSignatureHelpDocCommentIs(""); verify.currentParameterHelpArgumentDocCommentIs(""); -//goTo.marker('48q'); -//verify.quickInfoIs("(method) c.prop2(b: string): number (+1 overload)", ""); +goTo.marker('48q'); +verify.quickInfoIs("(method) c.prop2(b: string): number (+1 overload)", ""); goTo.marker('49'); verify.currentSignatureHelpDocCommentIs(""); @@ -506,8 +506,8 @@ verify.quickInfoIs("(method) c.prop3(a: number): number (+1 overload)", ""); goTo.marker('50'); verify.currentSignatureHelpDocCommentIs("prop3 2"); verify.currentParameterHelpArgumentDocCommentIs(""); -//goTo.marker('50q'); -//verify.quickInfoIs("(method) c.prop3(b: string): number (+1 overload)", "prop3 2"); +goTo.marker('50q'); +verify.quickInfoIs("(method) c.prop3(b: string): number (+1 overload)", "prop3 2"); goTo.marker('51'); verify.currentSignatureHelpDocCommentIs("prop4 1"); @@ -518,8 +518,8 @@ verify.quickInfoIs("(method) c.prop4(a: number): number (+1 overload)", "prop4 1 goTo.marker('52'); verify.currentSignatureHelpDocCommentIs("prop4 2"); verify.currentParameterHelpArgumentDocCommentIs(""); -//goTo.marker('52q'); -//verify.quickInfoIs("(method) c.prop4(b: string): number (+1 overload)", "prop4 2"); +goTo.marker('52q'); +verify.quickInfoIs("(method) c.prop4(b: string): number (+1 overload)", "prop4 2"); goTo.marker('53'); verify.currentSignatureHelpDocCommentIs("prop5 1"); @@ -530,8 +530,8 @@ verify.quickInfoIs("(method) c.prop5(a: number): number (+1 overload)", "prop5 1 goTo.marker('54'); verify.currentSignatureHelpDocCommentIs("prop5 2"); verify.currentParameterHelpArgumentDocCommentIs(""); -//goTo.marker('54q'); -//verify.quickInfoIs("(method) c.prop5(b: string): number (+1 overload)", "prop5 2"); +goTo.marker('54q'); +verify.quickInfoIs("(method) c.prop5(b: string): number (+1 overload)", "prop5 2"); goTo.marker('55'); verify.currentSignatureHelpDocCommentIs(""); @@ -730,4 +730,4 @@ goTo.marker('106'); verify.quickInfoIs("(method) c.prop5(b: string): number (+1 overload)", "prop5 2"); goTo.marker('107'); -verify.quickInfoIs("(method) c.prop5(a: number): number (+1 overload)", "prop5 1"); \ No newline at end of file +verify.quickInfoIs("(method) c.prop5(a: number): number (+1 overload)", "prop5 1"); diff --git a/tests/cases/fourslash/completionListInstanceProtectedMembers.ts b/tests/cases/fourslash/completionListInstanceProtectedMembers.ts index e396a71247a..767dd1dff27 100644 --- a/tests/cases/fourslash/completionListInstanceProtectedMembers.ts +++ b/tests/cases/fourslash/completionListInstanceProtectedMembers.ts @@ -31,15 +31,15 @@ // Same class, everything is visible -//goTo.marker("1"); -//verify.memberListContains('privateMethod'); -//verify.memberListContains('privateProperty'); -//verify.memberListContains('protectedMethod'); -//verify.memberListContains('protectedProperty'); -//verify.memberListContains('publicMethod'); -//verify.memberListContains('publicProperty'); -//verify.memberListContains('protectedOverriddenMethod'); -//verify.memberListContains('protectedOverriddenProperty'); +goTo.marker("1"); +verify.memberListContains('privateMethod'); +verify.memberListContains('privateProperty'); +verify.memberListContains('protectedMethod'); +verify.memberListContains('protectedProperty'); +verify.memberListContains('publicMethod'); +verify.memberListContains('publicProperty'); +verify.memberListContains('protectedOverriddenMethod'); +verify.memberListContains('protectedOverriddenProperty'); goTo.marker("2"); verify.memberListContains('privateMethod'); @@ -60,4 +60,4 @@ verify.memberListContains('protectedProperty'); verify.memberListContains('publicMethod'); verify.memberListContains('publicProperty'); verify.not.memberListContains('protectedOverriddenMethod'); -verify.not.memberListContains('protectedOverriddenProperty'); \ No newline at end of file +verify.not.memberListContains('protectedOverriddenProperty'); diff --git a/tests/cases/fourslash/completionListInstanceProtectedMembers2.ts b/tests/cases/fourslash/completionListInstanceProtectedMembers2.ts index fed310aba21..c074bc06314 100644 --- a/tests/cases/fourslash/completionListInstanceProtectedMembers2.ts +++ b/tests/cases/fourslash/completionListInstanceProtectedMembers2.ts @@ -32,15 +32,15 @@ // Same class, everything is visible -//goTo.marker("1"); -//verify.not.memberListContains('privateMethod'); -//verify.not.memberListContains('privateProperty'); -//verify.memberListContains('protectedMethod'); -//verify.memberListContains('protectedProperty'); -//verify.memberListContains('publicMethod'); -//verify.memberListContains('publicProperty'); -//verify.memberListContains('protectedOverriddenMethod'); -//verify.memberListContains('protectedOverriddenProperty'); +goTo.marker("1"); +verify.not.memberListContains('privateMethod'); +verify.not.memberListContains('privateProperty'); +verify.memberListContains('protectedMethod'); +verify.memberListContains('protectedProperty'); +verify.memberListContains('publicMethod'); +verify.memberListContains('publicProperty'); +verify.memberListContains('protectedOverriddenMethod'); +verify.memberListContains('protectedOverriddenProperty'); // Can not access properties on super goTo.marker("2"); diff --git a/tests/cases/fourslash/completionListProtectedMembers.ts b/tests/cases/fourslash/completionListProtectedMembers.ts index 8ddd8aca4d0..4715a9fb714 100644 --- a/tests/cases/fourslash/completionListProtectedMembers.ts +++ b/tests/cases/fourslash/completionListProtectedMembers.ts @@ -18,25 +18,25 @@ ////var b: Base; ////f./*5*/ -//goTo.marker("1"); -//verify.memberListContains("y"); -//verify.memberListContains("x"); -//verify.not.memberListContains("z"); +goTo.marker("1"); +verify.memberListContains("y"); +verify.memberListContains("x"); +verify.not.memberListContains("z"); -//goTo.marker("2"); -//verify.memberListContains("y"); -//verify.memberListContains("x"); -//verify.memberListContains("z"); +goTo.marker("2"); +verify.memberListContains("y"); +verify.memberListContains("x"); +verify.memberListContains("z"); -//goTo.marker("3"); -//verify.memberListContains("y"); -//verify.memberListContains("x"); -//verify.not.memberListContains("z"); +goTo.marker("3"); +verify.memberListContains("y"); +verify.memberListContains("x"); +verify.not.memberListContains("z"); -//goTo.marker("4"); -//verify.memberListContains("y"); -//verify.memberListContains("x"); -//verify.memberListContains("z"); +goTo.marker("4"); +verify.memberListContains("y"); +verify.memberListContains("x"); +verify.memberListContains("z"); goTo.marker("5"); verify.not.memberListContains("x"); From 2fb6eabc2e1ced76709d6545c0a1c3508d3321cf Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 8 Oct 2015 08:11:33 -0700 Subject: [PATCH 2/4] Fix this.member completion+quickinfo of overloads 1. Completion after `this.` was empty. 2. Quick info of methods with overloads always chose the first overload, regardless of whether an argument whose type matched a different overload. Both have the same cause: the type parameter introduced by polymorphic `this` is not usable, whereas the original is. In both cases, the usage is simple -- it doesn't take advantage of the capabilities of polymorphic `this`. --- src/compiler/checker.ts | 5 +++++ src/services/services.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index fe203ac52c9..32fc57cee3a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7977,6 +7977,11 @@ namespace ts { return true; } // An instance property must be accessed through an instance of the enclosing class + if (type.flags & TypeFlags.ThisType) { + // get the original type -- represented as the type constraint of the this type + type = getConstraintOfTypeParameter(type); + } + // TODO: why is the first part of this check here? if (!(getTargetType(type).flags & (TypeFlags.Class | TypeFlags.Interface) && hasBaseType(type, enclosingClass))) { error(node, Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1, symbolToString(prop), typeToString(enclosingClass)); diff --git a/src/services/services.ts b/src/services/services.ts index 99c6e4d6032..5a8241a5c45 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -4105,7 +4105,7 @@ namespace ts { let useConstructSignatures = callExpression.kind === SyntaxKind.NewExpression || callExpression.expression.kind === SyntaxKind.SuperKeyword; let allSignatures = useConstructSignatures ? type.getConstructSignatures() : type.getCallSignatures(); - if (!contains(allSignatures, signature.target || signature)) { + if (!contains(allSignatures, signature.target) && !contains(allSignatures, signature)) { // Get the first signature if there signature = allSignatures.length ? allSignatures[0] : undefined; } From 10f9fa6da6cb02934f35b906039b09e78b6adcb4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 8 Oct 2015 09:30:08 -0700 Subject: [PATCH 3/4] Fix lint: remove trailing whitespace on empty line --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 32fc57cee3a..24b9f1651ac 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7981,7 +7981,7 @@ namespace ts { // get the original type -- represented as the type constraint of the this type type = getConstraintOfTypeParameter(type); } - + // TODO: why is the first part of this check here? if (!(getTargetType(type).flags & (TypeFlags.Class | TypeFlags.Interface) && hasBaseType(type, enclosingClass))) { error(node, Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1, symbolToString(prop), typeToString(enclosingClass)); From f19a2f54ed9186d866350d53ed6341d95645c869 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 9 Oct 2015 09:02:42 -0700 Subject: [PATCH 4/4] Fixup comments --- src/compiler/checker.ts | 2 +- src/services/services.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 24b9f1651ac..7027cbd7568 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7978,7 +7978,7 @@ namespace ts { } // An instance property must be accessed through an instance of the enclosing class if (type.flags & TypeFlags.ThisType) { - // get the original type -- represented as the type constraint of the this type + // get the original type -- represented as the type constraint of the 'this' type type = getConstraintOfTypeParameter(type); } diff --git a/src/services/services.ts b/src/services/services.ts index 5a8241a5c45..b707afa9ab5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -4106,7 +4106,8 @@ namespace ts { let allSignatures = useConstructSignatures ? type.getConstructSignatures() : type.getCallSignatures(); if (!contains(allSignatures, signature.target) && !contains(allSignatures, signature)) { - // Get the first signature if there + // Get the first signature if there is one -- allSignatures may contain + // either the original signature or its target, so check for either signature = allSignatures.length ? allSignatures[0] : undefined; }