Add seperate no-process-global lint rule (#1330)

docs/rules/no_node_globals.md

@@ -8,14 +8,14 @@ their defining modules as needed.
 
 ```typescript
 // foo.ts
-const foo = process.env.FOO; // process is not a global object in deno
+const buf = Buffer.from("foo", "utf-8"); // Buffer is not a global object in deno
 ```
 
 ### Valid:
 
 ```typescript
 // foo.ts
-import process from "node:process";
+import { Buffer } from "node:buffer";
 
-const foo = process.env.FOO;
+const foo = Buffer.from("foo", "utf-8");
 ```

docs/rules/no_process_global.md

@@ -0,0 +1,21 @@
+Disallows the use of NodeJS `process` global.
+
+NodeJS and Deno expose `process` global but they are hard to statically analyze
+by tools, so code should not assume they are available. Instead,
+`import process from "node:process"`.
+
+### Invalid:
+
+```typescript
+// foo.ts
+const foo = process.env.FOO;
+```
+
+### Valid:
+
+```typescript
+// foo.ts
+import process from "node:process";
+
+const foo = process.env.FOO;
+```

src/rules.rs

@@ -75,6 +75,7 @@ pub mod no_non_null_asserted_optional_chain;
 pub mod no_non_null_assertion;
 pub mod no_obj_calls;
 pub mod no_octal;
+pub mod no_process_global;
 pub mod no_prototype_builtins;
 pub mod no_redeclare;
 pub mod no_regex_spaces;
@@ -309,6 +310,7 @@ fn get_all_rules_raw() -> Vec<Box<dyn LintRule>> {
  Box::new(no_non_null_assertion::NoNonNullAssertion),
  Box::new(no_obj_calls::NoObjCalls),
  Box::new(no_octal::NoOctal),
+ Box::new(no_process_global::NoProcessGlobal),
  Box::new(no_prototype_builtins::NoPrototypeBuiltins),
  Box::new(no_redeclare::NoRedeclare),
  Box::new(no_regex_spaces::NoRegexSpaces),

src/rules/no_node_globals.rs

@@ -22,7 +22,6 @@ const CODE: &str = "no-node-globals";
 const MESSAGE: &str = "NodeJS globals are not available in Deno";
 
 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 }" },
@@ -201,8 +200,6 @@ mod tests {
  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;",
@@ -215,30 +212,6 @@ mod tests {
  fn invalid() {
  assert_lint_err! {
  NoNodeGlobals,
- "import a from 'b';\nconst e = process.env;": [
- {
- col: 10,
- line: 2,
- message: MESSAGE,
- hint: "Add `import process from \"node:process\";`",
- fix: (
- "Import from \"node:process\"",
- "import a from 'b';\nimport process from \"node:process\";\nconst e = process.env;"
- ),
- }
- ],
- "const a = process;": [
- {
- col: 10,
- line: 1,
- message: MESSAGE,
- hint: "Add `import process from \"node:process\";`",
- fix: (
- "Import from \"node:process\"",
- "import process from \"node:process\";\nconst a = process;"
- ),
- }
- ],
  "const b = Buffer;": [
  {
  col: 10,
@@ -287,15 +260,15 @@ mod tests {
  ),
  }
  ],
- "const a = process.env;\nconst b = Buffer;": [
+ "const a = setImmediate;\nconst b = Buffer;": [
  {
  col: 10,
  line: 1,
  message: MESSAGE,
- hint: "Add `import process from \"node:process\";`",
+ hint: "Add `import { setImmediate } from \"node:timers\";`",
  fix: (
- "Import from \"node:process\"",
- "import process from \"node:process\";\nconst a = process.env;\nconst b = Buffer;"
+ "Import from \"node:timers\"",
+ "import { setImmediate } from \"node:timers\";\nconst a = setImmediate;\nconst b = Buffer;"
  ),
  },
  {
@@ -305,19 +278,19 @@ mod tests {
  hint: "Add `import { Buffer } from \"node:buffer\";`",
  fix: (
  "Import from \"node:buffer\"",
- "import { Buffer } from \"node:buffer\";\nconst a = process.env;\nconst b = Buffer;"
+ "import { Buffer } from \"node:buffer\";\nconst a = setImmediate;\nconst b = Buffer;"
  ),
  }
  ],
- "// A copyright notice\n\nconst a = process.env;": [
+ "// A copyright notice\n\nconst a = setImmediate;": [
  {
  col: 10,
  line: 3,
  message: MESSAGE,
- hint: "Add `import process from \"node:process\";`",
+ hint: "Add `import { setImmediate } from \"node:timers\";`",
  fix: (
- "Import from \"node:process\"",
- "// A copyright notice\n\nimport process from \"node:process\";\nconst a = process.env;"
+ "Import from \"node:timers\"",
+ "// A copyright notice\n\nimport { setImmediate } from \"node:timers\";\nconst a = setImmediate;"
  ),
  }
  ]

src/rules/no_process_global.rs

@@ -0,0 +1,176 @@
+// 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 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 NoProcessGlobal;
+
+const CODE: &str = "no-process-globals";
+const MESSAGE: &str = "NodeJS process global is discouraged Deno";
+
+impl LintRule for NoProcessGlobal {
+ fn lint_program_with_ast_view<'view>(
+ &self,
+ context: &mut Context<'view>,
+ program: Program<'view>,
+ ) {
+ NoProcessGlobalHandler {
+ 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_process_global.md")
+ }
+}
+
+struct NoProcessGlobalHandler {
+ most_recent_import_range: Option<SourceRange>,
+}
+
+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 NoProcessGlobalHandler {
+ fn fix_change(&self, ctx: &mut Context) -> 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, leading, trailing) =
+ if let Some(range) = self.most_recent_import_range {
+ (SourceRange::new(range.end(), range.end()), "\n", "")
+ } else {
+ let code_start = program_code_start(ctx.program());
+ (SourceRange::new(code_start, code_start), "", "\n")
+ };
+
+ LintFixChange {
+ new_text: format!(
+ "{leading}import process from \"node:process\";{trailing}"
+ )
+ .into(),
+ range: fix_range,
+ }
+ }
+
+ fn add_diagnostic(&mut self, ctx: &mut Context, range: SourceRange) {
+ let change = self.fix_change(ctx);
+
+ ctx.add_diagnostic_with_fixes(
+ range,
+ CODE,
+ MESSAGE,
+ Some(String::from("Add `import process from \"node:process\";`")),
+ vec![LintFix {
+ description: "Import from \"node:process\"".into(),
+ changes: vec![change],
+ }],
+ );
+ }
+}
+
+impl Handler for NoProcessGlobalHandler {
+ fn ident(&mut self, id: &ast_view::Ident, ctx: &mut Context) {
+ if id.sym() != "process" {
+ return;
+ }
+ if id.ctxt() == ctx.unresolved_ctxt() {
+ self.add_diagnostic(ctx, id.range());
+ }
+ }
+
+ 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! {
+ NoProcessGlobal,
+ "import process from 'node:process';\nconst a = process.env;",
+ "const process = { env: {} };\nconst a = process.env;",
+ }
+ }
+
+ #[test]
+ fn invalid() {
+ assert_lint_err! {
+ NoProcessGlobal,
+ "import a from 'b';\nconst e = process.env;": [
+ {
+ col: 10,
+ line: 2,
+ message: MESSAGE,
+ hint: "Add `import process from \"node:process\";`",
+ fix: (
+ "Import from \"node:process\"",
+ "import a from 'b';\nimport process from \"node:process\";\nconst e = process.env;"
+ ),
+ }
+ ],
+ "const a = process;": [
+ {
+ col: 10,
+ line: 1,
+ message: MESSAGE,
+ hint: "Add `import process from \"node:process\";`",
+ fix: (
+ "Import from \"node:process\"",
+ "import process from \"node:process\";\nconst a = process;"
+ ),
+ }
+ ],
+ "// A copyright notice\n\nconst a = process.env;": [
+ {
+ col: 10,
+ line: 3,
+ message: MESSAGE,
+ hint: "Add `import process from \"node:process\";`",
+ fix: (
+ "Import from \"node:process\"",
+ "// A copyright notice\n\nimport process from \"node:process\";\nconst a = process.env;"
+ ),
+ }
+ ]
+ };
+ }
+}

www/static/docs.json

@@ -419,7 +419,7 @@
  },
  {
  "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",
+ "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 buf = Buffer.from(\"foo\", \"utf-8\"); // Buffer is not a global object in deno\n```\n\n### Valid:\n\n```typescript\n// foo.ts\nimport { Buffer } from \"node:buffer\";\n\nconst foo = Buffer.from(\"foo\", \"utf-8\");\n```\n",
  "tags": [
  "recommended"
  ]
@@ -448,6 +448,13 @@
  "recommended"
  ]
  },
+ {
+ "code": "no-process-globals",
+ "docs": "Disallows the use of NodeJS `process` global.\n\nNodeJS and Deno expose `process` global but they are hard to statically analyze\nby tools, so code should not assume they are available. Instead,\n`import process from \"node:process\"`.\n\n### Invalid:\n\n```typescript\n// foo.ts\nconst foo = process.env.FOO;\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-prototype-builtins",
  "docs": "Disallows the use of `Object.prototype` builtins directly\n\nIf objects are created via `Object.create(null)` they have no prototype\nspecified. This can lead to runtime errors when you assume objects have\nproperties from `Object.prototype` and attempt to call the following methods:\n\n- `hasOwnProperty`\n- `isPrototypeOf`\n- `propertyIsEnumerable`\n\nInstead, it's always encouraged to call these methods from `Object.prototype`\nexplicitly.\n\n### Invalid:\n\n```typescript\nconst a = foo.hasOwnProperty(\"bar\");\nconst b = foo.isPrototypeOf(\"bar\");\nconst c = foo.propertyIsEnumerable(\"bar\");\n```\n\n### Valid:\n\n```typescript\nconst a = Object.prototype.hasOwnProperty.call(foo, \"bar\");\nconst b = Object.prototype.isPrototypeOf.call(foo, \"bar\");\nconst c = Object.prototype.propertyIsEnumerable.call(foo, \"bar\");\n```\n",