search

denoland / deno_lint

Blazing fast linter for JavaScript and TypeScript written in Rust
https://lint.deno.land/
MIT License
1.53k stars 172 forks source link

`deno lint` needs detection of undefined identifiers (`no-undef`) to be useful #1327

Open mitranim opened 3 years ago

mitranim commented 3 years ago

Currently, deno lint doesn't complain about missing identifiers. The following JS program passes linting but explodes at runtime:

blah

In my experience, this is an extremely common source of bugs. The TS compiler checks this, but not every project uses TS. For JS, a linter able to detect this is essential; as a result, deno lint is not sufficient, forcing the use of eslint.

The main difficulty seems to be that no-undef requires knowledge of allowed globals, with different sets for different environments, and may involve config files like .eslintrc, which Deno seems to staunchly oppose. Right now, I don't have a concrete suggestion.

lucacasonato commented 3 years ago

We used to have this rule, but disabled this specifically because it requires knowledge of allowed globals. If there is an elegant approach to solve this I would be happy to look into it again, but right now it is a strong no from me.

mitranim commented 3 years ago

I would probably be satisfied if the linter pre-defined a small set of globals and required the user to supply the rest.

For comparison, eslint seems to pre-define an insanely huge list of globals, including "gotcha" globals such as event, close, toString, and others. This has led to actual bugs, and now my eslint configs have a huge list of banned globals, undoing eslint's mistake.

Even having an opt-in no-undef with no predefined globals, and a way to supply them, would make deno lint immensely more useful than it currently is. We might be able to settle on some set. Extensibility is a must.

lucacasonato commented 3 years ago

@mitranim The problem is the following:

Also have you considered using JS + TSDoc comments? That can also take care of this issue.

mitranim commented 3 years ago

Wasn't aware of TSDoc. Googling now, but it seems to involve extra comments, which seems like a burden. Looking for ways to validate "just code", just like eslint does today.

To avoid continuous maintenance and arguments, we could disable the rule by default and have no predefined globals. Certainly not a "batteries included" / "one size fits all" solution. But could make the tool actually useful to those willing to write a tiny config.

Configuration API is another question. Initially, we could just use CLI flags. However: linter configuration can get large; CLI flags don't allow per-directory configuration; CLI flags make editor integration much more difficult compared to auto-discovered config files. On the other hand, adding a config file just for linting might be questionable, because Deno has multiple features, and over time we might end up with a multitude of configs. Neither sounds ideal.

magurotuna commented 3 years ago

A sensible solution would be https://github.com/denoland/deno_lint/issues/622 - making the linter capable of parsing lib.*.d.ts to get global symbols. This way we (as Deno's developers) won't have to maintain the set of global variables all the time and perhaps users could specify any type declaration files they want to use.