Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion recipes/fs-access-mode-constants/codemod.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
schema_version: "1.0"
name: "@nodejs/fs-access-mode-constants"
version: "1.0.2"
version: "1.0.3"
description: Handle DEP0176 via transforming imports of `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` from the root `fs` module to `fs.constants`.
author: nekojanai (Jana)
license: MIT
Expand Down
2 changes: 1 addition & 1 deletion recipes/fs-access-mode-constants/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@nodejs/fs-access-mode-constants",
"version": "1.0.1",
"version": "1.0.3",
"description": "Handle DEP0176 via transforming imports of `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` from the root `fs` module to `fs.constants`.",
"type": "module",
"scripts": {
Expand Down
297 changes: 241 additions & 56 deletions recipes/fs-access-mode-constants/src/workflow.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The utilities here could use some unit tests 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand what is unclear 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean that you wan unit test on function written inside the codemod ?
that need to be done as this:

  • move them to utility package
  • add tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to extract them into a package? Just co-locate a .test.ts file here and cover the utils, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah we can do that I'm also going to see if we can reduce amount of code there

Original file line number Diff line number Diff line change
@@ -1,79 +1,264 @@
import { getNodeRequireCalls } from '@nodejs/codemod-utils/ast-grep/require-call';
import { getNodeImportStatements } from '@nodejs/codemod-utils/ast-grep/import-statement';
import type { Edit, SgRoot } from '@codemod.com/jssg-types/main';
import { resolveBindingPath } from '@nodejs/codemod-utils/ast-grep/resolve-binding-path';
import { getModuleDependencies } from '@nodejs/codemod-utils/ast-grep/module-dependencies';
import { updateBinding } from '@nodejs/codemod-utils/ast-grep/update-binding';
import type { Edit, SgNode, SgRoot } from '@codemod.com/jssg-types/main';
import type Js from '@codemod.com/jssg-types/langs/javascript';

const patterns = ['F_OK', 'R_OK', 'W_OK', 'X_OK'];
const PATTERN_SET = ['F_OK', 'R_OK', 'W_OK', 'X_OK'];

export default function tranform(root: SgRoot<Js>): string | null {
type BindingMapping = {
local: string;
replacement: string;
};

type RemovedBinding = {
imported: string;
local: string;
};

export default function transform(root: SgRoot<Js>): string | null {
const rootNode = root.root();
const edits: Edit[] = [];
const localBindings = new Map<string, string>();
const namespaceBindings = new Map<string, string>();

const requireStatements = getNodeRequireCalls(root, 'fs');
const depStatements = getModuleDependencies(root, 'fs');

for (const statement of requireStatements) {
const objectPattern = statement.find({
rule: { kind: 'object_pattern' },
});
if (!depStatements) return null;

for (const statement of depStatements) {
const promisesBinding = resolveBindingPath(statement, '$.promises');
const rewritten = rewriteBindings(statement, promisesBinding);
edits.push(...rewritten.edits);

for (const mapping of rewritten.mappings) {
localBindings.set(mapping.local, mapping.replacement);
}

for (const pattern of PATTERN_SET) {
const resolved = resolveBindingPath(statement, `$.${pattern}`);
if (!resolved?.includes('.') || resolved.includes('.constants.')) {
continue;
}

if (objectPattern) {
let objPatArr = objectPattern
.findAll({
rule: { kind: 'shorthand_property_identifier_pattern' },
})
.map((v) => v.text());
objPatArr = objPatArr.filter((v) => !patterns.includes(v));
objPatArr.push('constants');
edits.push(objectPattern.replace(`{ ${objPatArr.join(', ')} }`));
namespaceBindings.set(
resolved,
resolved.replace(`.${pattern}`, `.constants.${pattern}`),
);
}
}

const importStatements = getNodeImportStatements(root, 'fs');
let promisesImportName = '';
applyNamespaceReplacements(rootNode, edits, namespaceBindings);
applyLocalReplacements(rootNode, edits, localBindings);

if (!edits.length) return null;

return rootNode.commitEdits(edits);
}

function rewriteBindings(
statement: SgNode<Js>,
promisesBinding: string,
): { edits: Edit[]; mappings: BindingMapping[] } {
const objectPattern = statement.find({
rule: { kind: 'object_pattern' },
});

if (objectPattern) {
return rewriteObjectPattern(statement, objectPattern, promisesBinding);
}

const namedImports = statement.find({
rule: { kind: 'named_imports' },
});

if (namedImports) {
return rewriteNamedImports(statement, namedImports, promisesBinding);
}

return { edits: [], mappings: [] };
}

function rewriteObjectPattern(
statement: SgNode<Js>,
pattern: SgNode<Js>,
promisesBinding: string,
): { edits: Edit[]; mappings: BindingMapping[] } {
const kept: string[] = [];
const removed: RemovedBinding[] = [];

for (const statement of importStatements) {
const objectPattern = statement.find({
rule: { kind: 'named_imports' },
const shorthandBindings = pattern
.findAll({
rule: { kind: 'shorthand_property_identifier_pattern' },
})
.map((node) => node.text());
const aliasedBindings = pattern
.findAll({
rule: {
kind: 'pair_pattern',
has: {
field: 'key',
kind: 'property_identifier',
},
},
})
.map((pair) => {
const imported = pair.field('key')?.text() ?? '';
const local = pair.field('value')?.text() ?? imported;

return {
imported,
local,
text: pair.text(),
};
});

if (objectPattern) {
let objPatArr = objectPattern
.findAll({
rule: { kind: 'import_specifier' },
})
.map((v) => v.text());
objPatArr = objPatArr.filter((v) => !patterns.includes(v));
const promisesImport = objPatArr.find((v) => v.startsWith('promises'));
if (promisesImport) {
if (promisesImport.includes('as')) {
const m = promisesImport.matchAll(/promises as (\w+)/g);
m.forEach((v) => {
promisesImportName = v[1] ?? 'promises';
});
} else {
promisesImportName = promisesImport;
}
promisesImportName = `${promisesImportName}.`;
} else {
objPatArr.push('constants');
}
edits.push(objectPattern.replace(`{ ${objPatArr.join(', ')} }`));
for (const name of shorthandBindings) {
if (PATTERN_SET.includes(name)) {
removed.push({
imported: name,
local: name,
});
} else {
kept.push(name);
}
}

for (const binding of aliasedBindings) {
if (PATTERN_SET.includes(binding.imported)) {
removed.push({
imported: binding.imported,
local: binding.local,
});
} else {
kept.push(binding.text);
}
}

for (const _OK of patterns) {
for (const [prefix, replacement] of [
['fs.', 'fs.constants.'],
['', `${promisesImportName ? promisesImportName : ''}constants.`],
]) {
const patterns = rootNode.findAll({
rule: { pattern: `${prefix}${_OK}` },
return rewriteCollectedBindings({
statement,
pattern,
promisesBinding,
kept,
removed,
});
}

function rewriteNamedImports(
statement: SgNode<Js>,
pattern: SgNode<Js>,
promisesBinding: string,
): { edits: Edit[]; mappings: BindingMapping[] } {
const kept: string[] = [];
const removed: RemovedBinding[] = [];

const specifiers = pattern.findAll({
rule: { kind: 'import_specifier' },
});

for (const specifier of specifiers) {
const imported = specifier.field('name')?.text() ?? '';
const local = specifier.field('alias')?.text() ?? imported;

if (PATTERN_SET.includes(imported)) {
removed.push({
imported,
local,
});
for (const pattern of patterns) {
edits.push(pattern.replace(`${replacement}${_OK}`));
} else {
kept.push(specifier.text());
}
}

return rewriteCollectedBindings({
statement,
pattern,
promisesBinding,
kept,
removed,
});
}

function applyNamespaceReplacements(
rootNode: SgNode<Js>,
edits: Edit[],
replacements: Map<string, string>,
): void {
for (const [path, replacement] of replacements) {
const nodes = rootNode.findAll({ rule: { pattern: path } });

for (const node of nodes) {
edits.push(node.replace(replacement));
}
}
}

function applyLocalReplacements(
rootNode: SgNode<Js>,
edits: Edit[],
replacements: Map<string, string>,
): void {
for (const [local, replacement] of replacements) {
const identifiers = rootNode.findAll({
rule: {
kind: 'identifier',
regex: `^${escapeRegExp(local)}$`,
},
});

for (const identifier of identifiers) {
if (
!identifier.inside({ rule: { kind: 'named_imports' } }) ||
!identifier.inside({ rule: { kind: 'object_pattern' } })
) {
edits.push(identifier.replace(replacement));
}
}
}
}

return rootNode.commitEdits(edits);
function rewriteCollectedBindings({
statement,
pattern,
promisesBinding,
kept,
removed,
}: {
statement: SgNode<Js>;
pattern: SgNode<Js>;
promisesBinding: string;
kept: string[];
removed: RemovedBinding[];
}): { edits: Edit[]; mappings: BindingMapping[] } {
if (!removed.length) return { edits: [], mappings: [] };

const replacementPrefix = promisesBinding
? `${promisesBinding}.constants`
: 'constants';
const mappings = removed.map((binding) => ({
local: binding.local,
replacement: `${replacementPrefix}.${binding.imported}`,
}));

const shouldAddConstants = !promisesBinding && !kept.includes('constants');

if (removed.length === 1) {
const singleBindingEdit = updateBinding(statement, {
old: removed[0].imported,
new: shouldAddConstants ? 'constants' : undefined,
}).edit;

if (singleBindingEdit) return { edits: [singleBindingEdit], mappings };
}

if (shouldAddConstants) kept.push('constants');

return {
edits: [pattern.replace(`{ ${kept.join(', ')} }`)],
mappings,
};
}

function escapeRegExp(text: string): string {
return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { access, constants } from 'node:fs';

access('/path/to/file', constants.F_OK | constants.X_OK, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { access, constants } = require('node:fs');

access('/path/to/file', constants.F_OK | constants.R_OK, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { access, constants } from 'node:fs';

access('/path/to/file', constants.F_OK, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import * as fs from 'node:fs';

fs.access('/path/to/file', fs.constants.F_OK, callback);
fs.access('/path/to/file', fs.constants.R_OK | fs.constants.W_OK, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const fs = require('node:fs');

fs.accessSync('/path/to/file', fs.constants.X_OK);
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const { constants } = require('node:fs');
const { F_OK, R_OK } = constants;

console.log(F_OK, R_OK);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { F_OK } from './local-constants.js';

console.log(F_OK);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { F_OK } = require('./local-constants.cjs');

console.log(F_OK);
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { readFileSync } from 'node:fs';

const F_OK = 1;
console.log(readFileSync, F_OK);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { access, F_OK as fileExists, X_OK as canExec } from 'node:fs';

access('/path/to/file', fileExists | canExec, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { access, F_OK: fileExists, R_OK: canRead } = require('node:fs');

access('/path/to/file', fileExists | canRead, callback);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { access, constants } from 'node:fs';

access('/path/to/file', constants.F_OK, callback);
Loading
Loading