Skip to content

Conversation

@mdjermanovic
Copy link
Member

The processor doesn't account for BOM, which would be passed in by ESLint if the file has it.

This causes incorrect adjustments of locations and fix ranges.

Repro: https://github.com/mdjermanovic/eslint-markdown-bom

In the above repro, space-in-parens rule fixes ( a) to ( ) instead of (a).

This PR updates preprocess() to remove BOM from the text it operates on.

Comment on lines 838 to 844
it("should translate indented column numbers", () => {
const result = processor.postprocess(messages);

assert.strictEqual(result[2].column, 9);
assert.strictEqual(result[3].column, 4);
assert.strictEqual(result[4].column, 2);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only test that was failing with BOM before the code change.

Comment on lines 846 to 851
it("should adjust fix range properties", () => {
const result = processor.postprocess(messages);

assert(result[2].fix.range, [185, 185]);
assert(result[4].fix.range, [264, 265]);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how, but this test was passing with BOM before the code change.

Copy link
Member Author

@mdjermanovic mdjermanovic Sep 13, 2024

Choose a reason for hiding this comment

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

Oh, this test was missing .deepStrictEqual after assert.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, in 68aede7 I fixed this test and added another one as per the repro from the original post. Both would fail without the code change.

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.

LGTM. Thanks!

@nzakas nzakas merged commit 01bceae into main Sep 16, 2024
@nzakas nzakas deleted the fix-bom branch September 16, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants