Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

New rule to forbid access to commonly-confused ambient globals #922

Closed
adidahiya opened this issue Jan 18, 2016 · 7 comments
Closed

New rule to forbid access to commonly-confused ambient globals #922

adidahiya opened this issue Jan 18, 2016 · 7 comments

Comments

@adidahiya
Copy link
Contributor

For example, window.event (here in lib.d.ts) is often referenced in erroneous code like this:

function broken(evt: Event) {
    // Meant to do something with `evt` but typed it incorrectly.
    Event.target;  // compiler error
    event.target;  // should be a lint failure
}
@myitcv
Copy link
Contributor

myitcv commented Jan 21, 2016

Also referencing this thread because I'm unclear how such a lint rule could even be defined given the number of such variables.

Perhaps the solution can be approached by using a modified lib.dom.d.ts that does not define the globals like event but instead relies on the fact that the same objects/functions are available via window (which would be one of a well defined, very small number of globals that would include core types). That way, people would be forced to do something like this:

function Apple() {
  console.log(event); // error; event no longer defined as a global
  console.log(window.event); // ok
}

Thoughts?

Not tested this so this might break some type definitions...?

@adidahiya
Copy link
Contributor Author

@myitcv Is such an approach feasible without lib.d.ts modularity in TS 2.0?

This issue is fairly similar to #327. Both would almost certainly involve using the type checker (but not necessarily #680 because we could just tell langauge services to consider lib.d.ts and the type checker will pick up those global types).

@myitcv
Copy link
Contributor

myitcv commented Jan 21, 2016

@adidahiya

Is such an approach feasible without lib.d.ts modularity in TS 2.0?

It is possible but perhaps not the cleanest solution.

We use custom lib.core.d.ts and lib.dom.d.ts via the --noLib compiler option. The custom lib.core.d.ts and lib.dom.d.ts files we use are well defined transformations of the TypeScript provided versions. The transformations we currently apply are:

My plan would be to try a version of lib.dom.d.ts (the var's in lib.core.d.ts are all core types) that removes all var definitions which I would prefer to explicitly access via window.

Theoretically, if I published these modified files others would also be able to use the --noLib switch, install these type definitions somehow (DefinitelyTyped or similar) and benefit from this modular approach.

@Patrik-Lundqvist
Copy link

Patrik-Lundqvist commented Oct 10, 2017

Any progress on this? ESLint has the rule no-restricted-globals which covers this, maybe a similar definition can be used here too?

@JoshuaKGoldberg
Copy link
Contributor

Copying discussion from #3824:

What if you declare something in an externals.d.ts file, or define your own lib.dom.d.ts with either that name or my-lib.d.ts?

Or what about a project that uses namespaces and has a const name = "HA!"; somewhere?

Maybe the rule should get the list of files TS is compiling with (can we do that?) along with its compiler options, and add lib.d.ts-like files as per lib and --noLib compiler settings?
Maybe the rule should keep a default list of regular expressions to match names against, like lib*.d.ts?

@Zzzen
Copy link
Contributor

Zzzen commented Apr 28, 2018

What if you declare something in an externals.d.ts file, or define your own lib.dom.d.ts with either that name or my-lib.d.ts?

IMO, there should be no difference between official lib.dom.d.ts, 3rd party library declaration and custom declaration.

Or what about a project that uses namespaces and has a const name = "HA!"; somewhere?

A more concret example would be helpful. Maybe This issue is related. microsoft/TypeScript#2715

Maybe the rule should get the list of files TS is compiling with (can we do that?) along with its compiler options, and add lib.d.ts-like files as per lib and --noLib compiler settings?
Maybe the rule should keep a default list of regular expressions to match names against, like lib*.d.ts?

We are banning globals, no matter where they are declared.

@cazgp
Copy link

cazgp commented Apr 25, 2019

This rule was added: https://palantir.github.io/tslint/rules/no-restricted-globals/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants