From 53b7380dbd3b3dbfe7302a03f3bb07fde3fb376d Mon Sep 17 00:00:00 2001 From: jackofdiamond5 <16020256+jackofdiamond5@users.noreply.github.com> Date: Thu, 18 Nov 2021 15:32:48 +0200 Subject: [PATCH 1/4] refactor(tsUtils): use typechecker to determine property types --- .../migrations/common/tsUtils.ts | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/projects/igniteui-angular/migrations/common/tsUtils.ts b/projects/igniteui-angular/migrations/common/tsUtils.ts index c0a58a9f23f..c803a9e9be5 100644 --- a/projects/igniteui-angular/migrations/common/tsUtils.ts +++ b/projects/igniteui-angular/migrations/common/tsUtils.ts @@ -264,6 +264,30 @@ 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 typeChecker = langServ.getProgram().getTypeChecker(); + // const node = (tss as any).getTouchingPropertyName(sourceFile, position); -> tss internal that looks up a node + const node = findNodeAtPosition(sourceFile, position); + const symbolFromNode = typeChecker.getSymbolAtLocation(node) + const nodeType = typeChecker.getTypeOfSymbolAtLocation(symbolFromNode, node) + if (nodeType) { + 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 fileName = nodeType.getSymbol()?.getDeclarations()[0].getSourceFile().fileName; + if (name && fileName) { + return { name, fileName }; + } + } + } + } 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 +335,6 @@ export const getTypeDefinitionAtPosition = return null; }; - /** * Determines if a member belongs to a type in the `igniteui-angular` toolkit. * @@ -339,6 +362,24 @@ export const isMemberIgniteUI = && change.definedIn.indexOf(typeDef.name) !== -1; }; +/** + * Looks up a node which end property matches the specified position. + */ +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()) { + 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,7 +388,7 @@ 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.MemberAccess || content[matchPosition - 1] === SynaxTokens.Question); return matchPosition; }; @@ -390,7 +431,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. * From 22edd3a061d4d3af1ad553bec08c98e2a788face Mon Sep 17 00:00:00 2001 From: jackofdiamond5 <16020256+jackofdiamond5@users.noreply.github.com> Date: Fri, 19 Nov 2021 11:08:54 +0200 Subject: [PATCH 2/4] fix(tsUtils): clean match string before comparing to node.text --- .../migrations/common/UpdateChanges.ts | 15 +++++++-------- .../igniteui-angular/migrations/common/tsUtils.ts | 12 +++++++----- 2 files changed, 14 insertions(+), 13 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 c803a9e9be5..acb4a3c96b8 100644 --- a/projects/igniteui-angular/migrations/common/tsUtils.ts +++ b/projects/igniteui-angular/migrations/common/tsUtils.ts @@ -12,7 +12,7 @@ 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 = '?' @@ -45,7 +45,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) => { @@ -348,7 +350,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; } @@ -388,8 +390,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; }; From fdb510ccb86ae1efc3ef3cd32e183b29fd98d434 Mon Sep 17 00:00:00 2001 From: jackofdiamond5 <16020256+jackofdiamond5@users.noreply.github.com> Date: Fri, 19 Nov 2021 13:59:48 +0200 Subject: [PATCH 3/4] refactor(tsUtils): use getTypeAtLocation, separate logic, add comments --- .../migrations/common/tsUtils.ts | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/projects/igniteui-angular/migrations/common/tsUtils.ts b/projects/igniteui-angular/migrations/common/tsUtils.ts index acb4a3c96b8..d30c7ad252a 100644 --- a/projects/igniteui-angular/migrations/common/tsUtils.ts +++ b/projects/igniteui-angular/migrations/common/tsUtils.ts @@ -18,6 +18,11 @@ enum SyntaxTokens { 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); @@ -253,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; @@ -270,23 +275,12 @@ export const getTypeDefinitionAtPosition = // 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 typeChecker = langServ.getProgram().getTypeChecker(); // const node = (tss as any).getTouchingPropertyName(sourceFile, position); -> tss internal that looks up a node const node = findNodeAtPosition(sourceFile, position); - const symbolFromNode = typeChecker.getSymbolAtLocation(node) - const nodeType = typeChecker.getTypeOfSymbolAtLocation(symbolFromNode, node) - if (nodeType) { - 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 fileName = nodeType.getSymbol()?.getDeclarations()[0].getSourceFile().fileName; - if (name && fileName) { - return { name, fileName }; - } + if (node) { + const memberInfo = resolveMemberInfo(langServ, node); + if (memberInfo) { + return memberInfo; } } } @@ -364,6 +358,28 @@ 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) { + 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. */ @@ -376,6 +392,8 @@ const findNodeAtPosition = (sourceFile: tss.SourceFile, position: number): tss.N } 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; } @@ -401,8 +419,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); From 383fba00954ebb08c368fe7bded0b9309617fce8 Mon Sep 17 00:00:00 2001 From: jackofdiamond5 <16020256+jackofdiamond5@users.noreply.github.com> Date: Fri, 19 Nov 2021 15:06:43 +0200 Subject: [PATCH 4/4] refactor(tsUtils): check declarations length, add comment --- projects/igniteui-angular/migrations/common/tsUtils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/projects/igniteui-angular/migrations/common/tsUtils.ts b/projects/igniteui-angular/migrations/common/tsUtils.ts index d30c7ad252a..ebb9d5626e9 100644 --- a/projects/igniteui-angular/migrations/common/tsUtils.ts +++ b/projects/igniteui-angular/migrations/common/tsUtils.ts @@ -369,7 +369,7 @@ const resolveMemberInfo = (langServ: tss.LanguageService, node: tss.Node): Membe // 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) { + if (declarations && declarations.length > 0) { const fileName = declarations[0].getSourceFile().fileName; if (name && fileName) { return { name, fileName }; @@ -382,6 +382,7 @@ const resolveMemberInfo = (langServ: tss.LanguageService, node: tss.Node): Membe /** * 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) {