Incorrect rename of imported declaration

[amcasey]

TypeScript Version: aa3734c14834afb454c49bc956489980d990a765

Search Terms:

Code

file1.ts

export const abc = 1;

file2.ts

import {abc} from "./file1";
abc; // Rename this one to def

Expected behavior:

file1.ts

export const def = 1;

file2.ts

import {def} from "./file1";
def; // Rename this one to def

Actual behavior:

file1.ts

export const def = 1;

file2.ts

import {abc as def} from "./file1";
def; // Rename this one to def

It looks like the prefix/suffix logic assumes the export is a separate statement and not the declaration itself.

[amcasey]

Version: 1.30.0-insider (user setup) Commit: 456e8e64beb2dceac66743959f6802b66e18e300 Date: 2018-11-26T11:05:14.406Z Electron: 2.0.12 Chrome: 61.0.3163.100 Node.js: 8.9.3 V8: 6.1.534.41 OS: Windows_NT x64 10.0.17763

[amcasey]

FYI @mjbvz

[mjbvz]

@amcasey I can't repo this in VS Code 1.30 using TS 3.3.0-dev.20181127. Do you think this is a VS Code issue?

[amcasey]

@mjbvz, no I don't think it's a VS Code issue, I just thought you'd be interested because the new rename functionality had come up in the editor sync a few times.

[sandersn]

I can't repro this in VS code either, with a typescript build from yesterday.

[sandersn]

Here's a more complex example that shows the error on Code 1.34 and typescript@next:

// @Filename: welove.ts
const g = 4567;
export = g;
// @Filename: app.tsx
import good = require('./welove')
console.log(good/*1*/)

Find-all-refs at /*1*/ finds all four references, which is arguable, since g and good have different names. Rename at /*1*/ renames all four references, which seems bad since g and good are not the same name, and welove.ts is likely a d.ts where renaming is going to cause a break.

// @Filename: welove.ts
const g = 4567;
export const good = g;
// @Filename: app.tsx
import { good } from './welove'
console.log(good/*1*/)

Here, find-all-refs at (1) finds the last 3 lines, which seems fine. But it does even with the import is import { good as go }, which seems wrong in the same way as the export= example. Rename is just broken. Renaming at (1) renames the last line, renames the import to import { good as go } and then incorrectly renames export const go = g.

The behaviour is the same in emacs, which is the only other editor I've tested. Here's my tsconfig in case that's required:

{
    "compilerOptions": {
        "strict": true,
        "noImplicitThis": false,
        "noImplicitAny": true,
        // "noUnusedLocals": true,
        //"noImplicitThis": true,
        //"noImplicitReturns": true,
        // "strictNullChecks": false,
        "target": "esnext",
        "lib": ["es5", "es2015", "es2016", "es2017", "esnext", "dom"],
        "allowJs": true,
        "checkJs": true,
        "outDir": "out",
        "jsx": "preserve",
        "esModuleInterop": true,
        "module": "commonjs"
    },
    "include": [
        "ex.d.ts",
        "other.d.ts",
        "mod1.js",
        "test.js",
        "welove.ts",
        "first.ts",
        "forever.js"
    ]
}

[sandersn]

After some more discussion with @amcasey, I think the current behaviour with previous two examples is OK, since we don't have a reliable way to know whether a symbol comes from a file that should not be renamed or not.

However, this example has a couple of problems with rename (and perhaps with find-all-refs):

// @Filename: welove.ts
const g = 4567;
export const good = g;
// @Filename: app.tsx
import { good/*0*/ as gold/*1*/ } from './welove'
console.log(gold/*2*/)

• Rename at (1) or (2) should rename only the introduced alias gold. Currently it renames all occurences of good as well (and find-all-refs includes good).
• Rename at (0) should rename everything except the introduced alias gold. That is what it does currently. Find-all-refs includes references to gold, which is not great, but OK.

[sandersn]

Note that this only affects VS (and emacs), not VS Code. I think the new providePrefixAndSuffixTextForRename avoids this bug.

[sandersn]

Further investigation shows that the server has

1. A configured project with all but one of the files explicitly listed.
2. An inferred project with that includes the missed file.

The inferred project provides the two incorrect references. I haven't investigated that part yet. I think this is therefore pretty low priority, although I'll check with @amcasey to find out how common this setup is.

In other words, this doesn't repro if you remove the "include" list from the tsconfig given above, or if you add "app.tsx" to it.