Skip to content

Fix #7611: Add support for String Literal Types in find all refs and occurances - #8366

Merged
Mohamed Hegazy (mhegazy) merged 3 commits into
masterfrom
Fix7611-2
May 2, 2016
Merged

Mohamed Hegazy (mhegazy) merged 3 commits into
masterfrom
Fix7611-2

Conversation

@mhegazy

Copy link
Copy Markdown
Contributor

Fixes #7611

Comment thread src/services/services.ts Outdated

const references: ReferenceEntry[] = [];

forEach(sourceFiles, sourceFile => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just use a for-of

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Apr 29, 2016 •

Copy link
Copy Markdown
Member

Can you add a test where an ordinary string literal (of type string) don't get affected by a rename?

@mhegazy

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser) any more comments?

Comment thread src/services/services.ts
const declarations = symbol.getDeclarations();
if (declarations && declarations.length > 0) {
// Disallow rename for elements that are defined in the standard TypeScript library.
if (forEach(declarations, isDefinedInLibraryFile)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should just do a pass at the end for all the references you get, instead of doing this logic here and below

@DanielRosenwasser

Copy link
Copy Markdown
Member

I meant a test in which contextually typed strings are considered references, but others of the same content are not.

Other than that and my last feedback, looks great 👍

@mhegazy
Mohamed Hegazy (mhegazy) merged commit afaffb1 into master May 2, 2016
@mhegazy
Mohamed Hegazy (mhegazy) deleted the Fix7611-2 branch May 2, 2016 18:29
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants