-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Check syntax kind in isDeclarationNameOrImportPropertyName #21379
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
Conversation
Otherwise, keywords are accepted. Assert was ``` fail (e:\ts_gh\src\compiler\core.ts:2867) assert (e:\ts_gh\src\compiler\core.ts:2837) isExportSpecifierAlias (e:\ts_gh\src\services\findAllReferences.ts:931) getLocalSymbolForExportSpecifier (e:\ts_gh\src\services\findAllReferences.ts:926) skipPastExportOrImportSpecifierOrUnion (e:\ts_gh\src\services\findAllReferences.ts:411) getReferencedSymbolsForSymbol (e:\ts_gh\src\services\findAllReferences.ts:359) getReferencedSymbolsForNode (e:\ts_gh\src\services\findAllReferences.ts:271) getReferenceEntriesForNode (e:\ts_gh\src\services\findAllReferences.ts:91) getSemanticDocumentHighlights (e:\ts_gh\src\services\documentHighlights.ts:25) getDocumentHighlights (e:\ts_gh\src\services\documentHighlights.ts:13) getDocumentHighlights (e:\ts_gh\src\services\services.ts:1584) getOccurrencesAtPositionCore (e:\ts_gh\src\services\services.ts:1588) getOccurrencesAtPosition (e:\ts_gh\src\services\services.ts:1567) Session.getOccurrences (e:\ts_gh\src\server\session.ts:718) Session.handlers.ts.createMapFromTemplate._a.(anonymous function) (e:\ts_gh\src\server\session.ts:1999) (anonymous function) (e:\ts_gh\src\server\session.ts:2079) Session.executeWithRequestId (e:\ts_gh\src\server\session.ts:2069) Session.executeCommand (e:\ts_gh\src\server\session.ts:2079) Session.onMessage (e:\ts_gh\src\server\session.ts:2101) (anonymous function) (e:\ts_gh\src\server\server.ts:593) ```
| @@ -0,0 +1,11 @@ | |||
| /// <reference path='fourslash.ts'/> | |||
|
|
|||
| // @Filename: file1.ts | |||
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.
File name isn't needed unless you're doing a test with multiple files, or an unusual file extension.
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.
Neat.
| @@ -0,0 +1,11 @@ | |||
| /// <reference path='fourslash.ts'/> | |||
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.
This doesn't seem document-highlights specific, so would probably be better as findAllRefsReExports3.ts. Then you could use verify.noReferences instead of adding a noDocumentHighlights method.
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 wrote a FAR test to confirm that it covers this change. I don't mind submitting it as well, but I think it's worthwhile to keep this test because (1) the scenario does not appear to have other test coverage, (2) it seems like there ought to be at least one test asserting that certain points in the code don't have reference highlighting.
Otherwise, keywords are accepted. Assert was