Skip to content

Replace endsWith() check with isDefaultLibFile() in getRenameInfo() #1990

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.

Sign up for GitHub

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

Merged
jramsay merged 4 commits into from
Feb 17, 2015
Merged

Replace endsWith() check with isDefaultLibFile() in getRenameInfo() #1990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf7bb51
Replace endsWith() with isDefaultLib()
jramsay Feb 9, 2015
a51ce92
switch to using host.getDefaultLibFileName(options)
jramsay Feb 13, 2015
93ca5ae
Merge branch 'master' into isDefaultLibFile
jramsay Feb 13, 2015
975f10c
Adding getCanonicalFileName to ensure case-sensitive systems do not h…
jramsay Feb 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5518,11 +5518,13 @@ module ts {
var declarations = symbol.getDeclarations();
if (declarations && declarations.length > 0) {
// Disallow rename for elements that are defined in the standard TypeScript library.
var defaultLibFile = getDefaultLibFileName(host.getCompilationSettings());
for (var i = 0; i < declarations.length; i++) {
var sourceFile = declarations[i].getSourceFile();
if (sourceFile && endsWith(sourceFile.fileName, defaultLibFile)) {
return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library.key));
var defaultLibFileName = host.getDefaultLibFileName(host.getCompilationSettings());
if (defaultLibFileName) {
for (var i = 0; i < declarations.length; i++) {
var sourceFile = declarations[i].getSourceFile();
if (sourceFile && getCanonicalFileName(ts.normalizePath(sourceFile.fileName)) === getCanonicalFileName(ts.normalizePath(defaultLibFileName))) {
return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library.key));
}
}
}

Expand All @@ -5544,10 +5546,6 @@ module ts {

return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_this_element.key));

function endsWith(string: string, value: string): boolean {
return string.lastIndexOf(value) + value.length === string.length;
}

function getRenameInfoError(localizedErrorMessage: string): RenameInfo {
return {
canRename: false,
Expand Down
5 changes: 1 addition & 4 deletions src/services/shims.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,7 @@ module ts {
}

public getDefaultLibFileName(options: CompilerOptions): string {
// Shim the API changes for 1.5 release. This should be removed once
// TypeScript 1.5 has shipped.
return "";
//return this.shimHost.getDefaultLibFileName(JSON.stringify(options));
return this.shimHost.getDefaultLibFileName(JSON.stringify(options));

@mhegazy mhegazy Feb 17, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you still need the comment, and you still need to shim it.

the problem is that the released 1.4 VS plugin do not have a function called "getDefaultLibFileName", it is called "getDefaultLibFilename" with lowercase "n". this if you use the latest from master on an existing 1.4 installation it will fail. this affects users who want to try the latest bits from master.

so, to work around this, you need to shim it.. something like:

if (typeof this.shimHost.getDefaultLibFileName === "function") { 
   return this.shimHost.getDefaultLibFileName(JSON.stringify(options));
}
return "";

and i would give it a quick try in the shipped 1.4 VS as well.

@jramsay jramsay Feb 17, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just discussed with Mohamed. I was going to add the typeof check for getDefaultLibFileName to ensure it is backward compatible with the previous lower-cased version (getDefaultLibFilename). Case does not appear to matter for the shimHost though as both work (they return typeof "unknown"). Using a completely different method that isn't on shimHost would return typeof "undefined".

}
}

Expand Down