Magic Trailing Commas in isort

README.md

@@ -2530,6 +2530,26 @@ known-third-party = ["src"]
 
 ---
 
+#### [`split-on-trailing-comma`](#split-on-trailing-comma)
+
+If a comma is placed after the last member in a multi-line import, then
+the imports will never be folded into one line.
+
+See isort's [`split-on-trailing-comma`](https://pycqa.github.io/isort/docs/configuration/options.html#split-on-trailing-comma) option.
+
+**Default value**: `true`
+
+**Type**: `bool`
+
+**Example usage**:
+
+```toml
+[tool.ruff.isort]
+split-on-trailing-comma = false
+```
+
+---
+
 ### `mccabe`
 
 #### [`max-complexity`](#max-complexity)

resources/test/fixtures/isort/magic_trailing_comma.py

@@ -0,0 +1,38 @@
+# This has a magic trailing comma, will be sorted, but not rolled into one line
+from sys import (
+    stderr,
+    argv,
+    stdout,
+    exit,
+)
+
+# No magic comma, this will be rolled into one line.
+from os import (
+    path,
+    environ,
+    execl,
+    execv
+)
+
+from glob import (
+    glob,
+    iglob,
+    escape,  # Ends with a comment, should still treat as magic trailing comma.
+)
+
+# These will be combined, but without a trailing comma.
+from foo import bar
+from foo import baz
+
+# These will be combined, _with_ a trailing comma.
+from module1 import member1
+from module1 import (
+    member2,
+    member3,
+)
+
+# These will be combined, _with_ a trailing comma.
+from module2 import member1, member2
+from module2 import (
+    member3,
+)

ruff.schema.json

@@ -1102,6 +1102,13 @@
           "items": {
             "type": "string"
           }
+        },
+        "split-on-trailing-comma": {
+          "description": "If a comma is placed after the last member in a multi-line import, then the imports will never be folded into one line.\n\nSee isort's [`split-on-trailing-comma`](https://pycqa.github.io/isort/docs/configuration/options.html#split-on-trailing-comma) option.",
+          "type": [
+            "boolean",
+            "null"
+          ]
         }
       },
       "additionalProperties": false

src/isort/format.rs

@@ -41,6 +41,7 @@ pub fn format_import_from(
     line_length: usize,
     force_wrap_aliases: bool,
     is_first: bool,
+    trailing_comma: bool,
 ) -> String {
     if aliases.len() == 1
         && aliases
@@ -53,9 +54,10 @@ pub fn format_import_from(
 
     // We can only inline if: (1) none of the aliases have atop comments, and (3)
     // only the last alias (if any) has inline comments.
-    if aliases
-        .iter()
-        .all(|(_, CommentSet { atop, .. })| atop.is_empty())
+    if !trailing_comma
+        && aliases
+            .iter()
+            .all(|(_, CommentSet { atop, .. })| atop.is_empty())
         && aliases
             .iter()
             .rev()

src/isort/helpers.rs

@@ -1,7 +1,37 @@
 use rustpython_ast::Stmt;
+use rustpython_parser::lexer;
+use rustpython_parser::lexer::Tok;
 
+use crate::ast::types::Range;
+use crate::isort::types::TrailingComma;
 use crate::source_code_locator::SourceCodeLocator;
 
+/// Return `true` if a `StmtKind::ImportFrom` statement ends with a magic
+/// trailing comma.
+pub fn trailing_comma(stmt: &Stmt, locator: &SourceCodeLocator) -> TrailingComma {
+    let contents = locator.slice_source_code_range(&Range::from_located(stmt));
+    let mut count: usize = 0;
+    let mut trailing_comma = TrailingComma::Absent;
+    for (_, tok, _) in lexer::make_tokenizer(&contents).flatten() {
+        if matches!(tok, Tok::Lpar) {
+            count += 1;
+        }
+        if matches!(tok, Tok::Rpar) {
+            count -= 1;
+        }
+        if count == 1 {
+            if matches!(tok, Tok::Newline | Tok::Indent | Tok::Dedent | Tok::Comment) {
+                continue;
+            } else if matches!(tok, Tok::Comma) {
+                trailing_comma = TrailingComma::Present;
+            } else {
+                trailing_comma = TrailingComma::Absent;
+            }
+        }
+    }
+    trailing_comma
+}
+
 /// Return `true` if a `Stmt` is preceded by a "comment break"
 pub fn has_comment_break(stmt: &Stmt, locator: &SourceCodeLocator) -> bool {
     // Starting from the `Stmt` (`def f(): pass`), we want to detect patterns like

src/isort/mod.rs

@@ -9,11 +9,14 @@ use rustpython_ast::{Stmt, StmtKind};
 
 use crate::isort::categorize::{categorize, ImportType};
 use crate::isort::comments::Comment;
+use crate::isort::helpers::trailing_comma;
 use crate::isort::sorting::{cmp_import_froms, cmp_members, cmp_modules};
 use crate::isort::track::{Block, Trailer};
 use crate::isort::types::{
     AliasData, CommentSet, ImportBlock, ImportFromData, Importable, OrderedImportBlock,
+    TrailingComma,
 };
+use crate::SourceCodeLocator;
 
 mod categorize;
 mod comments;
@@ -32,6 +35,7 @@ pub struct AnnotatedAliasData<'a> {
     pub atop: Vec<Comment<'a>>,
     pub inline: Vec<Comment<'a>>,
 }
+
 #[derive(Debug)]
 pub enum AnnotatedImport<'a> {
     Import {
@@ -45,12 +49,15 @@ pub enum AnnotatedImport<'a> {
         level: Option<&'a usize>,
         atop: Vec<Comment<'a>>,
         inline: Vec<Comment<'a>>,
+        trailing_comma: TrailingComma,
     },
 }
 
 fn annotate_imports<'a>(
     imports: &'a [&'a Stmt],
     comments: Vec<Comment<'a>>,
+    locator: &SourceCodeLocator,
+    split_on_trailing_comma: bool,
 ) -> Vec<AnnotatedImport<'a>> {
     let mut annotated = vec![];
     let mut comments_iter = comments.into_iter().peekable();
@@ -137,6 +144,11 @@ fn annotate_imports<'a>(
                     module: module.as_ref(),
                     names: aliases,
                     level: level.as_ref(),
+                    trailing_comma: if split_on_trailing_comma {
+                        trailing_comma(import, locator)
+                    } else {
+                        TrailingComma::default()
+                    },
                     atop,
                     inline,
                 });
@@ -190,7 +202,9 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
                 level,
                 atop,
                 inline,
+                trailing_comma,
             } => {
+                // Associate the comments with the first alias (best effort).
                 let single_import = names.len() == 1;
 
                 // If we're dealing with a multi-import block (i.e., a non-star, non-aliased
@@ -289,6 +303,15 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
                         entry.inline.push(comment.value);
                     }
                 }
+
+                // Propagate trailing commas.
+                if matches!(trailing_comma, TrailingComma::Present) {
+                    if let Some(entry) =
+                        block.import_from.get_mut(&ImportFromData { module, level })
+                    {
+                        entry.2 = trailing_comma;
+                    }
+                }
             }
         }
     }
@@ -412,6 +435,7 @@ fn sort_imports(block: ImportBlock) -> OrderedImportBlock {
                                         inline: comments.inline,
                                     },
                                 )]),
+                                TrailingComma::Absent,
                             ),
                         )
                     }),
@@ -439,40 +463,44 @@ fn sort_imports(block: ImportBlock) -> OrderedImportBlock {
                                         inline: comments.inline,
                                     },
                                 )]),
+                                TrailingComma::Absent,
                             ),
                         )
                     }),
             )
-            .map(|(import_from, (comments, aliases))| {
+            .map(|(import_from, (comments, aliases, locations))| {
                 // Within each `StmtKind::ImportFrom`, sort the members.
                 (
                     import_from,
                     comments,
+                    locations,
                     aliases
                         .into_iter()
                         .sorted_by(|(alias1, _), (alias2, _)| cmp_members(alias1, alias2))
                         .collect::<Vec<(AliasData, CommentSet)>>(),
                 )
             })
-            .sorted_by(|(import_from1, _, aliases1), (import_from2, _, aliases2)| {
-                cmp_import_froms(import_from1, import_from2).then_with(|| {
-                    match (aliases1.first(), aliases2.first()) {
-                        (None, None) => Ordering::Equal,
-                        (None, Some(_)) => Ordering::Less,
-                        (Some(_), None) => Ordering::Greater,
-                        (Some((alias1, _)), Some((alias2, _))) => cmp_members(alias1, alias2),
-                    }
-                })
-            }),
+            .sorted_by(
+                |(import_from1, _, _, aliases1), (import_from2, _, _, aliases2)| {
+                    cmp_import_froms(import_from1, import_from2).then_with(|| {
+                        match (aliases1.first(), aliases2.first()) {
+                            (None, None) => Ordering::Equal,
+                            (None, Some(_)) => Ordering::Less,
+                            (Some(_), None) => Ordering::Greater,
+                            (Some((alias1, _)), Some((alias2, _))) => cmp_members(alias1, alias2),
+                        }
+                    })
+                },
+            ),
     );
-
     ordered
 }
 
 #[allow(clippy::too_many_arguments)]
 pub fn format_imports(
     block: &Block,
     comments: Vec<Comment>,
+    locator: &SourceCodeLocator,
     line_length: usize,
     src: &[PathBuf],
     package: Option<&Path>,
@@ -481,9 +509,10 @@ pub fn format_imports(
     extra_standard_library: &BTreeSet<String>,
     combine_as_imports: bool,
     force_wrap_aliases: bool,
+    split_on_trailing_comma: bool,
 ) -> String {
     let trailer = &block.trailer;
-    let block = annotate_imports(&block.imports, comments);
+    let block = annotate_imports(&block.imports, comments, locator, split_on_trailing_comma);
 
     // Normalize imports (i.e., deduplicate, aggregate `from` imports).
     let block = normalize_imports(block, combine_as_imports);
@@ -521,14 +550,15 @@ pub fn format_imports(
         }
 
         // Format `StmtKind::ImportFrom` statements.
-        for (import_from, comments, aliases) in &import_block.import_from {
+        for (import_from, comments, trailing_comma, aliases) in &import_block.import_from {
             output.append(&format::format_import_from(
                 import_from,
                 comments,
                 aliases,
                 line_length,
                 force_wrap_aliases,
                 is_first_statement,
+                split_on_trailing_comma && matches!(trailing_comma, TrailingComma::Present),
             ));
             is_first_statement = false;
         }
@@ -588,6 +618,7 @@ mod tests {
     #[test_case(Path::new("split.py"))]
     #[test_case(Path::new("trailing_suffix.py"))]
     #[test_case(Path::new("type_comments.py"))]
+    #[test_case(Path::new("magic_trailing_comma.py"))]
     fn default(path: &Path) -> Result<()> {
         let snapshot = format!("{}", path.to_string_lossy());
         let mut checks = test_path(
@@ -646,4 +677,25 @@ mod tests {
         insta::assert_yaml_snapshot!(snapshot, checks);
         Ok(())
     }
+
+    #[test_case(Path::new("magic_trailing_comma.py"))]
+    fn no_split_on_trailing_comma(path: &Path) -> Result<()> {
+        let snapshot = format!("split_on_trailing_comma_{}", path.to_string_lossy());
+        let mut checks = test_path(
+            Path::new("./resources/test/fixtures/isort")
+                .join(path)
+                .as_path(),
+            &Settings {
+                isort: isort::settings::Settings {
+                    split_on_trailing_comma: false,
+                    ..isort::settings::Settings::default()
+                },
+                src: vec![Path::new("resources/test/fixtures/isort").to_path_buf()],
+                ..Settings::for_rule(CheckCode::I001)
+            },
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(snapshot, checks);
+        Ok(())
+    }
 }

src/isort/plugins.rs

@@ -72,6 +72,7 @@ pub fn check_imports(
     let expected = format_imports(
         block,
         comments,
+        locator,
         settings.line_length - indentation.len(),
         &settings.src,
         package,
@@ -80,6 +81,7 @@ pub fn check_imports(
         &settings.isort.extra_standard_library,
         settings.isort.combine_as_imports,
         settings.isort.force_wrap_aliases,
+        settings.isort.split_on_trailing_comma,
     );
 
     // Expand the span the entire range, including leading and trailing space.