Skip to content

Commit b50b9e0

Browse files
DanielRosenwasserDanielRosenwasser
authored
Fix check for get/set recommendation (#34885)
* Added more tests. * Accepted baselines. * Work better with any parameter type. * Accepted baselines. * Use the actual indexed expression. * Add tests that exhibit bad stringification. * Accepted baselines. * Short-circuit stringification on 'undefined'. * Accepted baselines. * Remove space. * Accepted baselines.
1 parent 0bbeab6 commit b50b9e0

9 files changed

+793
-109
lines changed

‎src/compiler/checker.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11995,7 +11995,7 @@ namespace ts {
1199511995
}
1199611996
}
1199711997
else {
11998-
const suggestion = getSuggestionForNonexistentIndexSignature(objectType, accessExpression);
11998+
const suggestion = getSuggestionForNonexistentIndexSignature(objectType, accessExpression, indexType);
1199911999
if (suggestion !== undefined) {
1200012000
error(accessExpression, Diagnostics.Element_implicitly_has_an_any_type_because_type_0_has_no_index_signature_Did_you_mean_to_call_1, typeToString(objectType), suggestion);
1200112001
}
@@ -23154,15 +23154,13 @@ namespace ts {
2315423154
return suggestion && symbolName(suggestion);
2315523155
}
2315623156

23157-
function getSuggestionForNonexistentIndexSignature(objectType: Type, expr: ElementAccessExpression): string | undefined {
23157+
function getSuggestionForNonexistentIndexSignature(objectType: Type, expr: ElementAccessExpression, keyedType: Type): string | undefined {
2315823158
// check if object type has setter or getter
23159-
const hasProp = (name: "set" | "get", argCount = 1) => {
23159+
function hasProp(name: "set" | "get") {
2316023160
const prop = getPropertyOfObjectType(objectType, <__String>name);
2316123161
if (prop) {
2316223162
const s = getSingleCallSignature(getTypeOfSymbol(prop));
23163-
if (s && getMinArgumentCount(s) === argCount && typeToString(getTypeAtPosition(s, 0)) === "string") {
23164-
return true;
23165-
}
23163+
return !!s && getMinArgumentCount(s) >= 1 && isTypeAssignableTo(keyedType, getTypeAtPosition(s, 0));
2316623164
}
2316723165
return false;
2316823166
};
@@ -23172,7 +23170,7 @@ namespace ts {
2317223170
return undefined;
2317323171
}
2317423172

23175-
let suggestion = tryGetPropertyAccessOrIdentifierToString(expr);
23173+
let suggestion = tryGetPropertyAccessOrIdentifierToString(expr.expression);
2317623174
if (suggestion === undefined) {
2317723175
suggestion = suggestedMethod;
2317823176
}

‎src/compiler/diagnosticMessages.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4518,7 +4518,7 @@
45184518
"category": "Error",
45194519
"code": 7051
45204520
},
4521-
"Element implicitly has an 'any' type because type '{0}' has no index signature. Did you mean to call '{1}' ?": {
4521+
"Element implicitly has an 'any' type because type '{0}' has no index signature. Did you mean to call '{1}'?": {
45224522
"category": "Error",
45234523
"code": 7052
45244524
},

‎src/compiler/utilities.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4244,9 +4244,12 @@ namespace ts {
42444244

42454245
export function tryGetPropertyAccessOrIdentifierToString(expr: Expression): string | undefined {
42464246
if (isPropertyAccessExpression(expr)) {
4247-
return tryGetPropertyAccessOrIdentifierToString(expr.expression) + "." + expr.name;
4247+
const baseStr = tryGetPropertyAccessOrIdentifierToString(expr.expression);
4248+
if (baseStr !== undefined) {
4249+
return baseStr + "." + expr.name;
4250+
}
42484251
}
4249-
if (isIdentifier(expr)) {
4252+
else if (isIdentifier(expr)) {
42504253
return unescapeLeadingUnderscores(expr.escapedText);
42514254
}
42524255
return undefined;

‎src/services/navigationBar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ namespace ts.NavigationBar {
901901
return "<function>";
902902
}
903903

904+
// See also 'tryGetPropertyAccessOrIdentifierToString'
904905
function getCalledExpressionName(expr: Expression): string | undefined {
905906
if (isIdentifier(expr)) {
906907
return expr.text;

‎tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt

Lines changed: 131 additions & 26 deletions
Large diffs are not rendered by default.

‎tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js

Lines changed: 102 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,52 @@ var d = {
1313
};
1414
const bar = d['hello'];
1515

16-
var e = {
17-
set: (key: string) => 'foobar',
18-
get: (key: string) => 'foobar'
19-
};
20-
e['hello'] = 'modified';
21-
e['hello'] += 1;
22-
e['hello'] ++;
16+
{
17+
let e = {
18+
get: (key: string) => 'foobar',
19+
set: (key: string) => 'foobar'
20+
};
21+
e['hello'];
22+
e['hello'] = 'modified';
23+
e['hello'] += 1;
24+
e['hello'] ++;
25+
}
26+
27+
{
28+
let e = {
29+
get: (key: string) => 'foobar',
30+
set: (key: string, value: string) => 'foobar'
31+
};
32+
e['hello'];
33+
e['hello'] = 'modified';
34+
e['hello'] += 1;
35+
e['hello'] ++;
36+
}
37+
38+
{
39+
let e = {
40+
get: (key: "hello" | "world") => 'foobar',
41+
set: (key: "hello" | "world", value: string) => 'foobar'
42+
};
43+
e['hello'];
44+
e['hello'] = 'modified';
45+
e['hello'] += 1;
46+
e['hello'] ++;
47+
}
48+
49+
{
50+
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'];
51+
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] = 'modified';
52+
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] += 1;
53+
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] ++;
54+
}
55+
56+
{
57+
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'];
58+
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] = 'modified';
59+
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] += 1;
60+
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] ++;
61+
}
2362

2463
const o = { a: 0 };
2564

@@ -41,6 +80,18 @@ o[numEnumKey];
4180
enum StrEnum { a = "a", b = "b" }
4281
let strEnumKey: StrEnum;
4382
o[strEnumKey];
83+
84+
85+
interface MyMap<K, T> {
86+
get(key: K): T;
87+
set(key: K, value: T): void;
88+
}
89+
90+
interface Dog { bark(): void; }
91+
let rover: Dog = { bark() {} };
92+
93+
declare let map: MyMap<Dog, string>;
94+
map[rover] = "Rover";
4495

4596

4697
//// [noImplicitAnyStringIndexerOnObject.js]
@@ -55,13 +106,48 @@ var d = {
55106
set: function (key) { return 'foobar'; }
56107
};
57108
var bar = d['hello'];
58-
var e = {
59-
set: function (key) { return 'foobar'; },
60-
get: function (key) { return 'foobar'; }
61-
};
62-
e['hello'] = 'modified';
63-
e['hello'] += 1;
64-
e['hello']++;
109+
{
110+
var e = {
111+
get: function (key) { return 'foobar'; },
112+
set: function (key) { return 'foobar'; }
113+
};
114+
e['hello'];
115+
e['hello'] = 'modified';
116+
e['hello'] += 1;
117+
e['hello']++;
118+
}
119+
{
120+
var e = {
121+
get: function (key) { return 'foobar'; },
122+
set: function (key, value) { return 'foobar'; }
123+
};
124+
e['hello'];
125+
e['hello'] = 'modified';
126+
e['hello'] += 1;
127+
e['hello']++;
128+
}
129+
{
130+
var e = {
131+
get: function (key) { return 'foobar'; },
132+
set: function (key, value) { return 'foobar'; }
133+
};
134+
e['hello'];
135+
e['hello'] = 'modified';
136+
e['hello'] += 1;
137+
e['hello']++;
138+
}
139+
{
140+
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'];
141+
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'] = 'modified';
142+
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'] += 1;
143+
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello']++;
144+
}
145+
{
146+
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'];
147+
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'] = 'modified';
148+
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'] += 1;
149+
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello']++;
150+
}
65151
var o = { a: 0 };
66152
o[k];
67153
o[k2];
@@ -80,3 +166,5 @@ var StrEnum;
80166
})(StrEnum || (StrEnum = {}));
81167
var strEnumKey;
82168
o[strEnumKey];
169+
var rover = { bark: function () { } };
170+
map[rover] = "Rover";

0 commit comments

Comments
 (0)