Skip to content
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

feat: add transform for v9 migration #29

Merged
merged 8 commits into from
Jun 6, 2024
Merged

feat: add transform for v9 migration #29

merged 8 commits into from
Jun 6, 2024

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented May 22, 2024

Fix #25

@snitin315 snitin315 marked this pull request as ready for review May 23, 2024 04:00
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Thanks, this looks like a great start. Some overall comments:

  1. I think we should call this v9-rule-migration to make it clear that this relates only to rules.
  2. It looks like we are missing getScope(), markVariableAsUsed(), and getAncestors()
  3. For things like sourceCode, is it possible to add a variable declaration for const sourceCode = context.sourceCode ?? context.getSourceCode() immediately inside the create() method and then replace all instances of context.getSourceCode() with that variable?

tests/lib/v9-migration/v9-migration.js Outdated Show resolved Hide resolved
@snitin315
Copy link
Contributor Author

@nzakas I've addressed your feedback. PTAL

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Looking better, I think we're close. Just a few notes.

tests/utils/index.js Outdated Show resolved Hide resolved
tests/lib/v9-rule-migration/v9-rule-migration.js Outdated Show resolved Hide resolved
tests/lib/v9-rule-migration/v9-rule-migration.js Outdated Show resolved Hide resolved
@aladdin-add aladdin-add self-requested a review June 3, 2024 09:14
nzakas
nzakas previously approved these changes Jun 3, 2024
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Nice! This looks really good.

Would like another review before merging.

const v9MigrationTransform = require("../../../lib/v9-rule-migration/v9-rule-migration");

describe("v9 migration transform", () => {
it("should migrate deprecated context methods to new properties", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

can you also add a test that the vars were defined outside (it's more commonly used to avoid repeating), something like:

module.exports = {
    create(context) {
        const sourceCode = context.getSourceCode();
        const cwd = context.getCwd();
        const filename = context.getFilename();
        const physicalFilename = context.getPhysicalFilename();
        return {
            Program(node) {
                //....
            },
        };
    }
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@aladdin-add
Copy link
Member

I've tried running it to eslint-plugin-n v15.0.0, but got the error:

node ../eslint-transforms/bin/eslint-transforms.js v9-rule-migration ./lib
Processing 67 files... 
Spawning 7 workers...
Sending 10 files to free worker...
Sending 10 files to free worker...
Sending 10 files to free worker...
Sending 10 files to free worker...
Sending 10 files to free worker...
Sending 10 files to free worker...
Sending 7 files to free worker...
 ERR lib/util/check-unsupported-builtins.js Transformation error (Cannot read properties of null (reading 'node'))
TypeError: Cannot read properties of null (reading 'node')
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:76:27)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:226:44)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/rules/no-path-concat.js Transformation error (Cannot read properties of undefined (reading 'name'))
TypeError: Cannot read properties of undefined (reading 'name')
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:228:56)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/util/visit-require.js Transformation error (Cannot read properties of undefined (reading 'name'))
TypeError: Cannot read properties of undefined (reading 'name')
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:228:56)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/rules/no-deprecated-api.js Transformation error (Cannot read properties of undefined (reading 'name'))
TypeError: Cannot read properties of undefined (reading 'name')
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:228:56)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/util/check-prefer-global.js Transformation error (Cannot read properties of null (reading 'node'))
TypeError: Cannot read properties of null (reading 'node')
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:76:27)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:226:44)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/rules/no-unsupported-features/es-syntax.js Transformation error (Cannot read properties of null (reading 'node'))
TypeError: Cannot read properties of null (reading 'node')
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:76:27)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at getParentObjectMethod (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:81:12)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:226:44)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/rules/prefer-promises/fs.js Transformation error (Cannot read properties of undefined (reading 'name'))
TypeError: Cannot read properties of undefined (reading 'name')
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:228:56)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
 ERR lib/rules/prefer-promises/dns.js Transformation error (Cannot read properties of undefined (reading 'name'))
TypeError: Cannot read properties of undefined (reading 'name')
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:228:56)
    at NodePath.<anonymous> (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:142:47)
    at /Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:75:36
    at Array.forEach (<anonymous>)
    at Collection.forEach (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:74:18)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/collections/Node.js:140:17)
    at Collection.replaceWith (/Users/weiran/repo/github/eslint-transforms/node_modules/jscodeshift/src/Collection.js:413:43)
    at module.exports (/Users/weiran/repo/github/eslint-transforms/lib/v9-rule-migration/v9-rule-migration.js:203:8)
All done. 
Results: 
8 errors
41 unmodified
0 skipped
18 ok
Time elapsed: 2.956seconds

@snitin315
Copy link
Contributor Author

I've tried running it to eslint-plugin-n v15.0.0, but got the error:

Looking into it.

@snitin315
Copy link
Contributor Author

I've tried running it to eslint-plugin-n v15.0.0, but got the error:

@aladdin-add Fixed 👍🏻, it was failing when the methods were in some helper utils instead of being directly under the create() method.

Screenshot 2024-06-06 at 6 49 04 AM

Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! 💯

@aladdin-add aladdin-add merged commit 528b94f into main Jun 6, 2024
9 checks passed
@aladdin-add aladdin-add deleted the v9-codemod branch June 6, 2024 01:43
@github-actions github-actions bot mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Codemod for v8 -> v9 rule API changes
3 participants