Better errors for indexing gettable/settable values#26446
Better errors for indexing gettable/settable values#26446RyanCavanaugh merged 5 commits intomicrosoft:masterfrom
Conversation
| "category": "Error", | ||
| "code": 7042 | ||
| }, | ||
| "Element implicitly has an 'any' type because type '{0}' has no index signature. Did you mean to call '{1}' ?": { |
There was a problem hiding this comment.
Remove the stray space at the end.
src/compiler/checker.ts
Outdated
| const s = getSingleCallSignature(getTypeOfSymbol(prop)); | ||
| if (s && getMinArgumentCount(s) === 1 && typeToString(getTypeAtPosition(s, 0)) === "string") { | ||
| const suggestion = symbolToString(objectType.symbol) + "." + symbolToString(prop); | ||
| suggestions = (!suggestions) ? suggestion : suggestions.concat(" or " + suggestion); |
There was a problem hiding this comment.
This "or " is not localizable (i.e. just concatenating it into a diagnostic means that it might not translate correctly in another language.
src/compiler/checker.ts
Outdated
| getPropertyOfObjectType(objectType, <__String>"set") | ||
| ]; | ||
|
|
||
| for (const prop of props) { |
There was a problem hiding this comment.
I think that this is a little too general. You really only want to recommend "set" when the element access is on the left hand side of some mutating operator (e.g. =, +=, ++, etc.). You want to try to recommend "get" otherwise.
| }; | ||
| d['hello']; | ||
|
|
||
| // Should give suggestion 'e.get or e.set' |
There was a problem hiding this comment.
I would try to avoid (and remove) these comments regarding error messages in general. The baselines should communicate that on their own.
|
Thanks for being patient! This is a good start! I left some feedback to iterate on. |
b02e898 to
0ae6376
Compare
|
Hi @DanielRosenwasser, I've made the requested changes. Feel free to take another look at this, thanks! |
DanielRosenwasser
left a comment
There was a problem hiding this comment.
Sorry for the delay! Hopefully my feedback is still useful.
src/compiler/checker.ts
Outdated
| const prop = getPropertyOfObjectType(objectType, <__String>name); | ||
| if (prop) { | ||
| const s = getSingleCallSignature(getTypeOfSymbol(prop)); | ||
| if (s && getMinArgumentCount(s) === 1 && typeToString(getTypeAtPosition(s, 0)) === "string") { |
There was a problem hiding this comment.
The arg count is incorrect for set methods (it should probably be 2).
src/compiler/checker.ts
Outdated
|
|
||
| if (isAssignmentTarget(expr)) { | ||
| if (hasProp("set")) { | ||
| suggestion = symbolToString(objectType.symbol) + ".set"; |
There was a problem hiding this comment.
Unfortunately symbolToString might not work if the expression isn't a simple binding. You'd be better off with something like
const suggestedMethod = isAssignmentTarget(expr) ? "set" : "get";
if (!hasProp(suggestedMethod)) {
return undefined;
}
let suggestion = tryGetPropertyAccessOrIdentifierToString(expr);
if (suggestion === undefined) {
suggestion = suggestedMethod;
}
else {
suggestion += "." + suggestedMethod;
}where you could define tryGetPropertyAccessOrIdentifierToString in utilities.ts.
function tryGetPropertyAccessOrIdentifierToString(expr: Expression): string | undefined {
if (isPropertyAccessExpression(expr)) {
return tryGetPropertyAccessOrIdentifierToString(expr.expression) + "." + x.name;
}
if (isIdentifier(expr)) {
return unescapeLeadingUnderscores(expr.escapedText);
}
return undefined;
}There was a problem hiding this comment.
Right! Let me do this in a few. Thank you.
src/compiler/checker.ts
Outdated
| else { | ||
| error(accessExpression, Diagnostics.Element_implicitly_has_an_any_type_because_type_0_has_no_index_signature, typeToString(objectType)); | ||
| const suggestion = getSuggestionForNonexistentIndexSignature(objectType, accessExpression); | ||
| if (suggestion) { |
There was a problem hiding this comment.
Other nit - use suggestion !== undefined.
|
@collin5 would be good to merge this after the last round of comments + merge conflicts are addressed. Thanks! |
|
Updated. Thank you. |
Gives suggestions where a type has no index signature but has
getorsetproperties.Fixes #26098