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

Isolated declarations fix signature serialization scoping #58409

Merged
merged 12 commits into from
May 10, 2024
+4,756 −3,916
Merged

Isolated declarations fix signature serialization scoping #58409

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fdcae86
Updated baseline.
dragomirtitian May 2, 2024
b0231f7
Updated baselines
dragomirtitian May 2, 2024
9317c6b
Revert basline to contain expansions.
dragomirtitian May 2, 2024
8753792
Fix signature scoping.
dragomirtitian May 2, 2024
3621579
Removed baseline files.
dragomirtitian May 2, 2024
73c9509
Removed redundant tests.
dragomirtitian May 2, 2024
b9e6523
Reduce NodeBuilderContext copying and polymorphism
dragomirtitian May 3, 2024
c68c4f5
Fix regression in node reuse.
dragomirtitian May 3, 2024
5263634
Improve test for symbol is the same at the new location.
dragomirtitian May 7, 2024
d288055
Consider export symbols for symbol tracking in type node reuse.
dragomirtitian May 8, 2024
fa7e6a0
Switched to using nodeflags to find synthetic scopes in name resolution
dragomirtitian May 10, 2024
9c3d5ff
Switch to getExportSymbolOfValueSymbolIfExported to get the export sy…
dragomirtitian May 10, 2024
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
22 changes: 13 additions & 9 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8301,21 +8301,25 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return { introducesError, node: attachSymbolToLeftmostIdentifier(node) as T };
}
sym = resolveEntityName(leftmost, meaning, /*ignoreErrors*/ true, /*dontResolveAlias*/ true);
if (sym === unknownSymbol) {
// When we create a fake scope some parameters may actually not be usable
// either because they are the expanded rest parameter,
// or because they are the newly added parameters from the tuple, which might have different meanings in teh original context
// So we add unknownSymbol for these, and we fall back to inference when we encounter them
introducesError = true;
return { introducesError, node, sym };
}
if (
context.enclosingDeclaration &&
(getNodeLinks(context.enclosingDeclaration).fakeScopeForSignatureDeclaration || !findAncestor(node, n => n === context.enclosingDeclaration)) &&
!(sym && sym.flags & SymbolFlags.TypeParameter)
dragomirtitian marked this conversation as resolved.
Show resolved Hide resolved
) {
// Some declarations may be transplanted to a new location.
dragomirtitian marked this conversation as resolved.
Show resolved Hide resolved
// When this happens we need to make sure that the name has the same meaning at both locations
// We also check for the unknownSymbol because when we create a fake scope some parameters may actually not be usable
// either because they are the expanded rest parameter,
// or because they are the newly added parameters from the tuple, which might have different meanings in the original context
const symAtLocation = resolveEntityName(leftmost, meaning, /*ignoreErrors*/ true, /*dontResolveAlias*/ true, context.enclosingDeclaration);
if ((symAtLocation !== sym && (!symAtLocation || !isTransientSymbol(symAtLocation) || symAtLocation.links.target !== sym)) || symAtLocation === unknownSymbol) {
if (
// Check for unusable parameters symbols
symAtLocation === unknownSymbol ||
// If the symbol is not found, but was not found in the original scope either we probably have an error, don't reuse the node
dragomirtitian marked this conversation as resolved.
Show resolved Hide resolved
(symAtLocation === undefined && sym !== undefined) ||
// If the symbol is found both in declaration scope and in current scope then it shoudl point to the same reference
(symAtLocation && sym && !getSymbolIfSameReference(symAtLocation, sym))
) {
dragomirtitian marked this conversation as resolved.
Show resolved Hide resolved
introducesError = true;
return { introducesError, node, sym };
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/utilitiesPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,7 +2320,8 @@ function isDeclarationKind(kind: SyntaxKind) {
|| kind === SyntaxKind.VariableDeclaration
|| kind === SyntaxKind.JSDocTypedefTag
|| kind === SyntaxKind.JSDocCallbackTag
|| kind === SyntaxKind.JSDocPropertyTag;
|| kind === SyntaxKind.JSDocPropertyTag
|| kind === SyntaxKind.NamedTupleMember;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NamedTupleMember seemed to be missing here. This meant that tuple member names were being funneled through trackExistingEntityName even when other declaration names were not.

                if (isEntityName(node) || isEntityNameExpression(node)) {
                    if (isDeclarationName(node)) {
                        return node;
                    }
                    const { introducesError, node: result } = trackExistingEntityName(node, context);
                    hadError = hadError || introducesError;
                    // We should not go to child nodes of the entity name, they will not be accessible
                    return result;
                }

}

function isDeclarationStatementKind(kind: SyntaxKind) {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/assertionsAndNonReturningFunctions.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function assertIsString(x) {
*/
function assert2(check) {
>assert2 : (check: boolean) => asserts check
> : ^ ^^ ^^^^^^^^^^^^^^^^^^
> : ^ ^^ ^^^^^
>check : boolean
> : ^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/importingExportingTypes.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ declare module "fs" {
=== /index.js ===
import { writeFile, WriteFileOptions, WriteFileOptions as OtherName } from "fs";
>writeFile : (path: string, data: any, options: WriteFileOptions, callback: (err: Error) => void) => void
> : ^ ^^ ^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^ ^^ ^^^^^^^^^
> : ^ ^^ ^^ ^^ ^^ ^^ ^^ ^^ ^^^^^^^^^
>WriteFileOptions : any
> : ^^^
>WriteFileOptions : any
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsDeclarationsTypedefAndLatebound.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ stringSet.addAll(stringSet);
>stringSet.addAll(stringSet) : void
> : ^^^^
>stringSet.addAll : (iterable: LazySet) => void
> : ^ ^^^^^^^^^^^^^^^^^^
> : ^ ^^ ^^^^^^^^^
>stringSet : LazySet
> : ^^^^^^^
>addAll : (iterable: LazySet) => void
> : ^ ^^^^^^^^^^^^^^^^^^
> : ^ ^^ ^^^^^^^^^
>stringSet : LazySet
> : ^^^^^^^

Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/jsEnumTagOnObjectFrozen.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
const { Thing, useThing, cbThing } = require("./index");
>Thing : Readonly<{ a: "thing"; b: "chill"; }>
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>useThing : (x: string) => void
> : ^ ^^^^^^^^^^^^^^^^^
>cbThing : (x: (x: string) => void) => void
> : ^ ^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
>useThing : (x: Thing) => void
> : ^ ^^ ^^^^^^^^^
>cbThing : (x: (x: Thing) => void) => void
> : ^ ^^ ^^^^^^^^^
>require("./index") : typeof import("index")
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>require : any
Expand All @@ -17,8 +17,8 @@ const { Thing, useThing, cbThing } = require("./index");
useThing(Thing.a);
>useThing(Thing.a) : void
> : ^^^^
>useThing : (x: string) => void
> : ^ ^^^^^^^^^^^^^^^^^
>useThing : (x: Thing) => void
> : ^ ^^ ^^^^^^^^^
>Thing.a : "thing"
> : ^^^^^^^
>Thing : Readonly<{ a: "thing"; b: "chill"; }>
Expand All @@ -35,8 +35,8 @@ useThing(Thing.a);
cbThing(type => {
>cbThing(type => { /** @type {LogEntry} */ const logEntry = { time: Date.now(), type, };}) : void
> : ^^^^
>cbThing : (x: (x: string) => void) => void
> : ^ ^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
>cbThing : (x: (x: Thing) => void) => void
> : ^ ^^ ^^^^^^^^^
>type => { /** @type {LogEntry} */ const logEntry = { time: Date.now(), type, };} : (type: string) => void
> : ^ ^^^^^^^^^^^^^^^^^
>type : string
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/returnTagTypeGuard.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function f(chunk) {
*/
function isBoolean(value) {
>isBoolean : (value: any) => value is boolean
> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^
> : ^ ^^ ^^^^^
>value : any

return typeof value === "boolean";
Expand Down Expand Up @@ -150,7 +150,7 @@ function foo(val) {
/** @type {Cb} */
function isNumber(x) { return typeof x === "number" }
>isNumber : (x: unknown) => x is number
> : ^ ^^ ^^^^^^^^^^^^^^^^
> : ^ ^^ ^^^^^
>x : unknown
> : ^^^^^^^
>typeof x === "number" : boolean
Expand Down