From c195aa409e3cbc2277d609144aef883164f36c38 Mon Sep 17 00:00:00 2001 From: Boris Penkov Date: Fri, 19 Nov 2021 16:59:40 +0200 Subject: [PATCH] feat(migrations): use TypeChecker to determine property types #10572 (#10558) --- .../migrations/common/UpdateChanges.ts | 15 ++-- .../migrations/common/tsUtils.ts | 81 ++++++++++++++++--- 2 files changed, 78 insertions(+), 18 deletions(-) diff --git a/projects/igniteui-angular/migrations/common/UpdateChanges.ts b/projects/igniteui-angular/migrations/common/UpdateChanges.ts index 7b2b92a8fa8..e12d77660fd 100644 --- a/projects/igniteui-angular/migrations/common/UpdateChanges.ts +++ b/projects/igniteui-angular/migrations/common/UpdateChanges.ts @@ -42,8 +42,8 @@ export class UpdateChanges { // and no actual angular metadata will be resolved for the rest of the migration const wsProject = this.workspace.projects[this.workspace.defaultProject] || this.workspace.projects[0]; const mainRelPath = wsProject.architect?.build?.options['main'] ? - path.join(wsProject.root, wsProject.architect?.build?.options['main']) : - `src/main.ts`; + path.join(wsProject.root, wsProject.architect?.build?.options['main']) : + `src/main.ts`; // patch TSConfig so it includes angularOptions.strictTemplates // ivy ls requires this in order to function properly on templates this.patchTsConfig(); @@ -491,9 +491,8 @@ export class UpdateChanges { // use the absolute path for ALL LS operations // do not overwrite the entryPath, as Tree operations require relative paths const changes = new Set<{ change; position }>(); - let langServ; + let langServ: tss.LanguageService; for (const change of memberChanges.changes) { - if (!content.includes(change.member)) { continue; } @@ -532,7 +531,7 @@ export class UpdateChanges { originalContent = this.serverHost.readFile(this.tsconfigPath); } catch { this.context?.logger - .warn(`Could not read ${this.tsconfigPath}. Some Angular Ivy features might be unavailable during migrations.`); + .warn(`Could not read ${this.tsconfigPath}. Some Angular Ivy features might be unavailable during migrations.`); return; } let content; @@ -542,9 +541,9 @@ export class UpdateChanges { content = result.config; } else { this.context?.logger - .warn(`Could not parse ${this.tsconfigPath}. Angular Ivy language service might be unavailable during migrations.`); + .warn(`Could not parse ${this.tsconfigPath}. Angular Ivy language service might be unavailable during migrations.`); this.context?.logger - .warn(`Error:\n${result.error}`); + .warn(`Error:\n${result.error}`); return; } if (!content.angularCompilerOptions) { @@ -552,7 +551,7 @@ export class UpdateChanges { } if (!content.angularCompilerOptions.strictTemplates) { this.context?.logger - .info(`Adding 'angularCompilerOptions.strictTemplates' to ${this.tsconfigPath} for migration run.`); + .info(`Adding 'angularCompilerOptions.strictTemplates' to ${this.tsconfigPath} for migration run.`); content.angularCompilerOptions.strictTemplates = true; this.host.overwrite(this.tsconfigPath, JSON.stringify(content)); // store initial state and restore it once migrations are finished diff --git a/projects/igniteui-angular/migrations/common/tsUtils.ts b/projects/igniteui-angular/migrations/common/tsUtils.ts index c0a58a9f23f..ebb9d5626e9 100644 --- a/projects/igniteui-angular/migrations/common/tsUtils.ts +++ b/projects/igniteui-angular/migrations/common/tsUtils.ts @@ -12,12 +12,17 @@ export const NG_CORE_PACKAGE_NAME = '@angular/core'; export const CUSTOM_TS_PLUGIN_PATH = './tsPlugin'; export const CUSTOM_TS_PLUGIN_NAME = 'igx-ts-plugin'; -enum SynaxTokens { +enum SyntaxTokens { ClosingParenthesis = ')', MemberAccess = '.', Question = '?' } +export class MemberInfo implements Pick { + public name: string; + public fileName: string; +} + /** Returns a source file */ // export function getFileSource(sourceText: string): ts.SourceFile { // return ts.createSourceFile('', sourceText, ts.ScriptTarget.Latest, true); @@ -45,7 +50,9 @@ export const getIdentifierPositions = (source: string | ts.SourceFile, name: str return false; } } - return node.text === name; + // for methods the node.text will not contain characters like () + const cleanName = name.match(/\w+/g)[0] || name; + return node.text === cleanName; }; const findIdentifiers = (node: ts.Node) => { @@ -251,7 +258,7 @@ const getTypeDefinitions = (langServ: tss.LanguageService, entryPath: string, po * @param position Index of identifier */ export const getTypeDefinitionAtPosition = - (langServ: tss.LanguageService, entryPath: string, position: number): Pick | null => { + (langServ: tss.LanguageService, entryPath: string, position: number): MemberInfo | null => { const definition = langServ.getDefinitionAndBoundSpan(entryPath, position)?.definitions[0]; if (!definition) { return null; @@ -264,6 +271,19 @@ export const getTypeDefinitionAtPosition = if (definition.kind.toString() === 'method') { return getMethodTypeDefinition(langServ, definition); } + if (entryPath.endsWith('.ts')) { + // for ts files we can use the type checker to look up a specific node + // and attempt to resolve its actual type + const sourceFile = langServ.getProgram().getSourceFile(entryPath); + // const node = (tss as any).getTouchingPropertyName(sourceFile, position); -> tss internal that looks up a node + const node = findNodeAtPosition(sourceFile, position); + if (node) { + const memberInfo = resolveMemberInfo(langServ, node); + if (memberInfo) { + return memberInfo; + } + } + } let typeDefs = getTypeDefinitions(langServ, definition.fileName || entryPath, definition.textSpan.start); // if there are no type definitions found, the identifier is a ts property, referred in an internal/external template @@ -311,7 +331,6 @@ export const getTypeDefinitionAtPosition = return null; }; - /** * Determines if a member belongs to a type in the `igniteui-angular` toolkit. * @@ -325,7 +344,7 @@ export const isMemberIgniteUI = const content = langServ.getProgram().getSourceFile(entryPath).getText(); matchPosition = shiftMatchPosition(matchPosition, content); const prevChar = content.substr(matchPosition - 1, 1); - if (prevChar === SynaxTokens.ClosingParenthesis) { + if (prevChar === SyntaxTokens.ClosingParenthesis) { // methodCall().identifier matchPosition = langServ.getBraceMatchingAtPosition(entryPath, matchPosition - 1)[0]?.start ?? matchPosition; } @@ -339,6 +358,49 @@ export const isMemberIgniteUI = && change.definedIn.indexOf(typeDef.name) !== -1; }; +const resolveMemberInfo = (langServ: tss.LanguageService, node: tss.Node): MemberInfo | null => { + const typeChecker = langServ.getProgram().getTypeChecker(); + const nodeType = typeChecker.getTypeAtLocation(node); + const typeArguments = typeChecker.getTypeArguments(nodeType as tss.TypeReference); + if (typeArguments && typeArguments.length < 1) { + // it's not a generic type so try to look up its name and fileName + // atm we do not support migrating union/intersection generic types + // a type symbol (type) should have only one declaration + // if the type is 'any' or 'some', there will be no type symbol + const name = nodeType.getSymbol()?.getName(); + const declarations = nodeType.getSymbol()?.getDeclarations(); + if (declarations && declarations.length > 0) { + const fileName = declarations[0].getSourceFile().fileName; + if (name && fileName) { + return { name, fileName }; + } + } + } + + return null; +} + +/** + * Looks up a node which end property matches the specified position. + * Can go to the next node if the currently found one is invalid (comment for example) + */ +const findNodeAtPosition = (sourceFile: tss.SourceFile, position: number): tss.Node | null => { + if (!sourceFile) { + return null; + } + + return findInnerNode(sourceFile, position); +} +const findInnerNode = (node: tss.Node, position: number): tss.Node | null => { + if (position <= node.getEnd()) { + // see tss.forEachChild for documentation + // look for the innermost child that matches the position + return node.forEachChild(cn => findInnerNode(cn, position)) || node; + } + + return null; +} + /** * Shifts the match position of the identifier to the left * until any character other than an empty string or a '.' is reached. #9347 @@ -347,8 +409,8 @@ const shiftMatchPosition = (matchPosition: number, content: string): number => { do { matchPosition--; } while (matchPosition > 0 && !content[matchPosition - 1].trim() - || content[matchPosition - 1] === SynaxTokens.MemberAccess - || content[matchPosition - 1] === SynaxTokens.Question); + || content[matchPosition - 1] === SyntaxTokens.MemberAccess + || content[matchPosition - 1] === SyntaxTokens.Question); return matchPosition; }; @@ -358,8 +420,7 @@ const shiftMatchPosition = (matchPosition: number, content: string): number => { * @param langServ The TypeScript LanguageService. * @param definition The method definition. */ -const getMethodTypeDefinition = (langServ: tss.LanguageService, definition: tss.DefinitionInfo): - Pick | null => { +const getMethodTypeDefinition = (langServ: tss.LanguageService, definition: tss.DefinitionInfo): MemberInfo | null => { // TODO: use typechecker for all the things? const sourceFile = langServ.getProgram().getSourceFile(definition.fileName); @@ -390,7 +451,7 @@ const getMethodTypeDefinition = (langServ: tss.LanguageService, definition: tss. // there should never be a case where a type is declared in more than one file /** * For union return types like T | null | undefined - * and interesection return types like T & null & undefined + * and intersection return types like T & null & undefined * the TypeChecker ignores null and undefined and returns only T which is not * marked as a union or intersection type. *