Skip to content

feat(napi/oxlint): add getFilename + getPhysicalFilename methods to Context #12481

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.

Sign up for GitHub

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

Closed
overlookmotel wants to merge 1 commit into from
Closed

feat(napi/oxlint): add getFilename + getPhysicalFilename methods to Context #12481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions napi/oxlint2/src-js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,24 @@ class Context {
});
}

/**
* Get the absolute path of the file being linted.
* @deprecated Use `filename` instead.
Copy link
Contributor

@camc314 camc314 Jul 23, 2025

Choose a reason for hiding this comment

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

i don't think we should implement deprecated APIs, just for the sake of filling out the context object

Copy link
Member Author

@overlookmotel overlookmotel Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I figured these methods are part of ESLint's API, so we should include them, even though they're deprecated. An older ESLint plugin might otherwise work quite happily with Oxlint, but it'd break if it uses these APIs.

Copy link
Contributor

@camc314 camc314 Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Lets chat in a bit? I don't quite agree, especially since we don't have full compat with eslint, i don't think we should fill out deprecated APIs, we shuold instead push plugin maintainers to use the new APIs.

More of my reasoning is adding something deprecated, means that we definitly will remove it at some point which means a breaking change ect

Copy link
Member Author

@overlookmotel overlookmotel Jul 24, 2025

Choose a reason for hiding this comment

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

OK, we disagree here. I think we're aiming for ESLint compat, so we need to provide all ESLint's APIs, regardless of whether those APIs are deprecated or not.

But it's not important (yet). Feel free to close this if you disagree and we can always return to it later once we've confirmed that we are definitely going for ESLint compat.

Copy link
Contributor

@camc314 camc314 Jul 24, 2025

Choose a reason for hiding this comment

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

yeah sorry to be a pain. I think let's close for now and return once it's confirmed.

* @returns {string} - The absolute path of the file being linted.
*/
getFilename() {
return this.filename;
}

/**
* Get the physical filename (absolute path) of the file being linted.
* @deprecated Use `physicalFilename` instead.
* @returns {string} - The physical filename of the file being linted.
*/
getPhysicalFilename() {
return this.physicalFilename;
}

static {
/**
* Update a `Context` with file-specific data.
Expand Down
26 changes: 25 additions & 1 deletion napi/oxlint2/test/__snapshots__/e2e.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,18 @@ exports[`oxlint2 CLI > should receive data via \`context\` 1`] = `
: ^
\`----

x context-plugin(log-context): getFilename(): files/1.js
,-[files/1.js:1:1]
1 | let x;
: ^
\`----

x context-plugin(log-context): getPhysicalFilename(): files/1.js
,-[files/1.js:1:1]
1 | let x;
: ^
\`----

x context-plugin(log-context): id: context-plugin/log-context
,-[files/2.js:1:1]
1 | let y;
Expand All @@ -360,7 +372,19 @@ exports[`oxlint2 CLI > should receive data via \`context\` 1`] = `
: ^
\`----

Found 0 warnings and 6 errors.
x context-plugin(log-context): getFilename(): files/2.js
,-[files/2.js:1:1]
1 | let y;
: ^
\`----

x context-plugin(log-context): getPhysicalFilename(): files/2.js
,-[files/2.js:1:1]
1 | let y;
: ^
\`----

Found 0 warnings and 10 errors.
Finished in Xms on 2 files using X threads."
`;

Expand Down
10 changes: 10 additions & 0 deletions napi/oxlint2/test/fixtures/context_properties/test_plugin/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ export default {
node: SPAN,
});

context.report({
message: `getFilename(): ${relativePath(context.getFilename())}`,
node: SPAN,
});

context.report({
message: `getPhysicalFilename(): ${relativePath(context.getPhysicalFilename())}`,
node: SPAN,
});

return {};
},
},
Expand Down
Loading