Skip to content

fix: undefined context.filename for inline lint #163

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: undefined context.filename for inline lint #163

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16f1a3a
fix: undefined `context.filename` for inline lint
david-vaclavek Dec 12, 2024
5e4f362
Update alias.ts
david-vaclavek Dec 13, 2024
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
7 changes: 6 additions & 1 deletion src/rules/alias.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ const rule = {
);
if (!isPathInImport) return;

const filename = context.filename;
let filename = context.filename;
if (typeof filename === 'undefined') {
Copy link
Owner

Choose a reason for hiding this comment

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

Currently this code just hides a problem for exists test cases.

Let's add tests that reproduce original problem from issue, and make sure all other tests will be run for that case too.

Copy link
Author

@david-vaclavek david-vaclavek Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@vitonsky
I'm not sure it is possible to properly cover it with the tests.

See the screenshot:

image

  1. Sample updated test.
  2. Test console.log statement.
  3. console.log output, which proves that both context.filename and context.getFilename() return the same value. If filename in test definition would be undefined, then both values are also undefined. It's not possible to pass getFilename: () => {...} function into test definition, as it's not part of the interface.

My conclusion:
Testing the behavior of context.getFilename() in ESLint can be done using the RuleTester class by passing a filename property in the test cases. It's a part of ESLint 7 usage that context.getFilename() must be used.

Reading the statement above, how exactly would you like me to cover it with tests? Could you provide an example?

const fullFilePath = context.getFilename();
const cwd = context.getCwd();
filename = path.relative(cwd, fullFilePath);
}

const resolvedIgnoredPaths = ignoredPaths.map((ignoredPath) =>
path.normalize(path.join(path.dirname(filename), ignoredPath)),
Expand Down
Loading