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

Lexer should consider BOM for the start offset #11732

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

dhruvmanila
Copy link
Member

Summary

This PR fixes a bug where the lexer didn't consider the BOM into the start offset.

fixes: #11731

Test Plan

Add multiple test cases which involves BOM character in the source for the lexer and verify the snapshot.

@dhruvmanila dhruvmanila added the bug Something isn't working label Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

crates/ruff_python_parser/src/lexer.rs Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila enabled auto-merge (squash) June 4, 2024 08:42
@dhruvmanila dhruvmanila merged commit 2567e14 into main Jun 4, 2024
15 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/lexer-bom-offset branch June 4, 2024 08:45
carljm added a commit that referenced this pull request Jun 4, 2024
* main:
  CI: add job to run tests under minimum supported rust version (msrv) (#11737)
  Lexer should consider BOM for the start offset (#11732)
  Use cursor offset for lexer checkpoint (#11734)
  red-knot: Change `resolve_global_symbol` to take `Module` as an argument (#11723)
  red-knot: Use `parse_unchecked` to get all parse errors (#11725)
  Respect per-file ignores for blanket and redirected noqa rules (#11728)
  [`pygrep_hooks`] Check blanket ignores via file-level pragmas (`PGH004`) (#11540)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule ANN201 cause panic
2 participants