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.

Sign up for GitHub

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

Added implementation for issue #25002 #25415

Closed
wants to merge 7 commits into from
Closed

Added implementation for issue #25002 #25415

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
51caec2
Implementation for issue #25002
bashdx Jul 3, 2018
2f0cc53
Implementation for issue #25002
bashdx Jul 3, 2018
f82d295
25002 - Resolved merge conflict
bashdx Jul 3, 2018
9aa2468
Resolve merge issue with upstream
bashdx Jul 4, 2018
bf1af7a
#25002 - changed error message
bashdx Jul 4, 2018
d39e424
#25002: Implemented changes from feedback
bashdx Jul 6, 2018
1d968ab
#25002: removed unneccessary newlines
bashdx Jul 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22046,7 +22046,7 @@ namespace ts {

// TypeScript 1.0 spec (April 2014): 4.5
// If both accessors include type annotations, the specified types must be identical.
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_same_type);
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getAnnotatedAccessorType, Diagnostics.get_and_set_accessor_must_have_the_same_type_0);
checkAccessorDeclarationTypesIdentical(node, otherAccessor, getThisTypeOfDeclaration, Diagnostics.get_and_set_accessor_must_have_the_same_this_type);
}
}
Expand All @@ -22062,7 +22062,14 @@ namespace ts {
const firstType = getAnnotatedType(first);
const secondType = getAnnotatedType(second);
if (firstType && secondType && !isTypeIdenticalTo(firstType, secondType)) {
error(first, message);
const typeNameSecondType = typeToString(secondType);

if(isGetAccessor(first)){

Copy link
Member

Choose a reason for hiding this comment

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

Remove newline.

const diagnostic: Diagnostic = error(first, message, typeNameSecondType);
addRelatedInfo(diagnostic, createDiagnosticForNode(first, Diagnostics.The_respective_set_accessor_has_the_type_0, typeNameSecondType));

}
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,7 @@
"category": "Error",
"code": 2379
},
"'get' and 'set' accessor must have the same type.": {
"'get' and 'set' accessor must have the same type '{0}'.": {
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking

Accessors must have identical types, but this 'get' accessor has type '{0}' and its respective 'set' accessor has type '{1}'.

with

The respective 'set' accessor is declared here.

However, to be honest, I'm not sure how I feel about this change; I feel like the simpler error message was actually better.

Copy link
Contributor

@mhegazy mhegazy Jul 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

why not just the the 'set' accessor is declared here:

"category": "Error",
"code": 2380
},
Expand Down Expand Up @@ -2409,6 +2409,10 @@
"category": "Error",
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be "Message" and not "Error"

"code": 2729
},
"The respective 'set' accessor has the type '{0}'.": {
"category": "Message",
"code": 2730
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
10 changes: 4 additions & 6 deletions tests/baselines/reference/abstractPropertyNegative.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
tests/cases/compiler/abstractPropertyNegative.ts(10,18): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/abstractPropertyNegative.ts(11,18): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/abstractPropertyNegative.ts(10,18): error TS2380: 'get' and 'set' accessor must have the same type 'number'.
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'm' from class 'B'.
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'mismatch' from class 'B'.
tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstract class 'C' does not implement inherited abstract member 'prop' from class 'B'.
Expand All @@ -19,7 +18,7 @@ tests/cases/compiler/abstractPropertyNegative.ts(40,9): error TS2676: Accessors
tests/cases/compiler/abstractPropertyNegative.ts(41,18): error TS2676: Accessors must both be abstract or non-abstract.


==== tests/cases/compiler/abstractPropertyNegative.ts (16 errors) ====
==== tests/cases/compiler/abstractPropertyNegative.ts (15 errors) ====
interface A {
prop: string;
m(): string;
Expand All @@ -31,10 +30,9 @@ tests/cases/compiler/abstractPropertyNegative.ts(41,18): error TS2676: Accessors
abstract m(): string;
abstract get mismatch(): string;
~~~~~~~~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'number'.
!!! related TS2730 tests/cases/compiler/abstractPropertyNegative.ts:10:18: The respective 'set' accessor has the type 'number'.
abstract set mismatch(val: number); // error, not same type
~~~~~~~~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
}
class C extends B {
~
Expand Down
3 changes: 2 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5156,7 +5156,7 @@ declare namespace ts {
Constructors_for_derived_classes_must_contain_a_super_call: DiagnosticMessage;
A_get_accessor_must_return_a_value: DiagnosticMessage;
Getter_and_setter_accessors_do_not_agree_in_visibility: DiagnosticMessage;
get_and_set_accessor_must_have_the_same_type: DiagnosticMessage;
get_and_set_accessor_must_have_the_same_type_0: DiagnosticMessage;
A_signature_with_an_implementation_cannot_use_a_string_literal_type: DiagnosticMessage;
Specialized_overload_signature_is_not_assignable_to_any_non_specialized_signature: DiagnosticMessage;
Overload_signatures_must_all_be_exported_or_non_exported: DiagnosticMessage;
Expand Down Expand Up @@ -5432,6 +5432,7 @@ declare namespace ts {
Cannot_find_lib_definition_for_0_Did_you_mean_1: DiagnosticMessage;
_0_was_declared_here: DiagnosticMessage;
Property_0_is_used_before_its_initialization: DiagnosticMessage;
The_respective_set_accessor_has_the_type_0: DiagnosticMessage;
Import_declaration_0_is_using_private_name_1: DiagnosticMessage;
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: DiagnosticMessage;
Type_parameter_0_of_exported_interface_has_or_is_using_private_name_1: DiagnosticMessage;
Expand Down
10 changes: 4 additions & 6 deletions tests/baselines/reference/getAndSetNotIdenticalType.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
tests/cases/compiler/getAndSetNotIdenticalType.ts(2,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType.ts(2,9): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/getAndSetNotIdenticalType.ts(2,9): error TS2380: 'get' and 'set' accessor must have the same type 'string'.
tests/cases/compiler/getAndSetNotIdenticalType.ts(5,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType.ts(5,9): error TS2380: 'get' and 'set' accessor must have the same type.


==== tests/cases/compiler/getAndSetNotIdenticalType.ts (4 errors) ====
==== tests/cases/compiler/getAndSetNotIdenticalType.ts (3 errors) ====
class C {
get x(): number {
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'string'.
!!! related TS2730 tests/cases/compiler/getAndSetNotIdenticalType.ts:2:9: The respective 'set' accessor has the type 'string'.
return 1;
}
set x(v: string) { }
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
}
10 changes: 4 additions & 6 deletions tests/baselines/reference/getAndSetNotIdenticalType2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
tests/cases/compiler/getAndSetNotIdenticalType2.ts(5,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType2.ts(5,9): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/getAndSetNotIdenticalType2.ts(5,9): error TS2380: 'get' and 'set' accessor must have the same type 'A<string>'.
tests/cases/compiler/getAndSetNotIdenticalType2.ts(8,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType2.ts(8,9): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/getAndSetNotIdenticalType2.ts(9,9): error TS2322: Type 'A<string>' is not assignable to type 'A<T>'.
Type 'string' is not assignable to type 'T'.


==== tests/cases/compiler/getAndSetNotIdenticalType2.ts (5 errors) ====
==== tests/cases/compiler/getAndSetNotIdenticalType2.ts (4 errors) ====
class A<T> { foo: T; }

class C<T> {
Expand All @@ -15,14 +14,13 @@ tests/cases/compiler/getAndSetNotIdenticalType2.ts(9,9): error TS2322: Type 'A<s
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'A<string>'.
!!! related TS2730 tests/cases/compiler/getAndSetNotIdenticalType2.ts:5:9: The respective 'set' accessor has the type 'A<string>'.
Copy link
Member

Choose a reason for hiding this comment

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

These should not repeat the same type in each message

return this.data;
}
set x(v: A<string>) {
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
this.data = v;
~~~~~~~~~
!!! error TS2322: Type 'A<string>' is not assignable to type 'A<T>'.
Expand Down
10 changes: 4 additions & 6 deletions tests/baselines/reference/getAndSetNotIdenticalType3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
tests/cases/compiler/getAndSetNotIdenticalType3.ts(5,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType3.ts(5,9): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/getAndSetNotIdenticalType3.ts(5,9): error TS2380: 'get' and 'set' accessor must have the same type 'A<string>'.
tests/cases/compiler/getAndSetNotIdenticalType3.ts(8,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/getAndSetNotIdenticalType3.ts(8,9): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/compiler/getAndSetNotIdenticalType3.ts(9,9): error TS2322: Type 'A<string>' is not assignable to type 'A<number>'.
Type 'string' is not assignable to type 'number'.


==== tests/cases/compiler/getAndSetNotIdenticalType3.ts (5 errors) ====
==== tests/cases/compiler/getAndSetNotIdenticalType3.ts (4 errors) ====
class A<T> { foo: T; }

class C<T> {
Expand All @@ -15,14 +14,13 @@ tests/cases/compiler/getAndSetNotIdenticalType3.ts(9,9): error TS2322: Type 'A<s
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'A<string>'.
!!! related TS2730 tests/cases/compiler/getAndSetNotIdenticalType3.ts:5:9: The respective 'set' accessor has the type 'A<string>'.
return this.data;
}
set x(v: A<string>) {
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
this.data = v;
~~~~~~~~~
!!! error TS2322: Type 'A<string>' is not assignable to type 'A<number>'.
Expand Down
18 changes: 7 additions & 11 deletions tests/baselines/reference/objectLiteralErrors.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,12 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(38,26)
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(39,13): error TS2300: Duplicate identifier 'a'.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(39,46): error TS1119: An object literal cannot have property and accessor with the same name.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(39,46): error TS2300: Duplicate identifier 'a'.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(42,16): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(42,47): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(42,16): error TS2380: 'get' and 'set' accessor must have the same type 'string'.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(43,22): error TS2322: Type '4' is not assignable to type 'string'.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(44,16): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(44,55): error TS2380: 'get' and 'set' accessor must have the same type.
tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(44,16): error TS2380: 'get' and 'set' accessor must have the same type 'string'.


==== tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts (78 errors) ====
==== tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts (76 errors) ====
// Multiple properties with the same name
var e1 = { a: 0, a: 0 };
~
Expand Down Expand Up @@ -268,15 +266,13 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts(44,55)
// Get and set accessor with mismatched type annotations
var g1 = { get a(): number { return 4; }, set a(n: string) { } };
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'string'.
!!! related TS2730 tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts:42:16: The respective 'set' accessor has the type 'string'.
var g2 = { get a() { return 4; }, set a(n: string) { } };
~~~~~~~~~
!!! error TS2322: Type '4' is not assignable to type 'string'.
var g3 = { get a(): number { return undefined; }, set a(n: string) { } };
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
~
!!! error TS2380: 'get' and 'set' accessor must have the same type.
!!! error TS2380: 'get' and 'set' accessor must have the same type 'string'.
!!! related TS2730 tests/cases/conformance/expressions/objectLiterals/objectLiteralErrors.ts:44:16: The respective 'set' accessor has the type 'string'.

6 changes: 2 additions & 4 deletions tests/baselines/reference/thisTypeInAccessorsNegative.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts(10,9): error TS2682: 'get' and 'set' accessor must have the same 'this' type.
tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts(11,9): error TS2682: 'get' and 'set' accessor must have the same 'this' type.


==== tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts (2 errors) ====
==== tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts (1 errors) ====
interface Foo {
n: number;
x: number;
Expand All @@ -15,9 +14,8 @@ tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts(11,9): err
get x(this: Foo) { return this.n; },
~
!!! error TS2682: 'get' and 'set' accessor must have the same 'this' type.
!!! related TS2730 tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts:10:9: The respective 'set' accessor has the type 'Bar'.
set x(this: Bar, n) { this.wrong = "method"; }
~
!!! error TS2682: 'get' and 'set' accessor must have the same 'this' type.
}
const contextual: Foo = {
n: 16,
Expand Down