microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.76k stars 12.46k forks source link

VS2015 code editor for TypeScript shows all the references incorrectly #8118

Closed ghost closed 8 years ago

ghost commented 8 years ago

TypeScript Version: 1.8.9 (Visual Studio 2015)

I have a problem of finding all references (uses) for imported declaration of an interface or class into a module.

For example i have a dialog module: dialog.ts (in any other module the same behaviour) https://github.com/BBGONE/JRIApp/blob/master/FRAMEWORK/CLIENT/addins/ui/jriapp_ui/dialog.ts

it contains Code

import { ITemplate, ITemplateEvents, IApplication, IVoidPromise, IEditable, IBaseObject,
    TEventHandler, ISelectableProvider, IDeferred } from "jriapp_core/shared";
import * as langMOD from "jriapp_core/lang";
import { BaseObject } from "jriapp_core/object";
import { Utils as utils, DomUtils as dom, ERROR } from "jriapp_utils/utils";
import { bootstrap } from "jriapp_core/bootstrap";
import { ViewModel } from "jriapp_parts/mvvm";

Actual behavior:

When i click in VS2015 code editor with right button on imported class or interface or variable and select "Find all references" in the menu it finds all the references for imported variable in the whole project !

P.S. - Stange but for import * as langMOD from "jriapp_core/lang";

if i click on langMOD to find refences for it then it produces correct result- for the references to langMOD in the current module. But for all other imported parts which use {} to import them. The result is references in the whole project.

Expected behavior:

Show only the references in the current module

mhegazy commented 8 years ago

with find-all-references we try to find them all when possible. For import * as langMOD from "jriapp_core/lang"; langMOD really means one thing, and it is local to this module, and cannot be accessed outside. but import { ViewModel } from "jriapp_parts/mvvm"; ViewModel can both mean the local name ViewModel and the name of the export ViewModule in "jriapp_parts/mvvm", so we show both. if that was renamed, i.e. import { ViewModel as localViewModel } from "jriapp_parts/mvvm"; then localViewModel would be local only.

the other aspect here is rename. Rename is built on find-all-references, we would want to make sure that starting with a working state, when your rename ViewModle you get to another working state, and not some of your imports are invalid.

ghost commented 8 years ago

Rename is a different thing, because if we rename then we rename it at the site of the declaration - not at the import. Imports create new variables inside the module that it imports, so we need to find the references inside the imported module. In the current state it is unusable. I need to refactor a large part of my framework, and i need to find unusable imports inside the module. How to do it?

ghost commented 8 years ago

@mhegazy if we look more closely at renaming at the site of import instead of renaming at the site of declaration then we can see how things can be broken easily. For example: we could have a definition file app.d.ts, and imported some types or variables into module1.ts, then at the site of import in module1.ts we rename that imported type and this will rename it to in the definition file too. But after regenerating the definition file our application is broken! Another example, if we rename at the site of the import and that import is used in other modules too and the other modules use local variables with exactly the same name as we will rename in the first module, then we get a name collision in other modules - the imports will have the names of local variables or exports. If we renaming at the site of declaration then we are aware of these possibilities, but if we rename at the site of import we are getting into it unawarely.

mhegazy commented 8 years ago

I think there is a part here that we need to fix, namely renaming something in a declaration file. if the module declaration is coming from a declaration file, renaming the import should not rename the export, as this is meaningless, we should rename the local import.

related to https://github.com/Microsoft/TypeScript/issues/7458 and https://github.com/Microsoft/TypeScript/issues/8213.

mhegazy commented 8 years ago

Filed a new issue for it in https://github.com/Microsoft/TypeScript/issues/8227