mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-27 12:29:48 +00:00
Only re-associate inline comments during normalization when necessary (#1380)
This commit is contained in:
parent
939f738a71
commit
fa54538bd1
4 changed files with 65 additions and 58 deletions
3
resources/test/fixtures/isort/comments.py
vendored
3
resources/test/fixtures/isort/comments.py
vendored
|
@ -23,3 +23,6 @@ from A import (
|
||||||
b, # Comment 10
|
b, # Comment 10
|
||||||
c, # Comment 11
|
c, # Comment 11
|
||||||
)
|
)
|
||||||
|
|
||||||
|
from D import a_long_name_to_force_multiple_lines # Comment 12
|
||||||
|
from D import another_long_name_to_force_multiple_lines # Comment 13
|
||||||
|
|
102
src/isort/mod.rs
102
src/isort/mod.rs
|
@ -191,33 +191,25 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
|
||||||
atop,
|
atop,
|
||||||
inline,
|
inline,
|
||||||
} => {
|
} => {
|
||||||
// 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
|
||||||
|
// import), associate the comments with the first alias (best
|
||||||
|
// effort).
|
||||||
if let Some(alias) = names.first() {
|
if let Some(alias) = names.first() {
|
||||||
if alias.name == "*" {
|
let entry = if alias.name == "*" {
|
||||||
let entry = block
|
block
|
||||||
.import_from_star
|
.import_from_star
|
||||||
.entry(ImportFromData { module, level })
|
.entry(ImportFromData { module, level })
|
||||||
.or_default();
|
.or_default()
|
||||||
for comment in atop {
|
|
||||||
entry.atop.push(comment.value);
|
|
||||||
}
|
|
||||||
for comment in inline {
|
|
||||||
entry.inline.push(comment.value);
|
|
||||||
}
|
|
||||||
} else if alias.asname.is_none() || combine_as_imports {
|
} else if alias.asname.is_none() || combine_as_imports {
|
||||||
let entry = &mut block
|
&mut block
|
||||||
.import_from
|
.import_from
|
||||||
.entry(ImportFromData { module, level })
|
.entry(ImportFromData { module, level })
|
||||||
.or_default()
|
.or_default()
|
||||||
.0;
|
.0
|
||||||
for comment in atop {
|
|
||||||
entry.atop.push(comment.value);
|
|
||||||
}
|
|
||||||
for comment in inline {
|
|
||||||
entry.inline.push(comment.value);
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
let entry = block
|
block
|
||||||
.import_from_as
|
.import_from_as
|
||||||
.entry((
|
.entry((
|
||||||
ImportFromData { module, level },
|
ImportFromData { module, level },
|
||||||
|
@ -226,30 +218,19 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
|
||||||
asname: alias.asname,
|
asname: alias.asname,
|
||||||
},
|
},
|
||||||
))
|
))
|
||||||
.or_default();
|
.or_default()
|
||||||
|
};
|
||||||
|
|
||||||
for comment in atop {
|
for comment in atop {
|
||||||
entry.atop.push(comment.value);
|
entry.atop.push(comment.value);
|
||||||
}
|
}
|
||||||
for comment in inline {
|
|
||||||
entry.inline.push(comment.value);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Create an entry for every alias.
|
// Associate inline comments with first alias if multiple names have been
|
||||||
for alias in names {
|
// imported, i.e., the comment applies to all names; otherwise, associate
|
||||||
if alias.name == "*" {
|
// with the alias.
|
||||||
let entry = block
|
if single_import
|
||||||
.import_from_star
|
&& (alias.name != "*" && (alias.asname.is_none() || combine_as_imports))
|
||||||
.entry(ImportFromData { module, level })
|
{
|
||||||
.or_default();
|
|
||||||
for comment in alias.atop {
|
|
||||||
entry.atop.push(comment.value);
|
|
||||||
}
|
|
||||||
for comment in alias.inline {
|
|
||||||
entry.inline.push(comment.value);
|
|
||||||
}
|
|
||||||
} else if alias.asname.is_none() || combine_as_imports {
|
|
||||||
let entry = block
|
let entry = block
|
||||||
.import_from
|
.import_from
|
||||||
.entry(ImportFromData { module, level })
|
.entry(ImportFromData { module, level })
|
||||||
|
@ -260,14 +241,36 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
|
||||||
asname: alias.asname,
|
asname: alias.asname,
|
||||||
})
|
})
|
||||||
.or_default();
|
.or_default();
|
||||||
for comment in alias.atop {
|
for comment in inline {
|
||||||
entry.atop.push(comment.value);
|
|
||||||
}
|
|
||||||
for comment in alias.inline {
|
|
||||||
entry.inline.push(comment.value);
|
entry.inline.push(comment.value);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
let entry = block
|
for comment in inline {
|
||||||
|
entry.inline.push(comment.value);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create an entry for every alias.
|
||||||
|
for alias in names {
|
||||||
|
let entry = if alias.name == "*" {
|
||||||
|
block
|
||||||
|
.import_from_star
|
||||||
|
.entry(ImportFromData { module, level })
|
||||||
|
.or_default()
|
||||||
|
} else if alias.asname.is_none() || combine_as_imports {
|
||||||
|
block
|
||||||
|
.import_from
|
||||||
|
.entry(ImportFromData { module, level })
|
||||||
|
.or_default()
|
||||||
|
.1
|
||||||
|
.entry(AliasData {
|
||||||
|
name: alias.name,
|
||||||
|
asname: alias.asname,
|
||||||
|
})
|
||||||
|
.or_default()
|
||||||
|
} else {
|
||||||
|
block
|
||||||
.import_from_as
|
.import_from_as
|
||||||
.entry((
|
.entry((
|
||||||
ImportFromData { module, level },
|
ImportFromData { module, level },
|
||||||
|
@ -276,10 +279,12 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
|
||||||
asname: alias.asname,
|
asname: alias.asname,
|
||||||
},
|
},
|
||||||
))
|
))
|
||||||
.or_default();
|
.or_default()
|
||||||
entry
|
};
|
||||||
.atop
|
|
||||||
.extend(alias.atop.into_iter().map(|comment| comment.value));
|
for comment in alias.atop {
|
||||||
|
entry.atop.push(comment.value);
|
||||||
|
}
|
||||||
for comment in alias.inline {
|
for comment in alias.inline {
|
||||||
entry.inline.push(comment.value);
|
entry.inline.push(comment.value);
|
||||||
}
|
}
|
||||||
|
@ -287,7 +292,6 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
block
|
block
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -7,14 +7,14 @@ expression: checks
|
||||||
row: 3
|
row: 3
|
||||||
column: 0
|
column: 0
|
||||||
end_location:
|
end_location:
|
||||||
row: 26
|
row: 29
|
||||||
column: 0
|
column: 0
|
||||||
fix:
|
fix:
|
||||||
content: "import B # Comment 4\n\n# Comment 3a\n# Comment 3b\nimport C\nimport D\n\n# Comment 5\n# Comment 6\nfrom A import (\n a, # Comment 7 # Comment 9\n b, # Comment 10\n c, # Comment 8 # Comment 11\n)\n"
|
content: "import B # Comment 4\n\n# Comment 3a\n# Comment 3b\nimport C\nimport D\n\n# Comment 5\n# Comment 6\nfrom A import (\n a, # Comment 7 # Comment 9\n b, # Comment 10\n c, # Comment 8 # Comment 11\n)\nfrom D import (\n a_long_name_to_force_multiple_lines, # Comment 12\n another_long_name_to_force_multiple_lines, # Comment 13\n)\n"
|
||||||
location:
|
location:
|
||||||
row: 3
|
row: 3
|
||||||
column: 0
|
column: 0
|
||||||
end_location:
|
end_location:
|
||||||
row: 26
|
row: 29
|
||||||
column: 0
|
column: 0
|
||||||
|
|
||||||
|
|
|
@ -10,7 +10,7 @@ expression: checks
|
||||||
row: 5
|
row: 5
|
||||||
column: 0
|
column: 0
|
||||||
fix:
|
fix:
|
||||||
content: "import a\n\n# Don't take this comment into account when determining whether the next import can fit on one line.\nfrom b import c\nfrom d import ( # Do take this comment into account when determining whether the next import can fit on one line.\n e,\n)\n"
|
content: "import a\n\n# Don't take this comment into account when determining whether the next import can fit on one line.\nfrom b import c\nfrom d import (\n e, # Do take this comment into account when determining whether the next import can fit on one line.\n)\n"
|
||||||
location:
|
location:
|
||||||
row: 1
|
row: 1
|
||||||
column: 0
|
column: 0
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue