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.

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 typechecker to determine property types #10558

Merged
merged 8 commits into from
Nov 19, 2021
15 changes: 7 additions & 8 deletions projects/igniteui-angular/migrations/common/UpdateChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -542,17 +541,17 @@ 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) {
content.angularCompilerOptions = {};
}
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
Expand Down
81 changes: 71 additions & 10 deletions projects/igniteui-angular/migrations/common/tsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<tss.DefinitionInfo, 'name' | 'fileName'> {
Copy link
Member

Choose a reason for hiding this comment

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

while this is related to members, this isn't the member info - it's still the definition info for whatever we looked up at a position and it's usually the container type of the member instead :) Don't much care for the name ATM, just saying it's not quite right.

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);
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

@jackofdiamond5 for future improvement: this can probably be resolved with some kind check and likely the method nodes will have name or something

return node.text === cleanName;
};

const findIdentifiers = (node: ts.Node) => {
Expand Down Expand Up @@ -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<tss.DefinitionInfo, 'name' | 'fileName'> | null => {
(langServ: tss.LanguageService, entryPath: string, position: number): MemberInfo | null => {
const definition = langServ.getDefinitionAndBoundSpan(entryPath, position)?.definitions[0];
if (!definition) {
return null;
Expand All @@ -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
Expand Down Expand Up @@ -311,7 +331,6 @@ export const getTypeDefinitionAtPosition =
return null;
};


/**
* Determines if a member belongs to a type in the `igniteui-angular` toolkit.
*
Expand All @@ -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;
}
Expand All @@ -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);
damyanpetev marked this conversation as resolved.
Show resolved Hide resolved
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;
damyanpetev marked this conversation as resolved.
Show resolved Hide resolved
}

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
Expand All @@ -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;
};

Expand All @@ -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<tss.DefinitionInfo, 'name' | 'fileName'> | 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);

Expand Down Expand Up @@ -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.
*
Expand Down