From 5ec3c5c3a46ca95f355a4520676b85ac619ca102 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Thu, 15 Aug 2024 13:43:04 -0700 Subject: [PATCH] feat(lint): Add lint for usage of node globals (with autofix) (#25048) From upgrading `deno_lint`. Previously if you had a node project that used a bunch of node globals (`process.env`, etc), you would have to fix the errors by hand. This PR includes a new lint that detects usages of node globals (`process`, `setImmediate`, `Buffer`, etc.) and provides an autofix to import the correct value. For instance: ```ts // main.ts const _foo = process.env.FOO; ``` `deno lint` gives you ```ts error[no-node-globals]: NodeJS globals are not available in Deno --> /home/foo.ts:1:14 | 1 | const _foo = process.env.FOO; | ^^^^^^^ = hint: Add `import process from "node:process";` docs: https://lint.deno.land/rules/no-node-globals Found 1 problem (1 fixable via --fix) Checked 1 file ``` And `deno lint --fix` adds the import for you: ```ts // main.ts import process from "node:process"; const _foo = process.env.FOO; ``` --- Cargo.lock | 5 ++-- cli/Cargo.toml | 2 +- cli/tools/lint/linter.rs | 14 +++++++++-- .../__test__.jsonc | 23 +++++++++++++++++++ .../error.out | 4 ++++ .../lint.out | 22 ++++++++++++++++++ .../node_globals_no_duplicate_imports/main.ts | 7 ++++++ 7 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 tests/specs/lint/node_globals_no_duplicate_imports/__test__.jsonc create mode 100644 tests/specs/lint/node_globals_no_duplicate_imports/error.out create mode 100644 tests/specs/lint/node_globals_no_duplicate_imports/lint.out create mode 100644 tests/specs/lint/node_globals_no_duplicate_imports/main.ts diff --git a/Cargo.lock b/Cargo.lock index e3cdc49d6f..f5ace65510 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1703,9 +1703,9 @@ dependencies = [ [[package]] name = "deno_lint" -version = "0.62.0" +version = "0.63.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69bb0887acec1830cac1a325ebe5cec96fdc41f61ee50e7b25447741007757b6" +checksum = "e0e6cc8fcb4819dd5e12d640d6fc455217c66bda00e30fd6d46d2844e3e1bdcf" dependencies = [ "anyhow", "deno_ast", @@ -1713,6 +1713,7 @@ dependencies = [ "if_chain", "log", "once_cell", + "phf 0.11.2", "regex", "serde", "serde_json", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index bb6508b55c..e53979298f 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -70,7 +70,7 @@ deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] deno_doc = { version = "0.146.0", features = ["html", "syntect"] } deno_emit = "=0.44.0" deno_graph = { version = "=0.81.2" } -deno_lint = { version = "=0.62.0", features = ["docs"] } +deno_lint = { version = "=0.63.1", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.21.4" deno_package_json.workspace = true diff --git a/cli/tools/lint/linter.rs b/cli/tools/lint/linter.rs index f6ea76c14c..777fe4d09b 100644 --- a/cli/tools/lint/linter.rs +++ b/cli/tools/lint/linter.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::collections::HashSet; use std::path::Path; use deno_ast::MediaType; @@ -225,14 +226,23 @@ fn apply_lint_fixes( if quick_fixes.is_empty() { return None; } + + let mut import_fixes = HashSet::new(); // remove any overlapping text changes, we'll circle // back for another pass to fix the remaining quick_fixes.sort_by_key(|change| change.range.start); for i in (1..quick_fixes.len()).rev() { let cur = &quick_fixes[i]; let previous = &quick_fixes[i - 1]; - let is_overlapping = cur.range.start < previous.range.end; - if is_overlapping { + // hack: deduplicate import fixes to avoid creating errors + if previous.new_text.trim_start().starts_with("import ") { + import_fixes.insert(previous.new_text.trim().to_string()); + } + let is_overlapping = cur.range.start <= previous.range.end; + if is_overlapping + || (cur.new_text.trim_start().starts_with("import ") + && import_fixes.contains(cur.new_text.trim())) + { quick_fixes.remove(i); } } diff --git a/tests/specs/lint/node_globals_no_duplicate_imports/__test__.jsonc b/tests/specs/lint/node_globals_no_duplicate_imports/__test__.jsonc new file mode 100644 index 0000000000..d8e00f47ad --- /dev/null +++ b/tests/specs/lint/node_globals_no_duplicate_imports/__test__.jsonc @@ -0,0 +1,23 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "run main.ts", + "output": "error.out", + "exitCode": 1 + }, + { + "args": "lint main.ts", + "output": "lint.out", + "exitCode": 1 + }, + { + "args": "lint --fix main.ts", + "output": "Checked 1 file\n" + }, + { + "args": "run --allow-env main.ts", + "output": "" + } + ] +} diff --git a/tests/specs/lint/node_globals_no_duplicate_imports/error.out b/tests/specs/lint/node_globals_no_duplicate_imports/error.out new file mode 100644 index 0000000000..5671b17b46 --- /dev/null +++ b/tests/specs/lint/node_globals_no_duplicate_imports/error.out @@ -0,0 +1,4 @@ +error: Uncaught (in promise) ReferenceError: process is not defined +const _foo = process.env.FOO; + ^ + at [WILDCARD]main.ts:3:14 diff --git a/tests/specs/lint/node_globals_no_duplicate_imports/lint.out b/tests/specs/lint/node_globals_no_duplicate_imports/lint.out new file mode 100644 index 0000000000..b396e71ebd --- /dev/null +++ b/tests/specs/lint/node_globals_no_duplicate_imports/lint.out @@ -0,0 +1,22 @@ +error[no-node-globals]: NodeJS globals are not available in Deno + --> [WILDCARD]main.ts:3:14 + | +3 | const _foo = process.env.FOO; + | ^^^^^^^ + = hint: Add `import process from "node:process";` + + docs: https://lint.deno.land/rules/no-node-globals + + +error[no-node-globals]: NodeJS globals are not available in Deno + --> [WILDCARD]main.ts:7:14 + | +7 | const _bar = process.env.BAR; + | ^^^^^^^ + = hint: Add `import process from "node:process";` + + docs: https://lint.deno.land/rules/no-node-globals + + +Found 2 problems (2 fixable via --fix) +Checked 1 file diff --git a/tests/specs/lint/node_globals_no_duplicate_imports/main.ts b/tests/specs/lint/node_globals_no_duplicate_imports/main.ts new file mode 100644 index 0000000000..bff428d012 --- /dev/null +++ b/tests/specs/lint/node_globals_no_duplicate_imports/main.ts @@ -0,0 +1,7 @@ +import {} from "node:console"; + +const _foo = process.env.FOO; + +import {} from "node:assert"; + +const _bar = process.env.BAR;