Skip to content
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

TSLint: show a warning when accessing node.js globals in common|browser #79223

Closed
bpasero opened this issue Aug 15, 2019 · 2 comments
Closed
Assignees
Labels
debt Code quality issues web Issues related to running VSCode in the web
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Aug 15, 2019

We have lots of TSLint rules today that help us prevent pushing badly layered code. However we fail to detect usage such as process or Buffer in common|browser layer.

I am not an expert in TS and have just thrown together a trivial version that checks on identifier names (in #79222):

image

I need someone with more expertise to make it smarter (@mjbvz ?). Ideally we can only report a warning if the resolved type is coming from a certain d.ts file (e.g. node.d.ts). Not sure if that is possible.

Implementation:

visitIdentifier(node: ts.Identifier) {
	if (this._config.unsafe.some(unsafe => unsafe === node.text)) {
		this.addFailureAtNode(node, `Unsafe global usage of ${node.text} in ${this._config.target}`);
	}

	super.visitIdentifier(node);
}
@bpasero bpasero added debt Code quality issues web Issues related to running VSCode in the web labels Aug 15, 2019
@bpasero bpasero added this to the August 2019 milestone Aug 15, 2019
@bpasero bpasero self-assigned this Aug 15, 2019
@bpasero
Copy link
Member Author

bpasero commented Aug 16, 2019

After looking into this a bit more I am less optimistic that this can be done with TSLint 4. With TSLint 5 it actually looks better, given: https://github.com/palantir/tslint/blob/master/src/rules/noRestrictedGlobalsRule.ts

@bpasero
Copy link
Member Author

bpasero commented Aug 19, 2019

landed via 90a35ec. Turns out you can do rules with resolved symbols and that allows to be smart.

@bpasero bpasero closed this as completed Aug 19, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues web Issues related to running VSCode in the web
Projects
None yet
Development

No branches or pull requests

1 participant