-
Notifications
You must be signed in to change notification settings - Fork 12.8k
moveToFile: fix check for global name resolving #60173
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
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a20bcb8
only skip imports for symbols that may be global types if they are no…
iisaduan 7112af8
add paste test, rename tests, fix `isGlobalType`
iisaduan 9bbf9ba
fix `isGlobalType` again
iisaduan 5fd1bef
add test
iisaduan 38b06af
add test
iisaduan 4e9562c
Merge branch 'main' of https://github.com/iisaduan/TypeScript into mo…
iisaduan a8f8da0
update tests for improved quote behaviors
iisaduan 58118b2
make sure resolved symbol is same symbol
iisaduan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
368 changes: 368 additions & 0 deletions
368
tests/baselines/reference/tsserver/fourslashServer/pasteEdits_globalAndLocal1.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,368 @@ | ||
| Info seq [hh:mm:ss:mss] currentDirectory:: /home/src/Vscode/Projects/bin useCaseSensitiveFileNames:: false | ||
| Info seq [hh:mm:ss:mss] libs Location:: /home/src/tslibs/TS/Lib | ||
| Info seq [hh:mm:ss:mss] globalTypingsCacheLocation:: /home/src/Library/Caches/typescript | ||
| Info seq [hh:mm:ss:mss] Provided types map file "/home/src/tslibs/TS/Lib/typesMap.json" doesn't exist | ||
| //// [/home/src/tslibs/TS/Lib/lib.d.ts] | ||
| lib.d.ts-Text | ||
|
|
||
| //// [/home/src/tslibs/TS/Lib/lib.decorators.d.ts] | ||
| lib.decorators.d.ts-Text | ||
|
|
||
| //// [/home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts] | ||
| lib.decorators.legacy.d.ts-Text | ||
|
|
||
| //// [/home/src/workspaces/project/globals.d.ts] | ||
| export {}; // Make this a module | ||
| declare global { | ||
| interface Disposable { | ||
| [Symbol.dispose](): void; | ||
| } | ||
| } | ||
|
|
||
| //// [/home/src/workspaces/project/target.ts] | ||
|
|
||
|
|
||
| //// [/home/src/workspaces/project/test.ts] | ||
| export interface Disposable { | ||
| (): string; | ||
| } | ||
| export interface EditingService extends Disposable { } | ||
|
|
||
| //// [/home/src/workspaces/project/tsconfig.json] | ||
| { "files": ["target.ts", "globals.d.ts", "test.ts"] } | ||
|
|
||
|
|
||
| Info seq [hh:mm:ss:mss] request: | ||
| { | ||
| "seq": 0, | ||
| "type": "request", | ||
| "arguments": { | ||
| "file": "/home/src/workspaces/project/target.ts" | ||
| }, | ||
| "command": "open" | ||
| } | ||
| Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: /home/src/workspaces/project/target.ts ProjectRootPath: undefined:: Result: /home/src/workspaces/project/tsconfig.json | ||
| Info seq [hh:mm:ss:mss] Creating ConfiguredProject: /home/src/workspaces/project/tsconfig.json, currentDirectory: /home/src/workspaces/project | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/workspaces/project/tsconfig.json 2000 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Config file | ||
| Info seq [hh:mm:ss:mss] Config: /home/src/workspaces/project/tsconfig.json : { | ||
| "rootNames": [ | ||
| "/home/src/workspaces/project/target.ts", | ||
| "/home/src/workspaces/project/globals.d.ts", | ||
| "/home/src/workspaces/project/test.ts" | ||
| ], | ||
| "options": { | ||
| "configFilePath": "/home/src/workspaces/project/tsconfig.json" | ||
| } | ||
| } | ||
| Info seq [hh:mm:ss:mss] event: | ||
| { | ||
| "seq": 0, | ||
| "type": "event", | ||
| "event": "projectLoadingStart", | ||
| "body": { | ||
| "projectName": "/home/src/workspaces/project/tsconfig.json", | ||
| "reason": "Creating possible configured project for /home/src/workspaces/project/target.ts to open" | ||
| } | ||
| } | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/workspaces/project/globals.d.ts 500 undefined WatchType: Closed Script info | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/workspaces/project/test.ts 500 undefined WatchType: Closed Script info | ||
| Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /home/src/workspaces/project/tsconfig.json | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/tslibs/TS/Lib/lib.d.ts 500 undefined WatchType: Closed Script info | ||
| Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/project/node_modules 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Failed Lookup Locations | ||
| Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/project/node_modules 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Failed Lookup Locations | ||
| Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/node_modules 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Failed Lookup Locations | ||
| Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/node_modules 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Failed Lookup Locations | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/tslibs/TS/Lib/lib.decorators.d.ts 500 undefined WatchType: Closed Script info | ||
| Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts 500 undefined WatchType: Closed Script info | ||
| Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/project/node_modules/@types 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Type roots | ||
| Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/project/node_modules/@types 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Type roots | ||
| Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/node_modules/@types 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Type roots | ||
| Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/workspaces/node_modules/@types 1 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Type roots | ||
| Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /home/src/workspaces/project/tsconfig.json projectStateVersion: 1 projectProgramVersion: 0 structureChanged: true structureIsReused:: Not Elapsed:: *ms | ||
| Info seq [hh:mm:ss:mss] Project '/home/src/workspaces/project/tsconfig.json' (Configured) | ||
| Info seq [hh:mm:ss:mss] Files (6) | ||
| /home/src/tslibs/TS/Lib/lib.d.ts Text-1 lib.d.ts-Text | ||
| /home/src/tslibs/TS/Lib/lib.decorators.d.ts Text-1 lib.decorators.d.ts-Text | ||
| /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts Text-1 lib.decorators.legacy.d.ts-Text | ||
| /home/src/workspaces/project/target.ts SVC-1-0 "" | ||
| /home/src/workspaces/project/globals.d.ts Text-1 "export {}; // Make this a module\ndeclare global {\n interface Disposable {\n [Symbol.dispose](): void;\n }\n}" | ||
| /home/src/workspaces/project/test.ts Text-1 "export interface Disposable {\n (): string;\n}\nexport interface EditingService extends Disposable { }" | ||
|
|
||
|
|
||
| ../../tslibs/TS/Lib/lib.d.ts | ||
| Default library for target 'es5' | ||
| ../../tslibs/TS/Lib/lib.decorators.d.ts | ||
| Library referenced via 'decorators' from file '../../tslibs/TS/Lib/lib.d.ts' | ||
| ../../tslibs/TS/Lib/lib.decorators.legacy.d.ts | ||
| Library referenced via 'decorators.legacy' from file '../../tslibs/TS/Lib/lib.d.ts' | ||
| target.ts | ||
| Part of 'files' list in tsconfig.json | ||
| globals.d.ts | ||
| Part of 'files' list in tsconfig.json | ||
| test.ts | ||
| Part of 'files' list in tsconfig.json | ||
|
|
||
| Info seq [hh:mm:ss:mss] ----------------------------------------------- | ||
| Info seq [hh:mm:ss:mss] event: | ||
| { | ||
| "seq": 0, | ||
| "type": "event", | ||
| "event": "projectLoadingFinish", | ||
| "body": { | ||
| "projectName": "/home/src/workspaces/project/tsconfig.json" | ||
| } | ||
| } | ||
| Info seq [hh:mm:ss:mss] event: | ||
| { | ||
| "seq": 0, | ||
| "type": "event", | ||
| "event": "configFileDiag", | ||
| "body": { | ||
| "triggerFile": "/home/src/workspaces/project/target.ts", | ||
| "configFile": "/home/src/workspaces/project/tsconfig.json", | ||
| "diagnostics": [] | ||
| } | ||
| } | ||
| Info seq [hh:mm:ss:mss] Project '/home/src/workspaces/project/tsconfig.json' (Configured) | ||
| Info seq [hh:mm:ss:mss] Files (6) | ||
|
|
||
| Info seq [hh:mm:ss:mss] ----------------------------------------------- | ||
| Info seq [hh:mm:ss:mss] Open files: | ||
| Info seq [hh:mm:ss:mss] FileName: /home/src/workspaces/project/target.ts ProjectRootPath: undefined | ||
| Info seq [hh:mm:ss:mss] Projects: /home/src/workspaces/project/tsconfig.json | ||
| Info seq [hh:mm:ss:mss] response: | ||
| { | ||
| "seq": 0, | ||
| "type": "response", | ||
| "command": "open", | ||
| "request_seq": 0, | ||
| "success": true, | ||
| "performanceData": { | ||
| "updateGraphDurationMs": * | ||
| } | ||
| } | ||
| After Request | ||
| watchedFiles:: | ||
| /home/src/tslibs/TS/Lib/lib.d.ts: *new* | ||
| {"pollingInterval":500} | ||
| /home/src/tslibs/TS/Lib/lib.decorators.d.ts: *new* | ||
| {"pollingInterval":500} | ||
| /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts: *new* | ||
| {"pollingInterval":500} | ||
| /home/src/workspaces/project/globals.d.ts: *new* | ||
| {"pollingInterval":500} | ||
| /home/src/workspaces/project/test.ts: *new* | ||
| {"pollingInterval":500} | ||
| /home/src/workspaces/project/tsconfig.json: *new* | ||
| {"pollingInterval":2000} | ||
|
|
||
| watchedDirectoriesRecursive:: | ||
| /home/src/workspaces/node_modules: *new* | ||
| {} | ||
| /home/src/workspaces/node_modules/@types: *new* | ||
| {} | ||
| /home/src/workspaces/project/node_modules: *new* | ||
| {} | ||
| /home/src/workspaces/project/node_modules/@types: *new* | ||
| {} | ||
|
|
||
| Projects:: | ||
| /home/src/workspaces/project/tsconfig.json (Configured) *new* | ||
| projectStateVersion: 1 | ||
| projectProgramVersion: 1 | ||
| autoImportProviderHost: false | ||
|
|
||
| ScriptInfos:: | ||
| /home/src/tslibs/TS/Lib/lib.d.ts *new* | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/tslibs/TS/Lib/lib.decorators.d.ts *new* | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts *new* | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/workspaces/project/globals.d.ts *new* | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/workspaces/project/target.ts (Open) *new* | ||
| version: SVC-1-0 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json *default* | ||
| /home/src/workspaces/project/test.ts *new* | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
|
|
||
| Info seq [hh:mm:ss:mss] request: | ||
| { | ||
| "seq": 1, | ||
| "type": "request", | ||
| "arguments": { | ||
| "formatOptions": { | ||
| "indentSize": 4, | ||
| "tabSize": 4, | ||
| "newLineCharacter": "\n", | ||
| "convertTabsToSpaces": true, | ||
| "indentStyle": 2, | ||
| "insertSpaceAfterConstructor": false, | ||
| "insertSpaceAfterCommaDelimiter": true, | ||
| "insertSpaceAfterSemicolonInForStatements": true, | ||
| "insertSpaceBeforeAndAfterBinaryOperators": true, | ||
| "insertSpaceAfterKeywordsInControlFlowStatements": true, | ||
| "insertSpaceAfterFunctionKeywordForAnonymousFunctions": false, | ||
| "insertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis": false, | ||
| "insertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets": false, | ||
| "insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces": true, | ||
| "insertSpaceAfterOpeningAndBeforeClosingTemplateStringBraces": false, | ||
| "insertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces": false, | ||
| "insertSpaceBeforeFunctionParenthesis": false, | ||
| "placeOpenBraceOnNewLineForFunctions": false, | ||
| "placeOpenBraceOnNewLineForControlBlocks": false, | ||
| "semicolons": "ignore", | ||
| "trimTrailingWhitespace": true, | ||
| "indentSwitchCase": true | ||
| } | ||
| }, | ||
| "command": "configure" | ||
| } | ||
| Info seq [hh:mm:ss:mss] Format host information updated | ||
| Info seq [hh:mm:ss:mss] response: | ||
| { | ||
| "seq": 0, | ||
| "type": "response", | ||
| "command": "configure", | ||
| "request_seq": 1, | ||
| "success": true | ||
| } | ||
| Info seq [hh:mm:ss:mss] request: | ||
| { | ||
| "seq": 2, | ||
| "type": "request", | ||
| "arguments": { | ||
| "file": "/home/src/workspaces/project/target.ts", | ||
| "pastedText": [ | ||
| "export interface EditingService extends Disposable { }" | ||
| ], | ||
| "pasteLocations": [ | ||
| { | ||
| "start": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| }, | ||
| "end": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| } | ||
| } | ||
| ], | ||
| "copiedFrom": { | ||
| "file": "/home/src/workspaces/project/test.ts", | ||
| "spans": [ | ||
| { | ||
| "start": { | ||
| "line": 4, | ||
| "offset": 1 | ||
| }, | ||
| "end": { | ||
| "line": 4, | ||
| "offset": 55 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "command": "getPasteEdits" | ||
| } | ||
| Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /home/src/workspaces/project/tsconfig.json | ||
| Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /home/src/workspaces/project/tsconfig.json projectStateVersion: 2 projectProgramVersion: 1 structureChanged: false structureIsReused:: Completely Elapsed:: *ms | ||
| Info seq [hh:mm:ss:mss] Project '/home/src/workspaces/project/tsconfig.json' (Configured) | ||
| Info seq [hh:mm:ss:mss] Files (6) | ||
| /home/src/tslibs/TS/Lib/lib.d.ts Text-1 lib.d.ts-Text | ||
| /home/src/tslibs/TS/Lib/lib.decorators.d.ts Text-1 lib.decorators.d.ts-Text | ||
| /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts Text-1 lib.decorators.legacy.d.ts-Text | ||
| /home/src/workspaces/project/target.ts SVC-1-1 "export interface EditingService extends Disposable { }" | ||
| /home/src/workspaces/project/globals.d.ts Text-1 "export {}; // Make this a module\ndeclare global {\n interface Disposable {\n [Symbol.dispose](): void;\n }\n}" | ||
| /home/src/workspaces/project/test.ts Text-1 "export interface Disposable {\n (): string;\n}\nexport interface EditingService extends Disposable { }" | ||
|
|
||
| Info seq [hh:mm:ss:mss] ----------------------------------------------- | ||
| Info seq [hh:mm:ss:mss] response: | ||
| { | ||
| "seq": 0, | ||
| "type": "response", | ||
| "command": "getPasteEdits", | ||
| "request_seq": 2, | ||
| "success": true, | ||
| "performanceData": { | ||
| "updateGraphDurationMs": * | ||
| }, | ||
| "body": { | ||
| "edits": [ | ||
| { | ||
| "fileName": "/home/src/workspaces/project/target.ts", | ||
| "textChanges": [ | ||
| { | ||
| "start": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| }, | ||
| "end": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| }, | ||
| "newText": "import { Disposable } from \"./test\";\n\n" | ||
| }, | ||
| { | ||
| "start": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| }, | ||
| "end": { | ||
| "line": 1, | ||
| "offset": 1 | ||
| }, | ||
| "newText": "export interface EditingService extends Disposable { }" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "fixId": "providePostPasteEdits" | ||
| } | ||
| } | ||
| After Request | ||
| Projects:: | ||
| /home/src/workspaces/project/tsconfig.json (Configured) *changed* | ||
| projectStateVersion: 3 *changed* | ||
| projectProgramVersion: 1 | ||
| dirty: true *changed* | ||
| autoImportProviderHost: false | ||
|
|
||
| ScriptInfos:: | ||
| /home/src/tslibs/TS/Lib/lib.d.ts | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/tslibs/TS/Lib/lib.decorators.d.ts | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/workspaces/project/globals.d.ts | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json | ||
| /home/src/workspaces/project/target.ts (Open) *changed* | ||
| version: SVC-1-2 *changed* | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json *default* | ||
| /home/src/workspaces/project/test.ts | ||
| version: Text-1 | ||
| containingProjects: 1 | ||
| /home/src/workspaces/project/tsconfig.json |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm having trouble wrapping my head around this function, I think because it takes a symbol but only uses it for its name. Do the function name and parameters need to be tweaked a bit to better reflect what this is trying to accomplish?
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 there another way to check if a symbol refers to a global, and can be checked reliably after symbol merging/through aliasing? I guess this function doesn't need a Symbol to be passed in as it currently is, but I couldn't think of a better way to do this check for globals.
The old code was checking if there was any global symbol with that name that was a type without seeing first if there was a local symbol with that name. The function name seems alright to me since it's checking if, at this node, the symbol refers to a global 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.
It’s checking if a given name refers to a global type; it’s not really checking anything about the symbol that’s being passed in. If you wanted to check whether the symbol itself is a type that is accessible as a global at a particular location, you might check for equality between the
symboland theresolveNameresult. But I’m not sure that’s relevant here.But the line added here seems to allow for a false negative of the simple question “does this name refer to a global type.” Just because a name resolves to a local value doesn’t mean it doesn’t also resolve to a global type at the same location:
If you pass in the node for the
Eventtype,isGlobalTypewill return false. That feels like the wrong answer just reading the function name, even if it has the desired behavior for Move To File (does it?).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.
The correct check to have here is if the symbol refers to a global type or not, and the previous check did not do that (and seems like the previous check wanted to check the symbol, given that they took a symbol instead of just the name).
It makes sense to me that if we try to move a symbol that references a local definition, but also happens to have an unrelated global definition, that we keep the reference in the new location as the same symbol that was in the original file. Otherwise, the code in the new file can be broken, like in the bug report above. I spoke to @navya9singh about this, and we concluded that we should generate the import to keep the original reference. Also, other codefixes keep local references across files, even if the symbol resolves to a global (added test to show this).
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.
@andrewbranch am I correct that you're mostly not a fan of the function doing the following?
Symbolinstead of astring(since all it needs is the name of the symbol)isGlobalType, since a type might not actually be declared in the global scope (so e.g.cannotResolveTypeAtwould be a "better" name)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.
Yes - I didn't recontextualize myself with the broader problem enough to tell whether the implementation/behavior is wrong; I can just tell from reading the function in isolation that something is a little fishy (specifically, your points above).
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.
@DanielRosenwasser @andrewbranch How do you feel about the new behavior? I can find a better way to do this check and/or rename the function if the new behavior makes sense.
I can change the first part of this function to
skipAlias(which in checker isresolveSymbol), but I think I'm confused because I've looked through the existing code, and I don't know how else to perform the second part of this check to find if there is a global without using the name resolver.