-
-
Notifications
You must be signed in to change notification settings - Fork 19
feat(process-assert-to-assert): introduce #200
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
base: main
Are you sure you want to change the base?
Changes from all commits
a96083c
48c4a1d
91b0098
ccec01a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# `process.assert` DEP0100 | ||
|
||
This recipe transforms the usage of `process.assert` to use `assert` module. | ||
|
||
See [DEP0100](https://github.com/nodejs/userland-migrations/issues/197). | ||
|
||
## Example | ||
|
||
**Before:** | ||
|
||
```js | ||
process.assert(condition, "Assertion failed"); | ||
``` | ||
|
||
**After:** | ||
|
||
```js | ||
import assert from "node:assert"; | ||
assert(condition, "Assertion failed"); | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
schema_version: "1.0" | ||
name: "@nodejs/process-assert-to-assert" | ||
version: 1.0.0 | ||
description: Handle DEP0100 via transforming `process.assert` to `assert`. | ||
author: matheusmorett2 | ||
license: MIT | ||
workflow: workflow.yaml | ||
category: migration | ||
|
||
targets: | ||
languages: | ||
- javascript | ||
- typescript | ||
|
||
keywords: | ||
- transformation | ||
- migration | ||
|
||
registry: | ||
access: public | ||
visibility: public |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
{ | ||
"name": "@nodejs/process-assert-to-assert", | ||
"version": "1.0.0", | ||
"description": "Handle DEP0100 via transforming `process.assert` to `assert`.", | ||
"type": "module", | ||
"scripts": { | ||
"test": "npx codemod jssg test -l typescript --ignore-whitespace ./src/workflow.ts ./" | ||
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "git+https://github.com/nodejs/userland-migrations.git", | ||
"directory": "recipes/process-assert-to-assert", | ||
"bugs": "https://github.com/nodejs/userland-migrations/issues" | ||
}, | ||
"author": "matheusmorett2", | ||
"license": "MIT", | ||
"homepage": "https://github.com/nodejs/userland-migrations/blob/main/recipes/process-assert-to-assert/README.md", | ||
"devDependencies": { | ||
"@codemod.com/jssg-types": "^1.0.3" | ||
}, | ||
"dependencies": { | ||
"@nodejs/codemod-utils": "*" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,183 @@ | ||||||||
import type { Edit, Range, Rule, SgNode, SgRoot } from "@codemod.com/jssg-types/main"; | ||||||||
import { getNodeRequireCalls } from "@nodejs/codemod-utils/ast-grep/require-call"; | ||||||||
import { getNodeImportStatements } from "@nodejs/codemod-utils/ast-grep/import-statement"; | ||||||||
import { resolveBindingPath } from "@nodejs/codemod-utils/ast-grep/resolve-binding-path"; | ||||||||
import { removeBinding } from "@nodejs/codemod-utils/ast-grep/remove-binding"; | ||||||||
import { removeLines } from "@nodejs/codemod-utils/ast-grep/remove-lines"; | ||||||||
import { EOL } from "node:os"; | ||||||||
|
||||||||
/** | ||||||||
* Transforms deprecated `process.assert` usage to the standard `assert` module. | ||||||||
* | ||||||||
* Transformations: | ||||||||
* 1. Replaces all `process.assert` references with `assert` | ||||||||
* 2. Adds the necessary import/require statement if not already present: | ||||||||
* - For ESM or files without require calls: adds `import assert from "node:assert"` | ||||||||
* - For CommonJS (.cjs files or files using require): adds `const assert = require("node:assert")` | ||||||||
* 3. Removes process import/require if it was only used for assert | ||||||||
* | ||||||||
* Examples: | ||||||||
* | ||||||||
* **Before**: | ||||||||
* ```js | ||||||||
* import process from "node:process"; | ||||||||
* process.assert(value); | ||||||||
* process.assert.strictEqual(a, b); | ||||||||
* ``` | ||||||||
* | ||||||||
* **After**: | ||||||||
* ```js | ||||||||
* import assert from "node:assert"; | ||||||||
* assert(value); | ||||||||
* assert.strictEqual(a, b); | ||||||||
* ``` | ||||||||
*/ | ||||||||
export default function transform(root: SgRoot): string | null { | ||||||||
const rootNode = root.root(); | ||||||||
const edits: Edit[] = []; | ||||||||
const linesToRemove: Range[] = []; | ||||||||
const replaceRules: Array<{ | ||||||||
importNode?: SgNode; | ||||||||
binding?: string; | ||||||||
rule: Rule; | ||||||||
replaceWith?: string; | ||||||||
}> = [{ | ||||||||
rule: { | ||||||||
kind: 'member_expression', | ||||||||
pattern: "process.assert", | ||||||||
}, | ||||||||
replaceWith: "assert" | ||||||||
}]; | ||||||||
|
||||||||
const processImportsToRemove = new Set<SgNode>(); | ||||||||
|
||||||||
function processImports(moduleName: "process" | "node:process") { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Utilities used inside this function have regex to catch both usage of |
||||||||
const requireCalls = getNodeRequireCalls(root, moduleName); | ||||||||
const importStatements = getNodeImportStatements(root, moduleName); | ||||||||
const allImports = [...requireCalls, ...importStatements]; | ||||||||
|
||||||||
for (const processImport of allImports) { | ||||||||
const binding = resolveBindingPath(processImport, "$.assert"); | ||||||||
replaceRules.push({ | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
importNode: processImport, | ||||||||
binding, | ||||||||
rule: { | ||||||||
kind: "identifier", | ||||||||
regex: binding, | ||||||||
inside: { | ||||||||
kind: 'call_expression', | ||||||||
} | ||||||||
}, | ||||||||
replaceWith: "assert" | ||||||||
}); | ||||||||
|
||||||||
if (binding) { | ||||||||
replaceRules.push({ | ||||||||
importNode: processImport, | ||||||||
binding, | ||||||||
rule: { | ||||||||
kind: "member_expression", | ||||||||
has: { | ||||||||
kind: "identifier", | ||||||||
regex: `^${binding}$`, | ||||||||
field: "object" | ||||||||
} | ||||||||
}, | ||||||||
replaceWith: "assert" | ||||||||
}); | ||||||||
} | ||||||||
|
||||||||
const processUsages = rootNode.findAll({ | ||||||||
rule: { | ||||||||
kind: 'member_expression', | ||||||||
has: { | ||||||||
kind: 'identifier', | ||||||||
regex: '^process$' | ||||||||
} | ||||||||
} | ||||||||
}); | ||||||||
Comment on lines
+74
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brunocroh correct me if I am wrong but here dinging also include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so |
||||||||
|
||||||||
let hasNonAssertUsage = false; | ||||||||
for (const usage of processUsages) { | ||||||||
const propertyNode = usage.field("property"); | ||||||||
if (propertyNode && propertyNode.text() !== "assert") { | ||||||||
hasNonAssertUsage = true; | ||||||||
break; | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
if (!hasNonAssertUsage && processUsages.length > 0) { | ||||||||
processImportsToRemove.add(processImport); | ||||||||
linesToRemove.push(processImport.range()); | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
processImports("process"); | ||||||||
processImports("node:process"); | ||||||||
|
||||||||
for (const replaceRule of replaceRules) { | ||||||||
const nodes = rootNode.findAll({ | ||||||||
rule: replaceRule.rule | ||||||||
}); | ||||||||
|
||||||||
for (const node of nodes) { | ||||||||
if (replaceRule.importNode) { | ||||||||
if (!processImportsToRemove.has(replaceRule.importNode)) { | ||||||||
const removeBind = removeBinding(replaceRule.importNode, replaceRule.binding); | ||||||||
|
||||||||
if (removeBind.edit) { | ||||||||
edits.push(removeBind.edit); | ||||||||
} | ||||||||
|
||||||||
if (removeBind.lineToRemove) { | ||||||||
linesToRemove.push(removeBind.lineToRemove); | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
if (replaceRule.rule.kind === "member_expression" && replaceRule.binding) { | ||||||||
const objectNode = node.field("object"); | ||||||||
if (objectNode) { | ||||||||
edits.push(objectNode.replace("assert")); | ||||||||
} | ||||||||
} else { | ||||||||
const replaceText = replaceRule.replaceWith || "assert"; | ||||||||
edits.push(node.replace(replaceText)); | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
let sourceCode = rootNode.commitEdits(edits); | ||||||||
sourceCode = removeLines(sourceCode, linesToRemove); | ||||||||
|
||||||||
if (edits.length === 0 && linesToRemove) { | ||||||||
return sourceCode; | ||||||||
} | ||||||||
|
||||||||
const alreadyRequiringAssert = getNodeRequireCalls(root, "assert"); | ||||||||
const alreadyImportingAssert = getNodeImportStatements(root, "assert"); | ||||||||
|
||||||||
if (!alreadyRequiringAssert.length && !alreadyImportingAssert.length) { | ||||||||
const usingRequire = rootNode.find({ | ||||||||
rule: { | ||||||||
kind: 'call_expression', | ||||||||
has: { | ||||||||
kind: 'identifier', | ||||||||
field: 'function', | ||||||||
regex: 'require' | ||||||||
} | ||||||||
} | ||||||||
}); | ||||||||
|
||||||||
const isCommonJs = root.filename().includes('.cjs'); | ||||||||
|
||||||||
if (Boolean(usingRequire) || isCommonJs) { | ||||||||
return `const assert = require("node:assert");${EOL}${sourceCode}`; | ||||||||
} | ||||||||
|
||||||||
return `import assert from "node:assert";${EOL}${sourceCode}`; | ||||||||
} | ||||||||
|
||||||||
return sourceCode; | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import assert from "node:assert"; | ||
assert(condition, "Basic assertion"); | ||
assert.strictEqual(a, b, "Values should be equal"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
const assert = require("node:assert"); | ||
assert(condition, "Basic assertion"); | ||
assert.strictEqual(a, b, "Values should be equal"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import assert from "node:assert"; | ||
assert(value, "Process assertion"); | ||
assert.strictEqual(obj1, obj2); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import assert from "node:assert"; | ||
import process from "node:process"; | ||
assert(value, "Process assertion"); | ||
process.env.NODE_ENV = "test"; | ||
console.log(process.pid); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
const assert = require("node:assert"); | ||
assert(value, "Process assertion"); | ||
assert.throws(() => { throw new Error(); }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import assert from "node:assert"; | ||
assert(condition, "Assertion from destructured import"); | ||
assert.throws(() => { throw new Error("test"); }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import assert from "node:assert"; | ||
import { env } from "node:process"; | ||
assert(value, "Using destructured assert"); | ||
console.log(env.NODE_ENV); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import assert from "node:assert"; | ||
assert(value, "Using aliased assert"); | ||
assert.notStrictEqual(a, b); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import assert from "node:assert"; | ||
import { env } from "node:process"; | ||
assert(value, "Using aliased assert"); | ||
assert.strictEqual(a, b); | ||
console.log(env.NODE_ENV); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
const assert = require("node:assert"); | ||
assert(value, "Destructured assert from require"); | ||
assert.strictEqual(a, b, "Should be equal"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import assert from "node:assert"; | ||
assert(value, "This should be transformed"); | ||
assert.strictEqual(a, b, "This should remain unchanged"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
const assert = require("node:assert"); | ||
assert(value, "This should be transformed"); | ||
assert.strictEqual(a, b, "This should remain unchanged"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
const config = { | ||
port: 3000, | ||
host: "localhost" | ||
}; | ||
|
||
console.log("Server config:", config); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import assert from "node:assert"; | ||
function testFunction() { | ||
assert(condition, "Assertion inside function"); | ||
|
||
if (someCondition) { | ||
assert.deepStrictEqual(obj1, obj2, "Deep comparison"); | ||
} | ||
|
||
return assert.ok(value) && true; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import assert from "node:assert"; | ||
assert(condition); | ||
assert.ok(value); | ||
assert.strictEqual(a, b); | ||
assert.notStrictEqual(a, c); | ||
assert.throws(() => { throw new Error(); }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
const assert = require("node:assert"); | ||
const fs = require("fs"); | ||
|
||
function readConfig(path) { | ||
assert(fs.existsSync(path), "Config file must exist"); | ||
const data = fs.readFileSync(path, "utf8"); | ||
assert.ok(data.length > 0, "Config file cannot be empty"); | ||
return JSON.parse(data); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import assert from "node:assert"; | ||
class Validator { | ||
static validate(data) { | ||
assert(data, "Data is required"); | ||
|
||
try { | ||
assert.strictEqual(typeof data, "object", "Data must be object"); | ||
} catch (error) { | ||
assert.fail("Validation failed"); | ||
} | ||
|
||
const results = [1, 2, 3].map(item => { | ||
assert.ok(item > 0, "Item must be positive"); | ||
return item * 2; | ||
}); | ||
|
||
return results; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
process.assert(condition, "Basic assertion"); | ||
process.assert.strictEqual(a, b, "Values should be equal"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
process.assert(condition, "Basic assertion"); | ||
process.assert.strictEqual(a, b, "Values should be equal"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ps: verify the link I am on mobile for this review