Completion for default export should be '.default' #16742
Conversation
ghost
force-pushed
the
completionsDefaultExport
branch
2 times, most recently
from
f58a503 to
88ce5b0
Compare
ghost
force-pushed
the
completionsDefaultExport
branch
from
88ce5b0 to
95613fb
Compare
ghost
mentioned this pull request
| @@ -22226,11 +22228,6 @@ namespace ts { | |||
| } | |||
|
|
|||
| switch (location.kind) { | |||
| case SyntaxKind.SourceFile: | |||
There was a problem hiding this comment.
I don't understand why this case goes away.
There was a problem hiding this comment.
This comes from the second Update comment -- exported symbols aren't really the same as the symbols in scope.
| ////a./**/; | ||
|
|
||
| goTo.marker(); | ||
| verify.completionListContains("default", "function f(): void"); |
There was a problem hiding this comment.
Interesting, is a.f legal here?
There was a problem hiding this comment.
No, the property name is default; function f(): void is just the display text.
|
|
||
| /** See comment on `declareModuleMember` in `binder.ts`. */ | ||
| export function getCombinedLocalAndExportSymbolFlags(symbol: Symbol): SymbolFlags { | ||
| return symbol.exportSymbol ? symbol.exportSymbol.flags | symbol.flags : symbol.flags; |
There was a problem hiding this comment.
why combine flags in the true case instead of just returning symbol.exportSymbol.flags?
There was a problem hiding this comment.
In findAllRefsForDefaultExport08 (and findAllRefsForDefaultExport02), we have an exported class and a non-exported namespace merged with it. So the local symbol will have flags that the exported symbol is lacking; it has NamespaceModule|ExportValue|ExportType while the exported symbol just has Class.
| @@ -949,6 +949,22 @@ namespace FourSlash { | |||
| this.verifySymbol(symbol, declarationRanges); | |||
| } | |||
|
|
|||
| public symbolsInScope(range: Range): ts.Symbol[] { | |||
| const node = this.goToAndGetNode(range); | |||
| return this.getChecker().getSymbolsInScope(node, ts.SymbolFlags.Value | ts.SymbolFlags.Type | ts.SymbolFlags.Namespace); | |||
There was a problem hiding this comment.
nit: doesn't SymbolFlags have an alias defined like SymbolFlags.All that's equivalent?
There was a problem hiding this comment.
Not currently; All wouldn't be a good name since that excludes some (Constructor, Signature, and everything from ExportValue on), but this combination is often useful; I'm tempted to just name it ValueTypeNamespace.
| @@ -3720,6 +3740,10 @@ namespace FourSlashInterface { | |||
| this.state.verifySymbolAtLocation(startRange, declarationRanges); | |||
| } | |||
|
|
|||
| public typeOfSymbolAtLocation(range: FourSlash.Range, symbol: ts.Symbol, expected: string) { | |||
There was a problem hiding this comment.
can't we get this from the quick info? I don't know whether we need a separate method for comparing the stringified type of a symbol.
There was a problem hiding this comment.
Well, quick info gets us the type of the symbol under the cursor -- this lets us call getTypeOfSymbolAtLocation on any symbol, at the current location.
src/services/services.ts
Outdated
| // We want to store any numbers/strings if they were a name that could be | ||
| // related to a declaration. So, if we have 'import x = require("something")' | ||
| // then we want 'something' to be in the name table. Similarly, if we have | ||
| // "a['propname']" then we want to store "propname" in the name table. |
Fixes #16706 and #12957
Just use
symbol.nameinstead ofgetDeclaredName, sincegetDeclaredNameintentionally gets the declared name of a default export instead of just "default".This change apparently also affects completions for unicode escapes, but I think the change is good. In this code:
We will now give you a completion for
B, which is correct --Bis in scope because\u0042is equivalent to it.Update: Also fixed #16741.
Update:
We must also change the behavior for
getSymbolsInScope()to return local symbols, not exported symbols. We need different behavior based on whether the symbols are in scope or from an export; today's example being that an export is accessed with '.default' while a local symbol is accessed by its name. Also fixed a bug wheregetSymbolAtLocationcouldn't handle a local symbol.