Skip to content

Salsa: "Implicit globals" leak into member completions but not global completions? #6654

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

Closed
egamma opened this issue Jan 27, 2016 · 7 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@egamma
Copy link
Member

egamma commented Jan 27, 2016

From @alexandrudima on January 27, 2016 13:34

Testing #2218

IMHO member completions should get less suggestions than the global completions, not the other way around:

function f() {
    // helloWorld leaks from here into the global space?
    if (helloWorld) {
        return 3;
    }
    return 5;
}

// helloWorld is proposed here
// Number(3).

// helloWorld is not proposed here
// hello

salsa-member-completion

Copied from original issue: microsoft/vscode#2447

@RyanCavanaugh
Copy link
Member

@billti how should we fix this?

  • Add all tokens in the current file to the global completion list. -- or --
  • Remove token-based items from the member list (when it is non-empty)

@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

the later is an problem when we guess things wrong:

var x = {};
x.a = 0;
x.b = "string";
x.|

The earlier seems like a more consistent approach. VSCode should show these completions as a different icon. the CompletionEntry.kind field should be the marker here, also CompletionEntry.sortKey should push these all to the bottom.

@egamma
Copy link
Member Author

egamma commented Jan 29, 2016

VSCode should show these completions as a different icon.

@mhegazy we have already changed the icon in our latest:
proposal2
.

@mhegazy mhegazy added the Bug A bug in TypeScript label Jan 29, 2016
@mhegazy mhegazy modified the milestones: Salsa 0.9, TypeScript 1.8.2 Feb 2, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Feb 2, 2016
@RyanCavanaugh RyanCavanaugh added the Fixed A PR has been merged for this issue label Feb 2, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Feb 5, 2016
@pflannery
Copy link

@RyanCavanaugh Did this fix scope variables leaking in to instance completions?

I just got the latest vscode 0.10.10 and now see this type of polluted intellisense all over my js apps:

image

I would only expect to see similar to this

image

Source:

class SomeClass {
  constructor(param1, param2, param3) {
    this.param1 = param1;
    this.param2 = param2;
    this.param3 = param3;
  }

  doSomething() {

  }
}
var a, b, c, d, e;
var instance = new SomeClass(1, 2, 3);

@billti
Copy link
Member

billti commented Mar 7, 2016

This is by design. We show the properties we know to exist with a definitive icon at the top, and the rest of the identifiers in the file afterwards. In the more recent builds the assignment to this. in the constructor would be recognized also (e.g. as shown below on my machine).

constructor

@pflannery
Copy link

@billti thanks for the info. Is there a way to turn of this noise?

@mnpenner
Copy link

@billti Is there any way to hide the extraneous identifiers? I don't see how x.Foo or x.get would ever make sense. It just looks like clutter to me.

@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
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

6 participants