Skip to content

Conversation

@menace31
Copy link

This PR introduces a new codemod to migrate the deprecated tls.SecurePair class to tls.TLSSocket.

Key features of this codemod:

- Constructor Transformation: Replaces new SecurePair() with new TLSSocket(socket).

- Smart Renaming: deep-renames variables (e.g., pair → socket, myPair → mySocket) while preserving CamelCase and scope integrity.

- Import Handling: Supports both CommonJS (require) and ESM (import) patterns, including destructured imports.

- Cleanup: Removes usage of deprecated properties like .cleartext and .encrypted.

- Safety: Adds a comment warning users that manual verification is required for the injected socket argument.

Testing: I have added comprehensive test cases covering:

- CommonJS and ESM import styles.

- Variable renaming in complex scopes (nested blocks).

- Destructuring patterns.

Tests were verified locally using a custom test script ensuring input/output matches expectations.

Copy link
Member

Choose a reason for hiding this comment

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

there are some copy pasta

@AugustinMauroy
Copy link
Member

Pr title should be updated + the architecture need to fit with the good way to create a codemod

@JakobJingleheimer JakobJingleheimer added the awaiting author Reviewer has requested something from the author label Dec 1, 2025
@menace31
Copy link
Author

menace31 commented Dec 5, 2025

@AugustinMauroy I have addressed the comments. Could you please review the code again? If the architecture is still not correct, could you please clarify the expected approach so I can be sure I understand?

@AugustinMauroy AugustinMauroy changed the title Maxime d feat(tls-securepair-to-tlssocket): introduce Dec 5, 2025
- Supports both `new tls.SecurePair()` and `new SecurePair()`.

## Examples

Copy link
Member

Choose a reason for hiding this comment

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

you can have on code snippets here that use diff

Copy link
Member

Choose a reason for hiding this comment

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

this file isn't needed codemod cli furnish us the needed tooling

const edits: Edit[] = [];
const linesToRemove: Range[] = [];

const importNodes = rootNode.findAll({
Copy link
Member

Choose a reason for hiding this comment

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

Maxime Devillet and others added 8 commits December 5, 2025 20:11
Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>
Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>
Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>
Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>
@menace31
Copy link
Author

I've addressed the previous PR issues. The transformation now uses the recommended utilities from @nodejs/codemod-utils for detecting and manipulating imports/requires (getNodeImportStatements, getNodeRequireCalls, updateBinding, removeLines). I've also removed test.sh from Git, updated the documentation to reference npm test, and all eight tests pass. I hope my work will pass the tests this time.

@menace31
Copy link
Author

I just corrected the codemod error and ran all tests with npm run test. It should work correctly this time. Thank you for your patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author Reviewer has requested something from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants