Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Associate inline comments with parenthesized ImportFrom statements #1609

Merged
merged 1 commit into from Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions pyproject.toml
Expand Up @@ -36,6 +36,3 @@ urls = { repository = "https://github.com/charliermarsh/ruff-lsp" }
[tool.maturin]
bindings = "bin"
strip = true

[tool.ruff.pydocstyle]
convention = "google"
11 changes: 11 additions & 0 deletions resources/test/fixtures/isort/inline_comments.py
@@ -0,0 +1,11 @@
from a.prometheus.metrics import ( # type:ignore[attr-defined]
TERMINAL_CURRENTLY_RUNNING_TOTAL,
)

from b.prometheus.metrics import (
TERMINAL_CURRENTLY_RUNNING_TOTAL, # type:ignore[attr-defined]
)

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]
54 changes: 19 additions & 35 deletions src/isort/mod.rs
Expand Up @@ -108,11 +108,20 @@ fn annotate_imports<'a>(
}

// Find comments inline.
// We associate inline comments with the import statement unless there's a
// single member, and it's a single-line import (like `from foo
// import bar # noqa`).
let mut inline = vec![];
while let Some(comment) =
comments_iter.next_if(|comment| comment.location.row() == import.location.row())
if names.len() > 1
|| names
.first()
.map_or(false, |alias| alias.location.row() > import.location.row())
{
inline.push(comment);
while let Some(comment) = comments_iter
.next_if(|comment| comment.location.row() == import.location.row())
{
inline.push(comment);
}
}

// Capture names.
Expand Down Expand Up @@ -206,11 +215,6 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
inline,
trailing_comma,
} => {
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() {
let entry = if alias.name == "*" {
block
Expand Down Expand Up @@ -240,29 +244,8 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
entry.atop.push(comment.value);
}

// Associate inline comments with first alias if multiple names have been
// imported, i.e., the comment applies to all names; otherwise, associate
// with the alias.
if single_import
&& (alias.name != "*" && (alias.asname.is_none() || combine_as_imports))
{
let entry = block
.import_from
.entry(ImportFromData { module, level })
.or_default()
.1
.entry(AliasData {
name: alias.name,
asname: alias.asname,
})
.or_default();
for comment in inline {
entry.inline.push(comment.value);
}
} else {
for comment in inline {
entry.inline.push(comment.value);
}
for comment in inline {
entry.inline.push(comment.value);
}
}

Expand Down Expand Up @@ -662,9 +645,14 @@ mod tests {
#[test_case(Path::new("fit_line_length_comment.py"))]
#[test_case(Path::new("force_wrap_aliases.py"))]
#[test_case(Path::new("import_from_after_import.py"))]
#[test_case(Path::new("inline_comments.py"))]
#[test_case(Path::new("insert_empty_lines.py"))]
#[test_case(Path::new("insert_empty_lines.pyi"))]
#[test_case(Path::new("leading_prefix.py"))]
#[test_case(Path::new("line_ending_cr.py"))]
#[test_case(Path::new("line_ending_crlf.py"))]
#[test_case(Path::new("line_ending_lf.py"))]
#[test_case(Path::new("magic_trailing_comma.py"))]
#[test_case(Path::new("natural_order.py"))]
#[test_case(Path::new("no_reorder_within_section.py"))]
#[test_case(Path::new("no_wrap_star.py"))]
Expand All @@ -684,10 +672,6 @@ 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"))]
#[test_case(Path::new("line_ending_lf.py"))]
#[test_case(Path::new("line_ending_crlf.py"))]
#[test_case(Path::new("line_ending_cr.py"))]
fn default(path: &Path) -> Result<()> {
let snapshot = format!("{}", path.to_string_lossy());
let checks = test_path(
Expand Down
21 changes: 21 additions & 0 deletions src/isort/snapshots/ruff__isort__tests__inline_comments.py.snap
@@ -0,0 +1,21 @@
---
source: src/isort/mod.rs
expression: checks
---
- kind: UnsortedImports
location:
row: 1
column: 0
end_location:
row: 12
column: 0
fix:
content: "from a.prometheus.metrics import ( # type:ignore[attr-defined]\n TERMINAL_CURRENTLY_RUNNING_TOTAL,\n)\nfrom b.prometheus.metrics import (\n TERMINAL_CURRENTLY_RUNNING_TOTAL, # type:ignore[attr-defined]\n)\nfrom c.prometheus.metrics import (\n TERMINAL_CURRENTLY_RUNNING_TOTAL, # type:ignore[attr-defined]\n)\nfrom d.prometheus.metrics import ( # type:ignore[attr-defined]\n OTHER_RUNNING_TOTAL,\n TERMINAL_CURRENTLY_RUNNING_TOTAL,\n)\n"
location:
row: 1
column: 0
end_location:
row: 12
column: 0
parent: ~