feat(crypto-fips): add migration recipe for crypto.fips #177
Conversation
….fips to crypto.getFips() and crypto.setFips()
|
Let me know if the Personally, I think having that helps |
You don't have to remove it entirely, but can it be a separate PR (to keep our commits organized)? Thank you! |
Co-authored-by: Aviv Keller <[email protected]>
|
@avivkeller Alright then. I'm gonna make another PR for another recipe where I'll include this change as a separate commit |
…ove code readability
Replaces array concatenation with generator-based iteration to improve readability, memory usage, and performance.
|
Thanks @AugustinMauroy! I'll update the suggested changes |
…tFips/setFips This update introduces a transformation for replacing deprecated `crypto.fips` calls with `crypto.getFips()` and `crypto.setFips()`. It includes enhancements to variable collection, import specifier updates, and destructuring adjustments to ensure proper usage of the new methods. Additionally, new test cases have been added to validate the changes.
|
@AugustinMauroy I think some functions I used can be put into |
|
For the utility functions thing, I'll examine those and probably open another PR for that specifically |
Co-authored-by: Augustin Mauroy <[email protected]>
| function updateCryptoImportSpecifiers(root: SgRoot): Edit[] { | ||
| const edits: Edit[] = []; | ||
|
|
||
| const importStmts = getNodeImportStatements(root, 'crypto'); | ||
|
|
||
| for (const stmt of importStmts) { | ||
| // import_clause contains default/namespace/named parts | ||
| const importClause = stmt.find({ rule: { kind: 'import_clause' } }); | ||
| if (!importClause) continue; | ||
|
|
||
| // named_imports = `{ ... }` | ||
| const namedImports = importClause.find({ rule: { kind: 'named_imports' } }); | ||
| if (!namedImports) continue; // nothing to edit if there is no `{ ... }` | ||
|
|
||
| // All specifiers inside `{ ... }` | ||
| const specifiers = namedImports.findAll({ | ||
| rule: { kind: 'import_specifier' }, | ||
| }); | ||
| if (!specifiers || specifiers.length === 0) continue; | ||
|
|
||
| let hasFips = false; | ||
| let hasGet = false; | ||
| let hasSet = false; | ||
|
|
||
| const keepTexts: string[] = []; | ||
|
|
||
| for (const spec of specifiers) { | ||
| // imported name is in field "name" | ||
| const importedNameNode = spec.find({ | ||
| rule: { has: { field: 'name', kind: 'identifier' } }, | ||
| }); | ||
| const importedName = importedNameNode | ||
| ?.find({ | ||
| rule: { kind: 'identifier' }, | ||
| }) | ||
| ?.text(); | ||
|
|
||
| if (importedName === 'fips') { | ||
| hasFips = true; // drop this one; we will add getFips/setFips instead | ||
| continue; | ||
| } | ||
| if (importedName === 'getFips') hasGet = true; | ||
| if (importedName === 'setFips') hasSet = true; | ||
|
|
||
| // Preserve other specifiers as-is (including aliases like `name as alias`) | ||
| keepTexts.push(spec.text()); | ||
| } | ||
|
|
||
| if (!hasFips) continue; // only rewrite when file was importing `fips` | ||
|
|
||
| // Ensure both getFips and setFips are present | ||
| if (!hasGet) keepTexts.push('getFips'); | ||
| if (!hasSet) keepTexts.push('setFips'); | ||
|
|
||
| // Replace the whole `{ ... }` | ||
| edits.push(namedImports.replace(`{ ${keepTexts.join(', ')} }`)); | ||
| } | ||
|
|
||
| return edits; | ||
| } |
There was a problem hiding this comment.
We have an utility function that removes the imported binding (utils/src/ast-grep/remove-binding).
I think it could be useful to reduce the number of lines of code. Could you please have a look at that?
There was a problem hiding this comment.
Will try to use the removeBinding function although I am facing some issues with named imports and renamed requires. It renames the alias after as or : instead of removing the entire thing and adding new imports for getFips and setFips
There was a problem hiding this comment.
Hmm, I created a new updateBinding utility function that handles both updates and removals. I also made the old removeBinding a wrapper around it. This new implementation might handle the scenario better, but it isn’t merged yet (#182).
If you want, you can copy and paste it to test whether it solves your scenario as well, and use it until the other branch is merged.
| function updateCryptoRequireDestructuring(root: SgRoot): Edit[] { | ||
| const edits: Edit[] = []; | ||
|
|
||
| const decls = getNodeRequireCalls(root, 'crypto'); | ||
|
|
||
| for (const decl of decls) { | ||
| const objPattern = decl.find({ rule: { kind: 'object_pattern' } }); | ||
| if (!objPattern) continue; | ||
|
|
||
| const props = objPattern.findAll({ | ||
| rule: { | ||
| any: [ | ||
| { kind: 'shorthand_property_identifier_pattern' }, // `{ foo }` | ||
| { kind: 'pair_pattern' }, // `{ foo: bar }` | ||
| ], | ||
| }, | ||
| }); | ||
| if (!props || props.length === 0) continue; | ||
|
|
||
| let hasFips = false; | ||
| let hasGet = false; | ||
| let hasSet = false; | ||
|
|
||
| const keepTexts: string[] = []; | ||
|
|
||
| for (const p of props) { | ||
| if (p.kind() === 'shorthand_property_identifier_pattern') { | ||
| const name = p.text().trim(); | ||
| if (name === 'fips') { | ||
| hasFips = true; | ||
| continue; | ||
| } | ||
| if (name === 'getFips') hasGet = true; | ||
| if (name === 'setFips') hasSet = true; | ||
| keepTexts.push(name); | ||
| } else { | ||
| // pair_pattern: has key + value (e.g. `fips: myFips`, `getFips: gf`) | ||
| const keyNode = p.find({ rule: { kind: 'property_identifier' } }); | ||
| const key = keyNode?.text(); | ||
|
|
||
| if (key === 'fips') { | ||
| hasFips = true; // drop any alias of fips | ||
| continue; | ||
| } | ||
| if (key === 'getFips') hasGet = true; | ||
| if (key === 'setFips') hasSet = true; | ||
|
|
||
| // Keep other pairs as-is (preserves aliasing/spacing nicely) | ||
| keepTexts.push(p.text().trim()); | ||
| } | ||
| } | ||
|
|
||
| if (!hasFips) continue; // only rewrite when it actually destructured `fips` | ||
|
|
||
| if (!hasGet) keepTexts.push('getFips'); | ||
| if (!hasSet) keepTexts.push('setFips'); | ||
|
|
||
| edits.push(objPattern.replace(`{ ${keepTexts.join(', ')} }`)); | ||
| } | ||
|
|
||
| return edits; | ||
| } |
Co-authored-by: Bruno Rodrigues <[email protected]>
Closes #175