-
Notifications
You must be signed in to change notification settings - Fork 214
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
Refactor symbol handling by introducing new name resolution phase #2640
base: main
Are you sure you want to change the base?
Conversation
Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status): Website: https://tspwebsitepr.z22.web.core.windows.net/prs/2640/ |
packages/compiler/src/core/links.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for testing?
node: TypeReferenceNode | MemberExpressionNode | IdentifierNode, | ||
mapper: TypeMapper | undefined, | ||
instantiateTemplate = true | ||
): { sym: Sym | null; type: Type } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i never fully get when we should use null
, always try to stick to undefined
so you don't have to check for multiple things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to use null for an explicit no thing value since undefined crops up randomly, but do note that much of this code will be completely rewritten once the resolver is operational.
} else if (symbolLinks.declaredType) { | ||
baseType = symbolLinks.declaredType; | ||
} else if (sym.flags & SymbolFlags.Member) { | ||
baseType = checkMemberSym(sym, mapper); | ||
} else { | ||
baseType = checkDeclaredType(sym, decl, mapper); | ||
baseType = getTypeForNode(decl, mapper); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the checker changes were from when I thought I could fix everything without refactoring how symbols work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah make sense
@@ -1177,6 +1250,9 @@ export function createChecker(program: Program): Checker { | |||
parentMapper: TypeMapper | undefined, | |||
instantiateTempalates = true | |||
): Type { | |||
if (templateNode.id.sv === "bar") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover debugger
fix #4764
This is nowhere near ready, but putting up in draft PR to share progress.
The goals of this change:
Details about the approach can be found in the comment at the top of
name-resolver.ts
.