Conversation
Contributor
|
@Copilot Thank you for the contribution! Could you fix the following issue(s)? ⚠ Invalid PR templateThis PR does not appear to have been filed using the MLflow PR template. Please copy the PR template from here and fill it out. |
…sses Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
…end positions Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copilot
AI
changed the title
The
Add end position tracking to Location and Noqa classes in clint linter
Nov 3, 2025
Location class in dev/clint/src/clint/linter.py currently does not capture the end position of a node, which can be useful for more precise error reporting and code analysis. To enhance this class, add end_lineno (type: int | None, default:...
harupy
reviewed
Nov 3, 2025
…s for offset calculation Updated the __add__ method to calculate end positions by adding self.end_lineno/end_col_offset with other.lineno/col_offset (the offset) rather than adding two end positions together. This maintains the span of the location while shifting it by the offset. Also updated all test assertions to compare only (lineno, col_offset) tuples instead of full Location objects, since Location now includes end position attributes that tests don't need to verify. Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Add __eq__ method to Location class that compares only start positions (lineno and col_offset) for backward compatibility. This enables cleaner test assertions using direct Location object comparison instead of tuple comparison. Revert 39 test files in dev/clint/tests/rules/ to use the cleaner syntax: - Before: assert (loc.lineno, loc.col_offset) == (2, 0) - After: assert loc == Location(2, 0) This improves code readability while maintaining full backward compatibility with the existing end position functionality added in previous commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Introduce Offset class to represent a position (lineno, col_offset) and refactor Location to use it, reducing from 4 fields to 2: - Before: lineno, col_offset, end_lineno, end_col_offset - After: start: Offset, end: Offset | None Key changes: - Add Offset dataclass with lineno and col_offset fields - Update Location to use start and end Offset objects - Add backward compatibility properties for lineno, col_offset, end_lineno, and end_col_offset - Update all Location constructors throughout codebase to use Location(Offset(...)) syntax - Fix code that was modifying Location properties to use immutable operations instead Benefits: - Cleaner structure with semantic grouping of start/end positions - Better type safety (can't have partial end positions) - More maintainable code with 2 fields instead of 4 - Full backward compatibility through properties 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
The Location class no longer needs the lineno, col_offset, end_lineno, and end_col_offset properties. Use direct Offset field access instead for more explicit code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Position is a more accurate name than Offset as it represents an absolute position in source code, not a displacement from something. Also refactored the Noqa class to use Position for start and end, matching the Location class structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Range is a more accurate and industry-standard name (used in LSP, VSCode API) for representing a range from start to end position in source code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Renamed Violation.loc and CodeBlock.loc to .range for consistency with the Range class name. Updated all usages in tests and source code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Replaced the __add__ method with a more explicit shift(Position) method for clarity. Changed the offset parameter in Linter from Range to Position since we only use the start position for shifting. Examples: - Before: range + Range(Position(1, 1)) - After: range.shift(Position(1, 1)) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Renamed Violation.range and CodeBlock.range to .rng to avoid shadowing Python's built-in range() function. This prevents potential issues when using range() in the same scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Renamed Position.lineno to .line and Position.col_offset to .char. The new names are clearer and more intuitive: - 'line' is simpler than 'lineno' - 'char' accurately describes character position (not an offset) Updated all usages in source and backward compatibility properties. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Renamed 'char' to 'col' for better clarity - 'col' is the standard abbreviation for column in editors and aligns with common terminology. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
- Removed lineno, col_offset, end_lineno, and end_col_offset properties from Noqa class - Updated Range.from_noqa() to access Position fields directly via noqa.start and noqa.end - All tests pass (140 passing) - No mypy type errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
ff5f528 to
96bbf17
Compare
96bbf17 to
83bcc26
Compare
Introduce Position and Range dataclasses to replace scattered lineno/col_offset tracking and the old 4-field Location class. The Range class uses lowercase `range` as the variable name throughout (e.g., `range: Range`), which avoids shadowing the built-in range() function while maintaining clean, readable code. Key changes: - Position dataclass with `line` and `col` fields - Range dataclass with `start: Position` and `end: Position | None` - Range.shift(Position) method for explicit position shifting - Consistent use of `range` as variable/field name 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
83bcc26 to
123e583
Compare
harupy
approved these changes
Nov 5, 2025
Convert Range from dataclass to regular class with __init__ that defaults end to start when not provided, simplifying conditional checks throughout the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
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
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.
What changes are proposed in this pull request?
Refactor the clint linter's position tracking system with cleaner, more intuitive naming and better structure.
Key changes:
Positiondataclass withlineandcolfields (replaces scattered lineno/col_offset)Rangedataclass withstart: Positionandend: Position | None(replaces 4-field Location)Range.shift(Position)method for explicit position shiftingrangeas variable/field name throughout (e.g.,range: Range,code_block_range)Noqaclass to use Position with backward compatibilityBenefits:
line/colvslineno/col_offset,RangevsLocationrangevariable doesn't shadow Python's built-inrange()functionrange: Rangereads perfectly and is intuitiveshift()is clearer than__add__()How is this PR tested?
Does this PR change the documentation?
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com