-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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.
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
Use missingType
in --noUncheckedIndexedAccess
mode
#51653
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
=== tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts === | ||
declare function invariant(condition: boolean): asserts condition; | ||
>invariant : Symbol(invariant, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 0)) | ||
>condition : Symbol(condition, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 27)) | ||
>condition : Symbol(condition, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 27)) | ||
|
||
function f1(obj: Record<string, string>) { | ||
>f1 : Symbol(f1, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 66)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) | ||
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) | ||
|
||
invariant("test" in obj); | ||
>invariant : Symbol(invariant, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 0)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) | ||
|
||
return obj.test; // string | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) | ||
} | ||
|
||
function f2(obj: Record<string, string>) { | ||
>f2 : Symbol(f2, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 5, 1)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) | ||
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) | ||
|
||
if ("test" in obj) { | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) | ||
|
||
return obj.test; // string | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) | ||
} | ||
return "default"; | ||
} | ||
|
||
function f3(obj: Record<string, string>) { | ||
>f3 : Symbol(f3, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 12, 1)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) | ||
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) | ||
|
||
obj.test; // string | undefined | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) | ||
|
||
if ("test" in obj) { | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) | ||
|
||
obj.test; // string | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) | ||
} | ||
else { | ||
obj.test; // undefined | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) | ||
} | ||
} | ||
|
||
function f4(obj: Record<string, string>) { | ||
>f4 : Symbol(f4, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 22, 1)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) | ||
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) | ||
|
||
obj.test; // string | undefined | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) | ||
|
||
if (obj.hasOwnProperty("test")) { | ||
>obj.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) | ||
>hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) | ||
|
||
obj.test; // string | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) | ||
} | ||
else { | ||
obj.test; // undefined | ||
>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
=== tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts === | ||
declare function invariant(condition: boolean): asserts condition; | ||
>invariant : (condition: boolean) => asserts condition | ||
>condition : boolean | ||
|
||
function f1(obj: Record<string, string>) { | ||
>f1 : (obj: Record<string, string>) => string | ||
>obj : Record<string, string> | ||
|
||
invariant("test" in obj); | ||
>invariant("test" in obj) : void | ||
>invariant : (condition: boolean) => asserts condition | ||
>"test" in obj : boolean | ||
>"test" : "test" | ||
>obj : Record<string, string> | ||
|
||
return obj.test; // string | ||
>obj.test : string | ||
>obj : Record<string, string> | ||
>test : string | ||
} | ||
|
||
function f2(obj: Record<string, string>) { | ||
>f2 : (obj: Record<string, string>) => string | ||
>obj : Record<string, string> | ||
|
||
if ("test" in obj) { | ||
>"test" in obj : boolean | ||
>"test" : "test" | ||
>obj : Record<string, string> | ||
|
||
return obj.test; // string | ||
>obj.test : string | ||
>obj : Record<string, string> | ||
>test : string | ||
} | ||
return "default"; | ||
>"default" : "default" | ||
} | ||
|
||
function f3(obj: Record<string, string>) { | ||
>f3 : (obj: Record<string, string>) => void | ||
>obj : Record<string, string> | ||
|
||
obj.test; // string | undefined | ||
>obj.test : string | undefined | ||
>obj : Record<string, string> | ||
>test : string | undefined | ||
|
||
if ("test" in obj) { | ||
>"test" in obj : boolean | ||
>"test" : "test" | ||
>obj : Record<string, string> | ||
|
||
obj.test; // string | ||
>obj.test : string | ||
>obj : Record<string, string> | ||
>test : string | ||
} | ||
else { | ||
obj.test; // undefined | ||
>obj.test : undefined | ||
>obj : Record<string, string> | ||
>test : undefined | ||
} | ||
} | ||
|
||
function f4(obj: Record<string, string>) { | ||
>f4 : (obj: Record<string, string>) => void | ||
>obj : Record<string, string> | ||
|
||
obj.test; // string | undefined | ||
>obj.test : string | undefined | ||
>obj : Record<string, string> | ||
>test : string | undefined | ||
|
||
if (obj.hasOwnProperty("test")) { | ||
>obj.hasOwnProperty("test") : boolean | ||
>obj.hasOwnProperty : (v: PropertyKey) => boolean | ||
>obj : Record<string, string> | ||
>hasOwnProperty : (v: PropertyKey) => boolean | ||
>"test" : "test" | ||
|
||
obj.test; // string | ||
>obj.test : string | ||
>obj : Record<string, string> | ||
>test : string | ||
} | ||
else { | ||
obj.test; // undefined | ||
>obj.test : undefined | ||
>obj : Record<string, string> | ||
>test : undefined | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
// @strict: true | ||
// @noEmit: true | ||
// @noUncheckedIndexedAccess: true | ||
|
||
declare function invariant(condition: boolean): asserts condition; | ||
|
||
function f1(obj: Record<string, string>) { | ||
invariant("test" in obj); | ||
return obj.test; // string | ||
} | ||
|
||
function f2(obj: Record<string, string>) { | ||
if ("test" in obj) { | ||
return obj.test; // string | ||
} | ||
return "default"; | ||
} | ||
|
||
function f3(obj: Record<string, string>) { | ||
obj.test; // string | undefined | ||
if ("test" in obj) { | ||
obj.test; // string | ||
} | ||
else { | ||
obj.test; // undefined | ||
} | ||
} | ||
|
||
function f4(obj: Record<string, string>) { | ||
obj.test; // string | undefined | ||
if (obj.hasOwnProperty("test")) { | ||
obj.test; // string | ||
} | ||
else { | ||
obj.test; // undefined | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've hoped that this solution would make it work (somehow through CFA) for at least the function f5(obj: Record<string, string>, prop: string) {
if (obj[prop]) {
obj[prop]; // actual: string | undefined; expected: string
}
if (prop in obj) {
obj[prop]; // actual: string | undefined; expected: string
}
} Is this kind of analysis supported for any kind of patterns today? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ye, I've figured out as much - although I wonder how hard a limitation that is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything gets duped to #10530 which is kind of misleading since that issue was originally about bracketed access with a literal key (which has since been fixed), but the picture that develops after enough hunting is that it’s a performance sinkhole. See e.g. #25109 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Andarist It works when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I understand that this seems difficult to implement and we might never get this feature, but I'm having a hard time understanding how this could be "by design". What I mean is, this seems like fairly straight forward JS and as far as I understand TypeScript's design goals, we really should be able to support (type correctly) this kind of code. I really am sympathetic to the fact that it might not be feasible to do in the current implementation, I'm just challenging the idea that we want it to be this way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. “By design” = ”intentional decision made during the design process”. Some such decisions are known compromises. I don’t use the phrase to imply it’s what the user wants, because nobody can even define that except for the individual users themselves, and the team can’t go around asking every single user before they’re allowed to label things “working as intended”. 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure there always needs to be compromises made in the design of the language, but this isn't that. This seems to be a limitation of the current implementation (an implementation detail if you will), not some inherent limitation in the design of the language itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, the implementation is still part of the design of the TypeScript compiler in particular. Obviously a language wouldn't be intentionally designed that way in isolation, but that's a moot point since there's no TypeScript language specification. There used to be a formal spec a long time ago, but nowadays the implementation is the specification, and that's an entirely pragmatic design process. If the implementation is designed a certain way, in practice that means the language is too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...at least until someone gets fed up with the limitations and makes an alternative implementation. 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, how does this one work if
hasOwnProperty
's signature is "just":what kind of sorcery is going on here? :D does this method have some special support in the compiler itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m going to assume that feature was implemented early on since nowadays feature requests to special-case built-in methods (like Object.assign) get rejected if the special case can’t be represented in the method signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, I found the code related to
hasOwnProperty
here