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

Jump to bottom

Fix check for get/set recommendation #34885

Merged
merged 11 commits into from
Nov 19, 2019
Merged

Fix check for get/set recommendation #34885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
05fe67e
Added more tests.
DanielRosenwasser Nov 2, 2019
4a496f9
Accepted baselines.
DanielRosenwasser Nov 2, 2019
4a70cf4
Work better with any parameter type.
DanielRosenwasser Nov 2, 2019
11c3867
Accepted baselines.
DanielRosenwasser Nov 2, 2019
1c429b4
Use the actual indexed expression.
DanielRosenwasser Nov 2, 2019
d2b2139
Add tests that exhibit bad stringification.
DanielRosenwasser Nov 2, 2019
b7e3ab6
Accepted baselines.
DanielRosenwasser Nov 2, 2019
9c6fac3
Short-circuit stringification on 'undefined'.
DanielRosenwasser Nov 2, 2019
7d0d062
Accepted baselines.
DanielRosenwasser Nov 2, 2019
409b6a2
Remove space.
DanielRosenwasser Nov 4, 2019
321efc1
Accepted baselines.
DanielRosenwasser Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12015,7 +12015,7 @@ namespace ts {
}
}
else {
const suggestion = getSuggestionForNonexistentIndexSignature(objectType, accessExpression);
const suggestion = getSuggestionForNonexistentIndexSignature(objectType, accessExpression, indexType);
if (suggestion !== undefined) {
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);
}
Expand Down Expand Up @@ -23119,15 +23119,13 @@ namespace ts {
return suggestion && symbolName(suggestion);
}

function getSuggestionForNonexistentIndexSignature(objectType: Type, expr: ElementAccessExpression): string | undefined {
function getSuggestionForNonexistentIndexSignature(objectType: Type, expr: ElementAccessExpression, keyedType: Type): string | undefined {
// check if object type has setter or getter
const hasProp = (name: "set" | "get", argCount = 1) => {
function hasProp(name: "set" | "get") {
const prop = getPropertyOfObjectType(objectType, <__String>name);
if (prop) {
const s = getSingleCallSignature(getTypeOfSymbol(prop));
if (s && getMinArgumentCount(s) === argCount && typeToString(getTypeAtPosition(s, 0)) === "string") {
return true;
}
return !!s && getMinArgumentCount(s) >= 1 && isTypeAssignableTo(keyedType, getTypeAtPosition(s, 0));
}
return false;
};
Expand All @@ -23137,7 +23135,7 @@ namespace ts {
return undefined;
}

let suggestion = tryGetPropertyAccessOrIdentifierToString(expr);
let suggestion = tryGetPropertyAccessOrIdentifierToString(expr.expression);
if (suggestion === undefined) {
suggestion = suggestedMethod;
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4502,7 +4502,7 @@
"category": "Error",
"code": 7051
},
"Element implicitly has an 'any' type because type '{0}' has no index signature. Did you mean to call '{1}' ?": {
"Element implicitly has an 'any' type because type '{0}' has no index signature. Did you mean to call '{1}'?": {
"category": "Error",
"code": 7052
},
Expand Down
7 changes: 5 additions & 2 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4244,9 +4244,12 @@ namespace ts {

export function tryGetPropertyAccessOrIdentifierToString(expr: Expression): string | undefined {
if (isPropertyAccessExpression(expr)) {
return tryGetPropertyAccessOrIdentifierToString(expr.expression) + "." + expr.name;
const baseStr = tryGetPropertyAccessOrIdentifierToString(expr.expression);
if (baseStr !== undefined) {
return baseStr + "." + expr.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great 👍

}
}
if (isIdentifier(expr)) {
else if (isIdentifier(expr)) {
return unescapeLeadingUnderscores(expr.escapedText);
}
return undefined;
Expand Down
1 change: 1 addition & 0 deletions src/services/navigationBar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ namespace ts.NavigationBar {
return "<function>";
}

// See also 'tryGetPropertyAccessOrIdentifierToString'
function getCalledExpressionName(expr: Expression): string | undefined {
if (isIdentifier(expr)) {
return expr.text;
Expand Down
157 changes: 131 additions & 26 deletions tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt
View file
Open in desktop

Large diffs are not rendered by default.

116 changes: 102 additions & 14 deletions tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,52 @@ var d = {
};
const bar = d['hello'];

var e = {
set: (key: string) => 'foobar',
get: (key: string) => 'foobar'
};
e['hello'] = 'modified';
e['hello'] += 1;
e['hello'] ++;
{
let e = {
get: (key: string) => 'foobar',
set: (key: string) => 'foobar'
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello'] ++;
}

{
let e = {
get: (key: string) => 'foobar',
set: (key: string, value: string) => 'foobar'
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello'] ++;
}

{
let e = {
get: (key: "hello" | "world") => 'foobar',
set: (key: "hello" | "world", value: string) => 'foobar'
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello'] ++;
}

{
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'];
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] = 'modified';
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] += 1;
({ get: (key: string) => 'hello', set: (key: string, value: string) => {} })['hello'] ++;
}

{
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'];
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] = 'modified';
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] += 1;
({ foo: { get: (key: string) => 'hello', set: (key: string, value: string) => {} } }).foo['hello'] ++;
}

const o = { a: 0 };

Expand All @@ -41,6 +80,18 @@ o[numEnumKey];
enum StrEnum { a = "a", b = "b" }
let strEnumKey: StrEnum;
o[strEnumKey];


interface MyMap<K, T> {
get(key: K): T;
set(key: K, value: T): void;
}

interface Dog { bark(): void; }
let rover: Dog = { bark() {} };

declare let map: MyMap<Dog, string>;
map[rover] = "Rover";


//// [noImplicitAnyStringIndexerOnObject.js]
Expand All @@ -55,13 +106,48 @@ var d = {
set: function (key) { return 'foobar'; }
};
var bar = d['hello'];
var e = {
set: function (key) { return 'foobar'; },
get: function (key) { return 'foobar'; }
};
e['hello'] = 'modified';
e['hello'] += 1;
e['hello']++;
{
var e = {
get: function (key) { return 'foobar'; },
set: function (key) { return 'foobar'; }
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello']++;
}
{
var e = {
get: function (key) { return 'foobar'; },
set: function (key, value) { return 'foobar'; }
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello']++;
}
{
var e = {
get: function (key) { return 'foobar'; },
set: function (key, value) { return 'foobar'; }
};
e['hello'];
e['hello'] = 'modified';
e['hello'] += 1;
e['hello']++;
}
{
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'];
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'] = 'modified';
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello'] += 1;
({ get: function (key) { return 'hello'; }, set: function (key, value) { } })['hello']++;
}
{
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'];
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'] = 'modified';
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello'] += 1;
({ foo: { get: function (key) { return 'hello'; }, set: function (key, value) { } } }).foo['hello']++;
}
var o = { a: 0 };
o[k];
o[k2];
Expand All @@ -80,3 +166,5 @@ var StrEnum;
})(StrEnum || (StrEnum = {}));
var strEnumKey;
o[strEnumKey];
var rover = { bark: function () { } };
map[rover] = "Rover";