fix: check_code misses lines_before_imports-only changes (#2242)#2563
Open
gaoflow wants to merge 1 commit into
Open
fix: check_code misses lines_before_imports-only changes (#2242)#2563gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
isort.core.process returned False (no changes needed) when the only difference between the input and the correctly-formatted output was the number of blank lines before the import block, controlled by lines_before_imports. Two cooperating issues caused this: 1. _has_changed() used .strip() to compare the before/after import sections. This stripped any leading newlines that output.sorted_imports had prepended (via lines_before_imports), making additions invisible. Fix: use .rstrip() so leading whitespace differences are preserved. 2. When blank lines appear before the first import block and lines_before_imports >= 0, those lines are buffered in lines_before and then discarded (not written) when an import section is present. The raw_import_section passed to _has_changed did not include these consumed blank lines, so removing them was never detected as a change. Fix: capture the buffered lines in above_import_section and prepend them to raw_import_section for the comparison.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2563 +/- ##
=======================================
Coverage 99.16% 99.16%
=======================================
Files 41 41
Lines 3101 3103 +2
Branches 671 671
=======================================
+ Hits 3075 3077 +2
Misses 14 14
Partials 12 12 🚀 New features to boost your workflow:
|
DanielNoord
reviewed
Jun 28, 2026
DanielNoord
left a comment
Member
There was a problem hiding this comment.
Did you see #2541? And if so, why should we continue with this approach instead of that one?
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.
Problem
isort.check_code(andisort.core.process) returnsTrue(no changes needed) whenlines_before_importsrequires adding or removing blank lines before the import block, but the imports themselves are already correctly sorted.Reported in #2242.
Root cause
Two cooperating issues in
isort/core.py:1.
_has_changed()used.strip()for comparison — this stripped leading newlines thatoutput.sorted_importshad prepended (vialines_before_imports), making additions invisible:2. Blank lines consumed before the import block were excluded from the comparison — when
lines_before_imports >= 0, blank lines preceding the import block are buffered inlines_beforeand then discarded (not written to the output stream). Theraw_import_sectionpassed to_has_changednever included those consumed lines, so removing them was also never detected as a change.Fix
_has_changedto use.rstrip()instead of.strip(), preserving leading whitespace differences while still ignoring insignificant trailing newlines.above_import_sectionand prepend them toraw_import_sectionbefore the comparison, so removed blank lines are detected.Tests
Added
test_check_code_detects_lines_before_import_changescovering:Note: PR #2541 addresses the same issue with only the
.rstrip()fix, which handles the "adding blank lines" case. This PR also fixes the "removing blank lines" case via theabove_import_sectionaddition, and includes the requested test.This pull request was prepared with the assistance of AI, under my direction and review.