[isort] Exclude pragma comments from line length calculation (I001)

crates/ruff_linter/resources/test/fixtures/isort/fit_line_length_pragma.py

@@ -0,0 +1,12 @@
+# The next import fits on one line once the pragma comment is excluded from the width;
+# it should not be wrapped (the `# noqa` must stay effective).
+from aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa import x  # noqa: TID251
+# The next import exceeds the line length even without the pragma comment;
+# it must still be wrapped.
+from bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb import x  # noqa: TID251
+
+
+def f():
+    # The next import fits on one line once the pragma comment is excluded from the width.
+    from cccccccccccccccccccccccccccccc.ccccccccccccccccccccccccccccccccccccc import bar  # noqa: PLC0415
+    bar()

crates/ruff_linter/src/line_width.rs

@@ -9,9 +9,34 @@ use unicode_width::UnicodeWidthChar;
 
 use ruff_cache::{CacheKey, CacheKeyHasher};
 use ruff_macros::CacheKey;
-use ruff_python_trivia::tab_offset;
+use ruff_python_trivia::{find_trailing_pragma_offset, is_pragma_comment, tab_offset};
 use ruff_text_size::TextSize;
 
+use crate::preview::is_trailing_pragma_in_line_length_enabled;
+use crate::settings::types::PreviewMode;
+
+/// Returns the offset within `comment` at which the pragma comment excluded from line-length
+/// measurement begins, or `None` if the comment contains no such pragma.
+///
+/// This is the shared policy for how pragma comments (e.g., `# noqa: F401` or `# type: ignore`)
+/// are excluded when measuring line width, used by `line-too-long` (E501),
+/// `doc-line-too-long` (W505), and isort's (I001) decision of whether an import fits on one
+/// line. The formatter applies the equivalent policy when measuring comment widths.
+///
+/// In stable mode, only comments that are pragmas in their entirety are excluded (the returned
+/// offset is `0`). In preview mode, a trailing pragma within a mixed comment (e.g.,
+/// `# explanation  # noqa: F401`) is also excluded, in which case the offset points at the `#`
+/// that begins the pragma.
+pub(crate) fn pragma_offset_for_line_length(comment: &str, preview: PreviewMode) -> Option<usize> {
+    if is_trailing_pragma_in_line_length_enabled(preview) {
+        find_trailing_pragma_offset(comment)
+    } else if is_pragma_comment(comment) {
+        Some(0)
+    } else {
+        None
+    }
+}
+
 /// The length of a line of text that is considered too long.
 ///
 /// The allowed range of values is 1..=65535
@@ -242,6 +267,33 @@ impl LineWidthBuilder {
         self.column += width;
         self
     }
+
+    /// Adds the width of a trailing comment, including the standard two-space separator that
+    /// precedes it, excluding any pragma comment per [`pragma_offset_for_line_length`].
+    ///
+    /// Pragma comments are excluded so that adding one to a line never affects whether the line
+    /// is considered to fit, consistent with how `line-too-long` (E501) measures lines. For
+    /// example, counting a `# noqa` comment towards an import's width could cause isort to wrap
+    /// an import that otherwise fits on one line, moving the pragma to a position where it no
+    /// longer applies to the import statement:
+    ///
+    /// ```python
+    /// from module import (
+    ///     member,  # noqa: PLC0415
+    /// )
+    /// ```
+    #[must_use]
+    pub(crate) fn add_comment(self, comment: &str, preview: PreviewMode) -> Self {
+        let counted = match pragma_offset_for_line_length(comment, preview) {
+            Some(offset) => comment[..offset].trim_end(),
+            None => comment,
+        };
+        if counted.is_empty() {
+            self
+        } else {
+            self.add_width(2).add_str(counted)
+        }
+    }
 }
 
 impl PartialEq<LineLength> for LineWidthBuilder {

crates/ruff_linter/src/rules/isort/format.rs

@@ -1,6 +1,7 @@
 use ruff_python_codegen::Stylist;
 
 use crate::line_width::{LineLength, LineWidthBuilder};
+use crate::settings::types::PreviewMode;
 
 use super::types::{AliasData, ImportCommentSet, ImportFromCommentSet, ImportFromData, Importable};
 
@@ -54,6 +55,7 @@ pub(crate) fn format_import_from(
     force_wrap_aliases: bool,
     is_first: bool,
     trailing_comma: bool,
+    preview: PreviewMode,
 ) -> String {
     if aliases.len() == 1
         && aliases
@@ -67,6 +69,7 @@ pub(crate) fn format_import_from(
             is_first,
             stylist,
             indentation_width,
+            preview,
         );
         return single_line;
     }
@@ -95,6 +98,7 @@ pub(crate) fn format_import_from(
             is_first,
             stylist,
             indentation_width,
+            preview,
         );
         if import_width <= line_length || aliases.iter().any(|(alias, _)| alias.name == "*") {
             return single_line;
@@ -114,6 +118,7 @@ fn format_single_line(
     is_first: bool,
     stylist: &Stylist,
     indentation_width: LineWidthBuilder,
+    preview: PreviewMode,
 ) -> (String, LineWidthBuilder) {
     let mut output = String::with_capacity(CAPACITY);
     let mut line_width = indentation_width;
@@ -156,37 +161,37 @@ fn format_single_line(
         output.push(' ');
         output.push(' ');
         output.push_str(comment);
-        line_width = line_width.add_width(2).add_str(comment);
+        line_width = line_width.add_comment(comment, preview);
     }
 
     for (_, comments) in aliases {
         for comment in &comments.atop {
             output.push(' ');
             output.push(' ');
             output.push_str(comment);
-            line_width = line_width.add_width(2).add_str(comment);
+            line_width = line_width.add_comment(comment, preview);
         }
 
         for comment in &comments.inline {
             output.push(' ');
             output.push(' ');
             output.push_str(comment);
-            line_width = line_width.add_width(2).add_str(comment);
+            line_width = line_width.add_comment(comment, preview);
         }
 
         for comment in &comments.trailing {
             output.push(' ');
             output.push(' ');
             output.push_str(comment);
-            line_width = line_width.add_width(2).add_str(comment);
+            line_width = line_width.add_comment(comment, preview);
         }
     }
 
     for comment in &comments.trailing {
         output.push(' ');
         output.push(' ');
         output.push_str(comment);
-        line_width = line_width.add_width(2).add_str(comment);
+        line_width = line_width.add_comment(comment, preview);
     }
 
     output.push_str(&stylist.line_ending());

crates/ruff_linter/src/rules/isort/mod.rs

@@ -20,6 +20,7 @@ use types::{AliasData, ImportBlock, TrailingComma};
 use crate::Locator;
 use crate::line_width::{LineLength, LineWidthBuilder};
 use crate::package::PackageRoot;
+use crate::settings::types::PreviewMode;
 use ruff_python_ast::PythonVersion;
 
 mod annotate;
@@ -77,6 +78,7 @@ pub(crate) fn format_imports(
     source_type: PySourceType,
     target_version: PythonVersion,
     settings: &Settings,
+    preview: PreviewMode,
     tokens: &Tokens,
 ) -> String {
     let trailer = &block.trailer;
@@ -104,6 +106,7 @@ pub(crate) fn format_imports(
             package,
             target_version,
             settings,
+            preview,
         );
 
         if !block_output.is_empty() && !output.is_empty() {
@@ -160,6 +163,7 @@ fn format_import_block(
     package: Option<PackageRoot<'_>>,
     target_version: PythonVersion,
     settings: &Settings,
+    preview: PreviewMode,
 ) -> String {
     #[derive(Debug, Copy, Clone, PartialEq, Eq)]
     enum LineInsertion {
@@ -289,6 +293,7 @@ fn format_import_block(
                         is_first_statement,
                         settings.split_on_trailing_comma
                             && matches!(trailing_comma, TrailingComma::Present),
+                        preview,
                     ));
 
                     if settings.from_first {
@@ -311,14 +316,16 @@ mod tests {
     use rustc_hash::{FxHashMap, FxHashSet};
     use test_case::test_case;
 
+    use ruff_python_ast::{PySourceType, SourceType};
     use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport};
 
     use crate::assert_diagnostics;
     use crate::registry::Rule;
     use crate::rules::isort::categorize::{ImportSection, KnownModules};
     use crate::settings::LinterSettings;
-    use crate::settings::types::IdentifierPattern;
-    use crate::test::{test_path, test_resource_path};
+    use crate::settings::types::{IdentifierPattern, PreviewMode};
+    use crate::source_kind::SourceKind;
+    use crate::test::{test_contents, test_path, test_resource_path};
 
     use super::categorize::ImportType;
     use super::settings::RelativeImportsOrder;
@@ -333,6 +340,8 @@ mod tests {
     #[test_case(Path::new("deduplicate_imports.py"))]
     #[test_case(Path::new("fit_line_length.py"))]
     #[test_case(Path::new("fit_line_length_comment.py"))]
+    #[test_case(Path::new("fit_line_length_mixed_pragma.py"))]
+    #[test_case(Path::new("fit_line_length_pragma.py"))]
     #[test_case(Path::new("force_sort_within_sections.py"))]
     #[test_case(Path::new("force_to_top.py"))]
     #[test_case(Path::new("force_wrap_aliases.py"))]
@@ -390,6 +399,59 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Path::new("fit_line_length_mixed_pragma.py"))]
+    #[test_case(Path::new("fit_line_length_pragma.py"))]
+    fn preview(path: &Path) -> Result<()> {
+        let snapshot = format!("preview__{}", path.to_string_lossy());
+        let diagnostics = test_path(
+            Path::new("isort").join(path).as_path(),
+            &LinterSettings {
+                preview: PreviewMode::Enabled,
+                src: vec![test_resource_path("fixtures/isort")],
+                ..LinterSettings::for_rule(Rule::UnsortedImports)
+            },
+        )?;
+        assert_diagnostics!(snapshot, diagnostics);
+        Ok(())
+    }
+
+    /// Fixing I001 must never leave behind a line that E501 then flags.
+    ///
+    /// isort excludes pragma comments (e.g., `# noqa: TID251`) from its width computation
+    /// per comment, while E501 measures the emitted line's single comment token as a whole.
+    /// When isort merges separate comments onto one line (e.g., a statement-level `# explain`
+    /// and an alias-level `# noqa`), the two computations could disagree; this test verifies
+    /// that the fix nonetheless converges to output that E501 accepts, in both stable and
+    /// preview modes.
+    #[test_case(PreviewMode::Disabled, "stable")]
+    #[test_case(PreviewMode::Enabled, "preview")]
+    fn no_line_too_long_after_fix(preview: PreviewMode, label: &str) -> Result<()> {
+        let path = test_resource_path("fixtures").join("isort/fit_line_length_merged_pragma.py");
+        let source_type = SourceType::Python(PySourceType::from(&path));
+        let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source");
+        let settings = LinterSettings {
+            preview,
+            src: vec![test_resource_path("fixtures/isort")],
+            ..LinterSettings::for_rules([Rule::UnsortedImports, Rule::LineTooLong])
+        };
+
+        // `test_contents` applies fixes to convergence (and panics if they fail to converge).
+        let (_, transformed) = test_contents(&source_kind, &path, &settings);
+        insta::assert_snapshot!(
+            format!("fit_line_length_merged_pragma_fixed_{label}"),
+            transformed.source_code()
+        );
+
+        // Re-linting the converged output must produce no diagnostics: I001 is fully fixed
+        // and, in particular, the fix must not have introduced any E501 violations.
+        let (diagnostics, _) = test_contents(&transformed, &path, &settings);
+        assert!(
+            diagnostics.is_empty(),
+            "expected no diagnostics after applying fixes, found:\n{diagnostics:#?}"
+        );
+        Ok(())
+    }
+
     fn pattern(pattern: &str) -> IdentifierPattern {
         IdentifierPattern::new(pattern).unwrap()
     }

crates/ruff_linter/src/rules/isort/rules/organize_imports.rs

@@ -169,6 +169,7 @@ pub(crate) fn organize_imports(
         source_type,
         target_version,
         &settings.isort,
+        settings.preview,
         tokens,
     );
 

crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__fit_line_length_pragma.py.snap

@@ -0,0 +1,25 @@
+---
+source: crates/ruff_linter/src/rules/isort/mod.rs
+---
+I001 [*] Import block is un-sorted or un-formatted
+ --> fit_line_length_pragma.py:3:1
+  |
+1 |   # The next import fits on one line once the pragma comment is excluded from the width;
+2 |   # it should not be wrapped (the `# noqa` must stay effective).
+3 | / from aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa import x  # noqa: TID251
+4 | | # The next import exceeds the line length even without the pragma comment;
+5 | | # it must still be wrapped.
+6 | | from bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb import x  # noqa: TID251
+  | |_________________________________________________________________________________________^
+help: Organize imports
+   |
+3  | from aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa import x  # noqa: TID251
+4  +
+5  | # The next import exceeds the line length even without the pragma comment;
+6  | # it must still be wrapped.
+   - from bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb import x  # noqa: TID251
+7  + from bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb import (
+8  +     x,  # noqa: TID251
+9  + )
+10 |
+   |

crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__inline_comments.py.snap

@@ -17,21 +17,15 @@ I001 [*] Import block is un-sorted or un-formatted
 11 | | from d.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL, OTHER_RUNNING_TOTAL  # type:ignore[attr-defined]
    | |______________________________________________________________________________________^
 help: Organize imports
-   |
-3  | )
-   -
-4  | from b.prometheus.metrics import (
-5  |     TERMINAL_CURRENTLY_RUNNING_TOTAL,  # type:ignore[attr-defined]
-6  | )
-   -
-   - from c.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL  # type:ignore[attr-defined]
-   -
-   - from d.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL, OTHER_RUNNING_TOTAL  # type:ignore[attr-defined]
-7  + from c.prometheus.metrics import (
-8  +     TERMINAL_CURRENTLY_RUNNING_TOTAL,  # type:ignore[attr-defined]
-9  + )
-10 + from d.prometheus.metrics import (  # type:ignore[attr-defined]
-11 +     OTHER_RUNNING_TOTAL,
-12 +     TERMINAL_CURRENTLY_RUNNING_TOTAL,
-13 + )
-   |
+  |
+3 | )
+  -
+4 | from b.prometheus.metrics import (
+5 |     TERMINAL_CURRENTLY_RUNNING_TOTAL,  # type:ignore[attr-defined]
+6 | )
+  -
+7 | from c.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL  # type:ignore[attr-defined]
+  -
+  - from d.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL, OTHER_RUNNING_TOTAL  # type:ignore[attr-defined]
+8 + from d.prometheus.metrics import OTHER_RUNNING_TOTAL, TERMINAL_CURRENTLY_RUNNING_TOTAL  # type:ignore[attr-defined]
+  |

crates/ruff_linter/src/rules/pycodestyle/overlong.rs

@@ -1,11 +1,10 @@
 use std::ops::Deref;
 
-use ruff_python_trivia::{CommentRanges, find_trailing_pragma_offset, is_pragma_comment};
+use ruff_python_trivia::CommentRanges;
 use ruff_source_file::Line;
 use ruff_text_size::{TextLen, TextRange};
 
-use crate::line_width::{IndentWidth, LineLength, LineWidthBuilder};
-use crate::preview::is_trailing_pragma_in_line_length_enabled;
+use crate::line_width::{IndentWidth, LineLength, LineWidthBuilder, pragma_offset_for_line_length};
 use crate::settings::types::PreviewMode;
 
 #[derive(Debug)]
@@ -134,18 +133,10 @@ impl<'a> StrippedLine<'a> {
         let comment = &line.as_str()[comment_range];
 
         // Ex) `# type: ignore` or (in preview) `# some comment # noqa: F401`
-        if is_trailing_pragma_in_line_length_enabled(preview) {
-            if let Some(offset) = find_trailing_pragma_offset(comment) {
-                // Strip only the pragma suffix from the comment, preserving any
-                // preceding non-pragma comment text.
-                let pragma_start = usize::from(comment_range.start()) + offset;
-                let prefix = line[..pragma_start].trim_end();
-                return Self::WithoutPragma(Line::new(prefix, line.start()));
-            }
-        }
-        // Stable behavior: only strip when the entire comment is a pragma.
-        else if is_pragma_comment(comment) {
-            let prefix = &line.as_str()[..usize::from(comment_range.start())].trim_end();
+        if let Some(offset) = pragma_offset_for_line_length(comment, preview) {
+            // Strip the pragma from the line, preserving any preceding non-pragma comment text.
+            let pragma_start = usize::from(comment_range.start()) + offset;
+            let prefix = line[..pragma_start].trim_end();
             return Self::WithoutPragma(Line::new(prefix, line.start()));
         }