From 4003dcf49de6cc65251271b9989582d494789257 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Tue, 13 Aug 2024 18:17:22 -0700 Subject: [PATCH] feat: Add rule disallowing use of NodeJS global objects (#1309) * Add no-node-globals lint rule * Remove debug print * Lint + fmt * Update docs.json * Address review comments --- Cargo.lock | 1 + Cargo.toml | 1 + docs/rules/no_node_globals.md | 21 +++ src/rules.rs | 2 + src/rules/no_node_globals.rs | 326 ++++++++++++++++++++++++++++++++++ www/static/docs.json | 7 + 6 files changed, 358 insertions(+) create mode 100644 docs/rules/no_node_globals.md create mode 100644 src/rules/no_node_globals.rs diff --git a/Cargo.lock b/Cargo.lock index d7f7b55b..7cadaf07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -312,6 +312,7 @@ dependencies = [ "log", "once_cell", "os_pipe", + "phf", "pulldown-cmark", "rayon", "regex", diff --git a/Cargo.toml b/Cargo.toml index 1dfb1008..ee5b3667 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ once_cell = "1.19.0" derive_more = { version = "0.99.17", features = ["display"] } anyhow = "1.0.79" if_chain = "1.0.2" +phf = { version = "0.11.2", features = ["macros"] } [dev-dependencies] ansi_term = "0.12.1" diff --git a/docs/rules/no_node_globals.md b/docs/rules/no_node_globals.md new file mode 100644 index 00000000..dd662471 --- /dev/null +++ b/docs/rules/no_node_globals.md @@ -0,0 +1,21 @@ +Disallows the use of NodeJS global objects. + +NodeJS exposes a set of global objects that differs from deno (and the web), so +code should not assume they are available. Instead, import the objects from +their defining modules as needed. + +### Invalid: + +```typescript +// foo.ts +const foo = process.env.FOO; // process is not a global object in deno +``` + +### Valid: + +```typescript +// foo.ts +import process from "node:process"; + +const foo = process.env.FOO; +``` diff --git a/src/rules.rs b/src/rules.rs index 33f80cd5..4f659d6a 100644 --- a/src/rules.rs +++ b/src/rules.rs @@ -70,6 +70,7 @@ pub mod no_irregular_whitespace; pub mod no_misused_new; pub mod no_namespace; pub mod no_new_symbol; +pub mod no_node_globals; pub mod no_non_null_asserted_optional_chain; pub mod no_non_null_assertion; pub mod no_obj_calls; @@ -301,6 +302,7 @@ fn get_all_rules_raw() -> Vec> { Box::new(no_misused_new::NoMisusedNew), Box::new(no_namespace::NoNamespace), Box::new(no_new_symbol::NoNewSymbol), + Box::new(no_node_globals::NoNodeGlobals), Box::new( no_non_null_asserted_optional_chain::NoNonNullAssertedOptionalChain, ), diff --git a/src/rules/no_node_globals.rs b/src/rules/no_node_globals.rs new file mode 100644 index 00000000..60717253 --- /dev/null +++ b/src/rules/no_node_globals.rs @@ -0,0 +1,326 @@ +// Copyright 2020-2024 the Deno authors. All rights reserved. MIT license. +use super::program_ref; +use super::Context; +use super::LintRule; +use crate::diagnostic::LintFix; +use crate::diagnostic::LintFixChange; +use crate::handler::Handler; +use crate::handler::Traverse; +use crate::Program; +use std::borrow::Cow; + +use deno_ast::view as ast_view; +use deno_ast::SourcePos; +use deno_ast::SourceRange; +use deno_ast::SourceRanged; +use deno_ast::SourceRangedForSpanned; + +#[derive(Debug)] +pub struct NoNodeGlobals; + +const CODE: &str = "no-node-globals"; +const MESSAGE: &str = "NodeJS globals are not available in Deno"; +const IMPORT_HINT: &str = "Import from the appropriate module instead"; +const IMPORT_FIX_DESC: &str = "Replace node global with module import"; +const REPLACE_FIX_DESC: &str = "Replace node global with corresponding value"; +const REPLACE_HINT: &str = "Use the corresponding value instead"; + +static NODE_GLOBALS: phf::Map<&'static str, FixKind> = phf::phf_map! { + "process" => FixKind::Import { module: "node:process", import: "process" }, + "Buffer" => FixKind::Import { module: "node:buffer", import: "{ Buffer }" }, + "global" => FixKind::Replace("globalThis"), + "setImmediate" => FixKind::Import { module: "node:timers", import: "{ setImmediate }" }, + "clearImmediate" => FixKind::Import { module: "node:timers", import: "{ clearImmediate }" }, +}; + +impl LintRule for NoNodeGlobals { + fn lint_program_with_ast_view<'view>( + &self, + context: &mut Context<'view>, + program: Program<'view>, + ) { + NoNodeGlobalsHandler { + most_recent_import_range: None, + } + .traverse(program, context); + } + + fn code(&self) -> &'static str { + CODE + } + + fn tags(&self) -> &'static [&'static str] { + &["recommended"] + } + + #[cfg(feature = "docs")] + fn docs(&self) -> &'static str { + include_str!("../../docs/rules/no_node_globals.md") + } +} + +struct NoNodeGlobalsHandler { + most_recent_import_range: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum FixKind { + Import { + module: &'static str, + import: &'static str, + }, + Replace(&'static str), +} + +#[derive(Default)] +enum AddNewline { + Leading, + Trailing, + #[default] + None, +} + +impl FixKind { + fn hint(&self) -> &'static str { + match self { + FixKind::Import { .. } => IMPORT_HINT, + FixKind::Replace(_) => REPLACE_HINT, + } + } + + fn description(&self) -> &'static str { + match self { + FixKind::Import { .. } => IMPORT_FIX_DESC, + FixKind::Replace(_) => REPLACE_FIX_DESC, + } + } + + fn to_text(self, newline: AddNewline) -> Cow<'static, str> { + match self { + FixKind::Import { module, import } => { + let (leading, trailing) = match newline { + AddNewline::Leading => ("\n", ""), + AddNewline::Trailing => ("", "\n"), + AddNewline::None => ("", ""), + }; + format!("{leading}import {import} from \"{module}\";{trailing}").into() + } + FixKind::Replace(new_text) => new_text.into(), + } + } +} + +fn program_code_start(program: Program) -> SourcePos { + match program_ref(program) { + ast_view::ProgramRef::Module(m) => m + .body + .first() + .map(|node| node.start()) + .unwrap_or(program.start()), + ast_view::ProgramRef::Script(s) => s + .body + .first() + .map(|node| node.start()) + .unwrap_or(program.start()), + } +} + +impl NoNodeGlobalsHandler { + fn fix_change( + &self, + ctx: &mut Context, + range: SourceRange, + fix_kind: FixKind, + ) -> LintFixChange { + // If the fix is an import, we want to insert it after the last import + // statement. If there are no import statements, we want to insert it at + // the beginning of the file (but after any header comments). + let (fix_range, add_newline) = if matches!(fix_kind, FixKind::Import { .. }) + { + if let Some(range) = self.most_recent_import_range { + ( + SourceRange::new(range.end(), range.end()), + AddNewline::Leading, + ) + } else { + let code_start = program_code_start(ctx.program()); + ( + SourceRange::new(code_start, code_start), + AddNewline::Trailing, + ) + } + } else { + (range, AddNewline::None) + }; + LintFixChange { + new_text: fix_kind.to_text(add_newline), + range: fix_range, + } + } + fn add_diagnostic( + &mut self, + ctx: &mut Context, + range: SourceRange, + fix_kind: FixKind, + ) { + let change = self.fix_change(ctx, range, fix_kind); + + ctx.add_diagnostic_with_fixes( + range, + CODE, + MESSAGE, + Some(fix_kind.hint().to_string()), + vec![LintFix { + description: fix_kind.description().into(), + changes: vec![change], + }], + ); + } +} + +impl Handler for NoNodeGlobalsHandler { + fn ident(&mut self, id: &ast_view::Ident, ctx: &mut Context) { + if !NODE_GLOBALS.contains_key(id.sym()) { + return; + } + if id.ctxt() == ctx.unresolved_ctxt() { + self.add_diagnostic(ctx, id.range(), NODE_GLOBALS[id.sym()]); + } + } + + fn import_decl(&mut self, imp: &ast_view::ImportDecl, _ctx: &mut Context) { + self.most_recent_import_range = Some(imp.range()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn valid() { + assert_lint_ok! { + NoNodeGlobals, + "import process from 'node:process';\nconst a = process.env;", + "const process = { env: {} };\nconst a = process.env;", + "import { Buffer } from 'node:buffer';\nconst b = Buffer;", + "const Buffer = {};\nconst b = Buffer;", + "const global = globalThis;\nconst c = global;", + "const setImmediate = () => {};\nconst d = setImmediate;", + "const clearImmediate = () => {};\nconst e = clearImmediate;", + } + } + + #[test] + fn invalid() { + assert_lint_err! { + NoNodeGlobals, + "import a from 'b';\nconst e = process.env;": [ + { + col: 10, + line: 2, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import a from 'b';\nimport process from \"node:process\";\nconst e = process.env;" + ), + } + ], + "const a = process;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import process from \"node:process\";\nconst a = process;" + ), + } + ], + "const b = Buffer;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import { Buffer } from \"node:buffer\";\nconst b = Buffer;" + ), + } + ], + "const c = global;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: REPLACE_HINT, + fix: ( + REPLACE_FIX_DESC, + "const c = globalThis;" + ), + } + ], + "const d = setImmediate;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import { setImmediate } from \"node:timers\";\nconst d = setImmediate;" + ), + } + ], + "const e = clearImmediate;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import { clearImmediate } from \"node:timers\";\nconst e = clearImmediate;" + ), + } + ], + "const a = process.env;\nconst b = Buffer;": [ + { + col: 10, + line: 1, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import process from \"node:process\";\nconst a = process.env;\nconst b = Buffer;" + ), + }, + { + col: 10, + line: 2, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "import { Buffer } from \"node:buffer\";\nconst a = process.env;\nconst b = Buffer;" + ), + } + ], + "// A copyright notice\n\nconst a = process.env;": [ + { + col: 10, + line: 3, + message: MESSAGE, + hint: IMPORT_HINT, + fix: ( + IMPORT_FIX_DESC, + "// A copyright notice\n\nimport process from \"node:process\";\nconst a = process.env;" + ), + } + ] + }; + } +} diff --git a/www/static/docs.json b/www/static/docs.json index 084532d0..a02b191f 100644 --- a/www/static/docs.json +++ b/www/static/docs.json @@ -417,6 +417,13 @@ "recommended" ] }, + { + "code": "no-node-globals", + "docs": "Disallows the use of NodeJS global objects.\n\nNodeJS exposes a set of global objects that differs from deno (and the web), so\ncode should not assume they are available. Instead, import the objects from\ntheir defining modules as needed.\n\n### Invalid:\n\n```typescript\n// foo.ts\nconst foo = process.env.FOO; // process is not a global object in deno\n```\n\n### Valid:\n\n```typescript\n// foo.ts\nimport process from \"node:process\";\n\nconst foo = process.env.FOO;\n```\n", + "tags": [ + "recommended" + ] + }, { "code": "no-non-null-asserted-optional-chain", "docs": "Disallow non-null assertions after an optional chain expression\n\n`?.` optional chain expressions provide undefined if an object is `null` or\n`undefined`. Using a `!` non-null assertion to assert the result of an `?.`\noptional chain expression is non-nullable is likely wrong.\n\n### Invalid:\n\n```typescript\nfoo?.bar!;\nfoo?.bar()!;\n```\n\n### Valid:\n\n```typescript\nfoo?.bar;\nfoo?.bar();\n```\n",