Repository navigation
Fix line numbering for non-standard line separators - #52
Merged
Merged
Conversation
str.splitlines() also breaks on form feed, vertical tab, \x1c-\x1e, \x85, U+2028 and U+2029, which ast and tree-sitter do not count as line ends. In a file containing one of them, every later finding was fingerprinted from the wrong line, inline suppressions after it attached to the wrong line, and LSP ranges were off. Add engine.text.split_lines and use it wherever source is indexed by line number: fingerprints, RuleContext.lines, dataflow snippets, suppression parsing, CG-SEC-009's import insertion and the LSP LineIndex (which now also handles CR-only documents).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Source code is now split into lines using only LF (
\n), CRLF (\r\n), and CR (\r) as line separators, matching how Python'sastmodule and tree-sitter number lines. Previously, other Unicode line breaks (form feed\x0c, vertical tab\x0b,\x85, U+2028, U+2029, etc.) were treated as line separators, causing all subsequent line numbers to be shifted. This led to incorrect fingerprints, failed suppression comments, and misaligned editor positions for findings after such characters.Type of change
Changes
split_lines()function in newsrc/codeguard/engine/text.pythat splits source text on standard line separators onlysplit_lines()instead ofstr.splitlines():fingerprint.py: fingerprint calculationsuppressions.py: parsing inline suppression commentscontext.py: caching source lines for rule contextdataflow/python.py: taint analysislsp/positions.py: LSP position conversionrules/security/cg_sec_009_insecure_random.py: import insertion logictests/test_engine/test_fingerprint.pyand newtests/test_engine/test_text.pytests/test_engine/test_suppressions.pyandtests/test_lsp/test_positions.pyChecklist
git commit -s— DCO sign-off on all commitspytestpasses locallyruff check src/ tests/cleanmypy src/cleanchangelog.d/Test evidence
New test classes added:
TestLineSeparatorsintest_fingerprint.py: Tests that form feeds and other Unicode line breaks don't shift line numbers or affect fingerprintsTestLineSeparators.test_finding_after_form_feed_uses_its_own_line: Verifies findings after form feeds get correct line numbersTestLineSeparators.test_unicode_line_breaks_do_not_shift_lines: Parametrized test for various Unicode line breaksTestLineSeparators.test_crlf_and_cr_line_endings: Tests CRLF and CR line ending handlingtest_text.py: Comprehensive tests forsplit_lines()function matchingstr.splitlines()behavior on standard separatorstest_suppressions.py: Tests that form feeds don't affect suppression comment parsingtest_positions.py: Tests that LSP position conversion handles non-standard line breaks correctlyNotes for reviewer
The key insight is that Python's
astmodule and tree-sitter only count LF, CRLF, and CR as line separators when numbering lines. Usingstr.splitlines()was incorrect because it treats many other Unicode characters as line breaks. The newsplit_lines()function ensures consistent line numbering across fingerprinting, suppression parsing, and position conversion.https://claude.ai/code/session_018woih8gNc6h8GSJVjbAixU