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

format ExprJoinedStr #5932

Merged
merged 6 commits into from
Aug 1, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(
f'{one}'
f'{two}'
)


rf"Not-so-tricky \"quote"
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use ruff_python_ast::str::is_implicit_concatenation;

use crate::expression::number::{FormatComplex, FormatFloat, FormatInt};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::expression::string::{FormatString, StringLayout, StringPrefix, StringQuotes};
use crate::expression::string::{
AnyString, FormatString, StringLayout, StringPrefix, StringQuotes,
};
use crate::prelude::*;
use crate::FormatNodeRule;

Expand Down Expand Up @@ -56,7 +58,9 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
ExprConstantLayout::Default => StringLayout::Default,
ExprConstantLayout::String(layout) => layout,
};
FormatString::new(item).with_layout(string_layout).fmt(f)
FormatString::new(&AnyString::Constant(item))
.with_layout(string_layout)
.fmt(f)
}
}
}
Expand Down
15 changes: 6 additions & 9 deletions crates/ruff_python_formatter/src/expression/expr_joined_str.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
use super::string::{AnyString, FormatString};
use crate::context::PyFormatContext;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::{not_yet_implemented_custom_text, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use crate::prelude::*;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::FormatResult;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprJoinedStr;

#[derive(Default)]
pub struct FormatExprJoinedStr;

impl FormatNodeRule<ExprJoinedStr> for FormatExprJoinedStr {
fn fmt_fields(&self, _item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
write!(
f,
[not_yet_implemented_custom_text(
r#"f"NOT_YET_IMPLEMENTED_ExprJoinedStr""#
)]
)
fn fmt_fields(&self, item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
FormatString::new(&AnyString::JoinedStr(item)).fmt(f)
}
}

Expand Down
111 changes: 89 additions & 22 deletions crates/ruff_python_formatter/src/expression/string.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::borrow::Cow;

use bitflags::bitflags;
use ruff_python_ast::{ExprConstant, Ranged};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::{self as ast, ExprConstant, ExprJoinedStr, Ranged};
use ruff_python_parser::lexer::{lex_starts_at, LexicalError, LexicalErrorType};
use ruff_python_parser::{Mode, Tok};
use ruff_source_file::Locator;
use ruff_text_size::{TextLen, TextRange, TextSize};

use ruff_formatter::{format_args, write, FormatError};
Expand All @@ -13,11 +15,62 @@ use crate::comments::{leading_comments, trailing_comments};
use crate::expression::parentheses::{
in_parentheses_only_group, in_parentheses_only_soft_line_break_or_space,
};
use crate::expression::Expr;
use crate::prelude::*;
use crate::QuoteStyle;

#[derive(Copy, Clone)]
enum Quoting {
CanChange,
Preserve,
}
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
enum Quoting {
CanChange,
Preserve,
}
#[derive(Copy, Clone)]
enum Quoting {
CanChange,
Preserve,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open to better names (of the enum) btw, there are lots of related things around quoting behaviour

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like Preserve and tried to come up with a better name than CanChange but failed. I thought about Normalize to align with normalize_string but this isn't disabling normalization, it only affects the quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i meant more to replace Quoting (since we already have StringQuotes and QuoteStyle)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine. The use is very local and it conveys way more information than a plain boolean.


pub(super) enum AnyString<'a> {
Constant(&'a ExprConstant),
JoinedStr(&'a ExprJoinedStr),
}

impl<'a> AnyString<'a> {
fn quoting(&self, locator: &Locator) -> Quoting {
match self {
Self::Constant(_) => Quoting::CanChange,
Self::JoinedStr(joined_str) => {
if joined_str.values.iter().any(|value| match value {
Expr::FormattedValue(ast::ExprFormattedValue { range, .. }) => {
let string_content = locator.slice(*range);
string_content.contains(['"', '\''])
}
_ => false,
}) {
Quoting::Preserve
} else {
Quoting::CanChange
}
}
}
}
}

impl Ranged for AnyString<'_> {
fn range(&self) -> TextRange {
match self {
Self::Constant(expr) => expr.range(),
Self::JoinedStr(expr) => expr.range(),
}
}
}

impl<'a> From<&AnyString<'a>> for AnyNodeRef<'a> {
fn from(value: &AnyString<'a>) -> Self {
match value {
AnyString::Constant(expr) => AnyNodeRef::ExprConstant(expr),
AnyString::JoinedStr(expr) => AnyNodeRef::ExprJoinedStr(expr),
}
}
}

pub(super) struct FormatString<'a> {
constant: &'a ExprConstant,
string: &'a AnyString<'a>,
layout: StringLayout,
}

Expand All @@ -30,10 +83,12 @@ pub enum StringLayout {
}

impl<'a> FormatString<'a> {
pub(super) fn new(constant: &'a ExprConstant) -> Self {
debug_assert!(constant.value.is_str() || constant.value.is_bytes());
pub(super) fn new(string: &'a AnyString) -> Self {
if let AnyString::Constant(constant) = string {
debug_assert!(constant.value.is_str() || constant.value.is_bytes());
}
Self {
constant,
string,
layout: StringLayout::Default,
}
}
Expand All @@ -48,40 +103,43 @@ impl<'a> Format<PyFormatContext<'_>> for FormatString<'a> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
match self.layout {
StringLayout::Default => {
let string_range = self.constant.range();
let string_range = self.string.range();
let string_content = f.context().locator().slice(string_range);

if is_implicit_concatenation(string_content) {
in_parentheses_only_group(&FormatStringContinuation::new(self.constant)).fmt(f)
in_parentheses_only_group(&FormatStringContinuation::new(self.string)).fmt(f)
} else {
FormatStringPart::new(string_range).fmt(f)
FormatStringPart::new(string_range, self.string.quoting(&f.context().locator()))
.fmt(f)
}
}
StringLayout::ImplicitConcatenatedBinaryLeftSide => {
FormatStringContinuation::new(self.constant).fmt(f)
FormatStringContinuation::new(self.string).fmt(f)
}
}
}
}

struct FormatStringContinuation<'a> {
constant: &'a ExprConstant,
string: &'a AnyString<'a>,
}

impl<'a> FormatStringContinuation<'a> {
fn new(constant: &'a ExprConstant) -> Self {
debug_assert!(constant.value.is_str() || constant.value.is_bytes());
Self { constant }
fn new(string: &'a AnyString<'a>) -> Self {
if let AnyString::Constant(constant) = string {
debug_assert!(constant.value.is_str() || constant.value.is_bytes());
}
Self { string }
}
}

impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
let comments = f.context().comments().clone();
let locator = f.context().locator();
let mut dangling_comments = comments.dangling_comments(self.constant);
let mut dangling_comments = comments.dangling_comments(self.string);

let string_range = self.constant.range();
let string_range = self.string.range();
let string_content = locator.slice(string_range);

// The AST parses implicit concatenation as a single string.
Expand Down Expand Up @@ -155,7 +213,7 @@ impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {
joiner.entry(&format_args![
line_suffix_boundary(),
leading_comments(leading_part_comments),
FormatStringPart::new(token_range),
FormatStringPart::new(token_range, self.string.quoting(&locator)),
trailing_comments(trailing_part_comments)
]);

Expand All @@ -178,11 +236,15 @@ impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {

struct FormatStringPart {
part_range: TextRange,
quoting: Quoting,
}

impl FormatStringPart {
const fn new(range: TextRange) -> Self {
Self { part_range: range }
const fn new(range: TextRange, quoting: Quoting) -> Self {
Self {
part_range: range,
quoting,
}
}
}

Expand All @@ -204,10 +266,15 @@ impl Format<PyFormatContext<'_>> for FormatStringPart {

let raw_content = &string_content[relative_raw_content_range];
let is_raw_string = prefix.is_raw_string();
let preferred_quotes = if is_raw_string {
preferred_quotes_raw(raw_content, quotes, f.options().quote_style())
} else {
preferred_quotes(raw_content, quotes, f.options().quote_style())
let preferred_quotes = match self.quoting {
Quoting::Preserve => quotes,
Quoting::CanChange => {
if is_raw_string {
preferred_quotes_raw(raw_content, quotes, f.options().quote_style())
} else {
preferred_quotes(raw_content, quotes, f.options().quote_style())
}
}
};

write!(f, [prefix, preferred_quotes])?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DebugVisitor(Visitor[T]):
```diff
--- Black
+++ Ruff
@@ -3,24 +3,29 @@
@@ -3,24 +3,24 @@
tree_depth: int = 0

def visit_default(self, node: LN) -> Iterator[T]:
Expand All @@ -53,30 +53,25 @@ class DebugVisitor(Visitor[T]):
if isinstance(node, Node):
_type = type_repr(node.type)
- out(f'{indent}{_type}', fg='yellow')
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
+ out(f"{indent}{_type}", fg="yellow")
self.tree_depth += 1
for child in node.children:
yield from self.visit(child)

self.tree_depth -= 1
- out(f'{indent}/{_type}', fg='yellow', bold=False)
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
+ out(f"{indent}/{_type}", fg="yellow", bold=False)
else:
_type = token.tok_name.get(node.type, str(node.type))
- out(f'{indent}{_type}', fg='blue', nl=False)
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
+ out(f"{indent}{_type}", fg="blue", nl=False)
if node.prefix:
# We don't have to handle prefixes for `Node` objects since
# that delegates to the first child anyway.
- out(f' {node.prefix!r}', fg='green', bold=False, nl=False)
- out(f' {node.value!r}', fg='blue', bold=False)
+ out(
+ f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
+ fg="green",
+ bold=False,
+ nl=False,
+ )
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
+ out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
+ out(f" {node.value!r}", fg="blue", bold=False)

@classmethod
def show(cls, code: str) -> None:
Expand All @@ -93,26 +88,21 @@ class DebugVisitor(Visitor[T]):
indent = " " * (2 * self.tree_depth)
if isinstance(node, Node):
_type = type_repr(node.type)
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
out(f"{indent}{_type}", fg="yellow")
self.tree_depth += 1
for child in node.children:
yield from self.visit(child)

self.tree_depth -= 1
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
out(f"{indent}/{_type}", fg="yellow", bold=False)
else:
_type = token.tok_name.get(node.type, str(node.type))
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
out(f"{indent}{_type}", fg="blue", nl=False)
if node.prefix:
# We don't have to handle prefixes for `Node` objects since
# that delegates to the first child anyway.
out(
f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
fg="green",
bold=False,
nl=False,
)
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
out(f" {node.value!r}", fg="blue", bold=False)

@classmethod
def show(cls, code: str) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def do_not_touch_this_prefix3():

def do_not_touch_this_prefix2():
- FR'There was a bug where docstring prefixes would be normalized even with -S.'
+ f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+ Rf"There was a bug where docstring prefixes would be normalized even with -S."


def do_not_touch_this_prefix3():
Expand All @@ -43,7 +43,7 @@ def do_not_touch_this_prefix():


def do_not_touch_this_prefix2():
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
Rf"There was a bug where docstring prefixes would be normalized even with -S."


def do_not_touch_this_prefix3():
Expand Down
Loading