Skip to content

Commit

Permalink
Avoid printing continuations within import identifiers (#7744)
Browse files Browse the repository at this point in the history
## Summary

It turns out that _some_ identifiers can contain newlines --
specifically, dot-delimited import identifiers, like:
```python
import foo\
    .bar
```

At present, we print all identifiers verbatim, which causes us to retain
the `\` in the formatted output. This also leads to violating some debug
assertions (see the linked issue, though that's a symptom of this
formatting failure).

This PR adds detection for import identifiers that contain newlines, and
formats them via `text` (slow) rather than `source_code_slice` (fast) in
those cases.

Closes #7734.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Oct 2, 2023
1 parent 0df2737 commit c71ff7e
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
from a import aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa, aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
from a import aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa as dfgsdfgsd, aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa as sdkjflsdjlahlfd

# Continuations.
import foo\
.bar

from foo\
.bar import baz

# At the top-level, force one empty line after an import, but allow up to two empty
# lines.
import os
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_python_formatter/src/other/alias.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_formatter::write;
use ruff_python_ast::Alias;

use crate::other::identifier::DotDelimitedIdentifier;
use crate::prelude::*;

#[derive(Default)]
Expand All @@ -13,7 +14,7 @@ impl FormatNodeRule<Alias> for FormatAlias {
name,
asname,
} = item;
name.format().fmt(f)?;
DotDelimitedIdentifier::new(name).fmt(f)?;
if let Some(asname) = asname {
write!(f, [space(), token("as"), space(), asname.format()])?;
}
Expand Down
40 changes: 40 additions & 0 deletions crates/ruff_python_formatter/src/other/identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,43 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Identifier {
FormatOwnedWithRule::new(self, FormatIdentifier)
}
}

/// A formatter for a dot-delimited identifier, as seen in import statements:
/// ```python
/// import foo.bar
/// ```
///
/// Dot-delimited identifiers can contain newlines via continuations (backslashes) after the
/// dot-delimited segment, as in:
/// ```python
/// import foo\
/// .bar
/// ```
///
/// While identifiers can typically be formatted via verbatim source code slices, dot-delimited
/// identifiers with newlines must be formatted via `text`. This struct implements both the fast
/// and slow paths for such identifiers.
#[derive(Debug)]
pub(crate) struct DotDelimitedIdentifier<'a>(&'a Identifier);

impl<'a> DotDelimitedIdentifier<'a> {
pub(crate) fn new(identifier: &'a Identifier) -> Self {
Self(identifier)
}
}

impl Format<PyFormatContext<'_>> for DotDelimitedIdentifier<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
// An import identifier can contain newlines by inserting continuations (backslashes) after
// a dot-delimited segment, as in:
// ```python
// import foo\
// .bar
// ```
if memchr::memchr(b'\\', f.context().source()[self.0.range()].as_bytes()).is_some() {
text(self.0.as_str(), Some(self.0.start())).fmt(f)
} else {
source_text_slice(self.0.range()).fmt(f)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use ruff_text_size::Ranged;
use crate::builders::{parenthesize_if_expands, PyFormatterExtensions, TrailingComma};
use crate::comments::{SourceComment, SuppressionKind};
use crate::expression::parentheses::parenthesized;
use crate::other::identifier::DotDelimitedIdentifier;
use crate::prelude::*;

#[derive(Default)]
Expand All @@ -31,7 +32,7 @@ impl FormatNodeRule<StmtImportFrom> for FormatStmtImportFrom {
}
Ok(())
}),
module.as_ref().map(AsFormat::format),
module.as_ref().map(DotDelimitedIdentifier::new),
space(),
token("import"),
space(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ from a import aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjf
from a import aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa, aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
from a import aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa as dfgsdfgsd, aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa as sdkjflsdjlahlfd
# Continuations.
import foo\
.bar
from foo\
.bar import baz
# At the top-level, force one empty line after an import, but allow up to two empty
# lines.
import os
Expand Down Expand Up @@ -98,6 +105,11 @@ from a import (
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa as sdkjflsdjlahlfd,
)
# Continuations.
import foo.bar
from foo.bar import baz
# At the top-level, force one empty line after an import, but allow up to two empty
# lines.
import os
Expand Down

0 comments on commit c71ff7e

Please sign in to comment.