From 0c53615315bd4b14099eae842bc8bc4eb4e3a60e Mon Sep 17 00:00:00 2001 From: Titian Cernicova-Dragomir Date: Thu, 28 Mar 2024 18:47:19 +0000 Subject: [PATCH] Addressed code review. --- src/compiler/checker.ts | 16 ++++++++++---- src/compiler/utilities.ts | 45 +++++++++++++++++++++------------------ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d8582b9624e96..dc35f7c38c27c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1512,7 +1512,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { globals, getSymbolOfDeclaration, error, - getNodeLinks, + getRequiresScopeChangeCache, + setRequiresScopeChangeCache, lookup: getSymbol, onPropertyWithInvalidInitializer: checkAndReportErrorForInvalidInitializer, onFailedToResolveSymbol, @@ -1526,7 +1527,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { globals, getSymbolOfDeclaration, error, - getNodeLinks, + getRequiresScopeChangeCache, + setRequiresScopeChangeCache, lookup: getSuggestionForSymbolNameLookup, }); // for public members that accept a Node or one of its subtypes, we must guard against @@ -3027,6 +3029,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } } + function getRequiresScopeChangeCache(node: FunctionLikeDeclaration) { + return getNodeLinks(node).declarationRequiresScopeChange; + } + function setRequiresScopeChangeCache(node: FunctionLikeDeclaration, value: boolean) { + getNodeLinks(node).declarationRequiresScopeChange = value; + } // The invalid initializer error is needed in two situation: // 1. When result is undefined, after checking for a missing "this." // 2. When result is defined @@ -3056,12 +3064,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { meaning: SymbolFlags, nameNotFoundMessage: DiagnosticMessage, ) { - const name = typeof nameArg === "string" ? nameArg : (nameArg as Identifier).escapedText; + const name = isString(nameArg) ? nameArg : (nameArg as Identifier).escapedText; addLazyDiagnostic(() => { if ( !errorLocation || errorLocation.parent.kind !== SyntaxKind.JSDocLink && - !checkAndReportErrorForMissingPrefix(errorLocation, name, nameArg) && // TODO: GH#18217 + !checkAndReportErrorForMissingPrefix(errorLocation, name, nameArg) && !checkAndReportErrorForExtendingInterface(errorLocation) && !checkAndReportErrorForUsingTypeAsNamespace(errorLocation, name, meaning) && !checkAndReportErrorForExportingPrimitiveType(errorLocation, name) && diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 873aca60fa8e6..63f60c8e74b8d 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -484,7 +484,9 @@ import { ResolvedModuleWithFailedLookupLocations, ResolvedTypeReferenceDirective, ResolvedTypeReferenceDirectiveWithFailedLookupLocations, + returnFalse, ReturnStatement, + returnUndefined, SatisfiesExpression, ScriptKind, ScriptTarget, @@ -10818,10 +10820,11 @@ export function createNameResolver({ getSymbolOfDeclaration, globals, lookup, - getNodeLinks, - onPropertyWithInvalidInitializer, - onFailedToResolveSymbol, - onSuccessfullyResolvedSymbol, + setRequiresScopeChangeCache = returnUndefined, + getRequiresScopeChangeCache = returnUndefined, + onPropertyWithInvalidInitializer = returnFalse, + onFailedToResolveSymbol = returnUndefined, + onSuccessfullyResolvedSymbol = returnUndefined, }: { compilerOptions: CompilerOptions; getSymbolOfDeclaration: (node: Declaration) => Symbol; @@ -10830,7 +10833,8 @@ export function createNameResolver({ argumentsSymbol: Symbol; requireSymbol: Symbol; lookup: (symbols: SymbolTable, name: __String, meaning: SymbolFlags) => Symbol | undefined; - getNodeLinks: (node: Node) => { declarationRequiresScopeChange?: boolean; }; + setRequiresScopeChangeCache: undefined | ((node: FunctionLikeDeclaration, value: boolean) => void); + getRequiresScopeChangeCache: undefined | ((node: FunctionLikeDeclaration) => boolean | undefined); onPropertyWithInvalidInitializer?: (location: Node | undefined, name: __String, declaration: PropertyDeclaration, result: Symbol | undefined) => boolean; onFailedToResolveSymbol?: ( location: Node | undefined, @@ -10869,7 +10873,7 @@ export function createNameResolver({ let associatedDeclarationForContainingInitializerOrBindingName: ParameterDeclaration | BindingElement | undefined; let withinDeferredContext = false; let grandparent: Node; - const name = typeof nameArg === "string" ? nameArg : (nameArg as Identifier).escapedText; + const name = isString(nameArg) ? nameArg : (nameArg as Identifier).escapedText; loop: while (location) { if (name === "const" && isConstAssertion(location)) { @@ -11231,18 +11235,16 @@ export function createNameResolver({ } } - if (nameNotFoundMessage && propertyWithInvalidInitializer && onPropertyWithInvalidInitializer!(originalLocation, name, propertyWithInvalidInitializer, result)) { - return undefined; - } - - if (!result) { - if (nameNotFoundMessage) { - onFailedToResolveSymbol!(originalLocation, nameArg, meaning, nameNotFoundMessage); + if (nameNotFoundMessage) { + if (propertyWithInvalidInitializer && onPropertyWithInvalidInitializer(originalLocation, name, propertyWithInvalidInitializer, result)) { + return undefined; + } + if (!result) { + onFailedToResolveSymbol(originalLocation, nameArg, meaning, nameNotFoundMessage); + } + else { + onSuccessfullyResolvedSymbol(originalLocation, result, meaning, lastLocation, associatedDeclarationForContainingInitializerOrBindingName, withinDeferredContext); } - return undefined; - } - else if (nameNotFoundMessage) { - onSuccessfullyResolvedSymbol!(originalLocation, result, meaning, lastLocation, associatedDeclarationForContainingInitializerOrBindingName, withinDeferredContext); } return result; @@ -11264,11 +11266,12 @@ export function createNameResolver({ // - nullish coalesce pre-es2020 // - spread assignment in binding pattern pre-es2017 if (target >= ScriptTarget.ES2015) { - const links = getNodeLinks(functionLocation); - if (links.declarationRequiresScopeChange === undefined) { - links.declarationRequiresScopeChange = forEach(functionLocation.parameters, requiresScopeChange) || false; + let declarationRequiresScopeChange = getRequiresScopeChangeCache(functionLocation); + if (declarationRequiresScopeChange === undefined) { + declarationRequiresScopeChange = forEach(functionLocation.parameters, requiresScopeChange) || false; + setRequiresScopeChangeCache(functionLocation, declarationRequiresScopeChange); } - return !links.declarationRequiresScopeChange; + return !declarationRequiresScopeChange; } } return false;