Skip to content

Commit

Permalink
All tests are passing
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Oct 16, 2024
1 parent a94914d commit ae9c6a9
Show file tree
Hide file tree
Showing 30 changed files with 1,069 additions and 317 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,7 @@ impl<Context> std::fmt::Debug for Group<'_, Context> {
/// layout doesn't exceed the line width too, in which case it falls back to the flat layout.
///
/// This IR is identical to the following [`best_fitting`] layout but is implemented as custom IR for
/// best performance.
/// better performance.
///
/// ```rust
/// # use ruff_formatter::prelude::*;
Expand Down
2 changes: 0 additions & 2 deletions crates/ruff_python_formatter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use ruff_source_file::Locator;
use std::fmt::{Debug, Formatter};
use std::ops::{Deref, DerefMut};

#[derive(Clone)]
pub struct PyFormatContext<'a> {
options: PyFormatOptions,
contents: &'a str,
Expand Down Expand Up @@ -52,7 +51,6 @@ impl<'a> PyFormatContext<'a> {
self.contents
}

#[allow(unused)]
pub(crate) fn locator(&self) -> Locator<'a> {
Locator::new(self.contents)
}
Expand Down
29 changes: 27 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use ruff_formatter::FormatRuleWithOptions;
use ruff_formatter::GroupId;
use ruff_python_ast::ExprBytesLiteral;
use ruff_python_ast::{AnyNodeRef, StringLike};

Expand All @@ -8,15 +10,38 @@ use crate::prelude::*;
use crate::string::{FormatImplicitConcatenatedString, StringLikeExtensions};

#[derive(Default)]
pub struct FormatExprBytesLiteral;
pub struct FormatExprBytesLiteral {
layout: ExprBytesLiteralLayout,
}

#[derive(Default)]
pub struct ExprBytesLiteralLayout {
pub implicit_group_id: Option<GroupId>,
}

impl FormatRuleWithOptions<ExprBytesLiteral, PyFormatContext<'_>> for FormatExprBytesLiteral {
type Options = ExprBytesLiteralLayout;

fn with_options(mut self, options: Self::Options) -> Self {
self.layout = options;
self
}
}

impl FormatNodeRule<ExprBytesLiteral> for FormatExprBytesLiteral {
fn fmt_fields(&self, item: &ExprBytesLiteral, f: &mut PyFormatter) -> FormatResult<()> {
let ExprBytesLiteral { value, .. } = item;

match value.as_slice() {
[bytes_literal] => bytes_literal.format().fmt(f),
_ => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f),
_ => match self.layout.implicit_group_id {
Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
.with_group_id(Some(group_id))
.fmt(f),
None => {
in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f)
}
},
}
}
}
Expand Down
31 changes: 28 additions & 3 deletions crates/ruff_python_formatter/src/expression/expr_f_string.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ruff_formatter::{FormatRuleWithOptions, GroupId};
use ruff_python_ast::{AnyNodeRef, ExprFString, StringLike};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
Expand All @@ -10,7 +11,23 @@ use crate::prelude::*;
use crate::string::{FormatImplicitConcatenatedString, Quoting, StringLikeExtensions};

#[derive(Default)]
pub struct FormatExprFString;
pub struct FormatExprFString {
layout: ExprFStringLayout,
}

#[derive(Default)]
pub struct ExprFStringLayout {
pub implicit_group_id: Option<GroupId>,
}

impl FormatRuleWithOptions<ExprFString, PyFormatContext<'_>> for FormatExprFString {
type Options = ExprFStringLayout;

fn with_options(mut self, options: Self::Options) -> Self {
self.layout = options;
self
}
}

impl FormatNodeRule<ExprFString> for FormatExprFString {
fn fmt_fields(&self, item: &ExprFString, f: &mut PyFormatter) -> FormatResult<()> {
Expand All @@ -22,7 +39,14 @@ impl FormatNodeRule<ExprFString> for FormatExprFString {
f_string_quoting(item, &f.context().locator()),
)
.fmt(f),
_ => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f),
_ => match self.layout.implicit_group_id {
Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
.with_group_id(Some(group_id))
.fmt(f),
None => {
in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f)
}
},
}
}
}
Expand All @@ -35,6 +59,7 @@ impl NeedsParentheses for ExprFString {
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
}
// TODO(dhruvmanila): Ideally what we want here is a new variant which
// is something like:
// - If the expression fits by just adding the parentheses, then add them and
Expand All @@ -53,7 +78,7 @@ impl NeedsParentheses for ExprFString {
// ```
// This isn't decided yet, refer to the relevant discussion:
// https://github.com/astral-sh/ruff/discussions/9785
} else if StringLike::FString(self).is_multiline(context.source()) {
else if StringLike::FString(self).is_multiline(context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
Expand Down
41 changes: 32 additions & 9 deletions crates/ruff_python_formatter/src/expression/expr_string_literal.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ruff_formatter::FormatRuleWithOptions;
use ruff_formatter::{FormatRuleWithOptions, GroupId};
use ruff_python_ast::{AnyNodeRef, ExprStringLiteral, StringLike};

use crate::expression::parentheses::{
Expand All @@ -10,14 +10,29 @@ use crate::string::{FormatImplicitConcatenatedString, StringLikeExtensions};

#[derive(Default)]
pub struct FormatExprStringLiteral {
kind: StringLiteralKind,
layout: ExprStringLiteralLayout,
}

#[derive(Default)]
pub struct ExprStringLiteralLayout {
pub kind: StringLiteralKind,
pub implicit_group_id: Option<GroupId>,
}

impl ExprStringLiteralLayout {
pub const fn docstring() -> Self {
Self {
kind: StringLiteralKind::Docstring,
implicit_group_id: None,
}
}
}

impl FormatRuleWithOptions<ExprStringLiteral, PyFormatContext<'_>> for FormatExprStringLiteral {
type Options = StringLiteralKind;
type Options = ExprStringLiteralLayout;

fn with_options(mut self, options: Self::Options) -> Self {
self.kind = options;
self.layout = options;
self
}
}
Expand All @@ -27,15 +42,23 @@ impl FormatNodeRule<ExprStringLiteral> for FormatExprStringLiteral {
let ExprStringLiteral { value, .. } = item;

match value.as_slice() {
[string_literal] => string_literal.format().with_options(self.kind).fmt(f),
[string_literal] => string_literal
.format()
.with_options(self.layout.kind)
.fmt(f),
_ => {
// This is just a sanity check because [`DocstringStmt::try_from_statement`]
// ensures that the docstring is a *single* string literal.
assert!(!self.kind.is_docstring());
assert!(!self.layout.kind.is_docstring());

in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item))
match self.layout.implicit_group_id {
Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
.with_group_id(Some(group_id))
.fmt(f),
None => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item))
.fmt(f),
}
}
.fmt(f),
}
}
}
Expand All @@ -48,7 +71,7 @@ impl NeedsParentheses for ExprStringLiteral {
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
} else if StringLike::String(self).is_multiline(context.source()) {
} else if StringLike::from(self).is_multiline(context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
Expand Down
12 changes: 8 additions & 4 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,15 +768,18 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
Expr::StringLiteral(ast::ExprStringLiteral { value, .. })
if value.is_implicit_concatenated() =>
{
self.update_max_precedence(OperatorPrecedence::String);
// FIXME make this a preview only change
// self.update_max_precedence(OperatorPrecedence::String);
return;
}
Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. })
if value.is_implicit_concatenated() =>
{
self.update_max_precedence(OperatorPrecedence::String);
// self.update_max_precedence(OperatorPrecedence::String);
return;
}
Expr::FString(ast::ExprFString { value, .. }) if value.is_implicit_concatenated() => {
self.update_max_precedence(OperatorPrecedence::String);
// self.update_max_precedence(OperatorPrecedence::String);
return;
}

Expand Down Expand Up @@ -1254,8 +1257,9 @@ pub(crate) fn is_splittable_expression(expr: &Expr, context: &PyFormatContext) -
}

// String like literals can expand if they are implicit concatenated.
// TODO REVIEW
Expr::FString(fstring) => fstring.value.is_implicit_concatenated(),
Expr::StringLiteral(string) => string.value.is_implicit_concatenated(),
Expr::StringLiteral(_) => false,
Expr::BytesLiteral(bytes) => bytes.value.is_implicit_concatenated(),

// Expressions that have no split points per se, but they contain nested sub expressions that might expand.
Expand Down
19 changes: 10 additions & 9 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ mod tests {
use ruff_python_trivia::CommentRanges;
use ruff_text_size::{TextRange, TextSize};

use crate::{format_module_ast, format_module_source, format_range, PyFormatOptions};
use crate::{
format_module_ast, format_module_source, format_range, PreviewMode, PyFormatOptions,
};

/// Very basic test intentionally kept very similar to the CLI
#[test]
Expand Down Expand Up @@ -188,13 +190,11 @@ if True:
#[test]
fn quick_test() {
let source = r#"
def main() -> None:
if True:
some_very_long_variable_name_abcdefghijk = Foo()
some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
== "This is a very long string abcdefghijk"
]
fstring = (
f"We have to remember to escape {braces}."
" Like {these}."
f" But not {this}."
)
"#;
let source_type = PySourceType::Python;
Expand All @@ -203,7 +203,8 @@ def main() -> None:
let source_path = "code_inline.py";
let parsed = parse(source, source_type.as_mode()).unwrap();
let comment_ranges = CommentRanges::from(parsed.tokens());
let options = PyFormatOptions::from_extension(Path::new(source_path));
let options = PyFormatOptions::from_extension(Path::new(source_path))
.with_preview(PreviewMode::Enabled);
let formatted = format_module_ast(&parsed, &comment_ranges, source, options).unwrap();

// Uncomment the `dbg` to print the IR.
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_python_formatter/src/other/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ fn is_huggable_string_argument(
arguments: &Arguments,
context: &PyFormatContext,
) -> bool {
// TODO: Implicit concatenated could become regular string. Although not if it is multiline. So that should be fine?
// Double check
if string.is_implicit_concatenated() || !string.is_multiline(context.source()) {
return false;
}
Expand Down
11 changes: 3 additions & 8 deletions crates/ruff_python_formatter/src/other/f_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,9 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
let quotes = StringQuotes::from(string_kind);
write!(f, [string_kind.prefix(), quotes])?;

f.join()
.entries(
self.value
.elements
.iter()
.map(|element| FormatFStringElement::new(element, context)),
)
.finish()?;
for element in &self.value.elements {
FormatFStringElement::new(element, context).fmt(f)?;
}

// Ending quote
quotes.fmt(f)
Expand Down
10 changes: 4 additions & 6 deletions crates/ruff_python_formatter/src/other/f_string_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a> FormatFStringLiteralElement<'a> {
impl Format<PyFormatContext<'_>> for FormatFStringLiteralElement<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
let literal_content = f.context().locator().slice(self.element.range());
let normalized = normalize_string(literal_content, 0, self.context.flags(), true);
let normalized = normalize_string(literal_content, 0, self.context.flags(), true, false);
match &normalized {
Cow::Borrowed(_) => source_text_slice(self.element.range()).fmt(f),
Cow::Owned(normalized) => text(normalized).fmt(f),
Expand Down Expand Up @@ -231,11 +231,9 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
if let Some(format_spec) = format_spec.as_deref() {
token(":").fmt(f)?;

f.join()
.entries(format_spec.elements.iter().map(|element| {
FormatFStringElement::new(element, self.context.f_string())
}))
.finish()?;
for element in &format_spec.elements {
FormatFStringElement::new(element, self.context.f_string()).fmt(f)?;
}

// These trailing comments can only occur if the format specifier is
// present. For example,
Expand Down
16 changes: 10 additions & 6 deletions crates/ruff_python_formatter/src/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,18 +289,22 @@ impl<'a> CanOmitOptionalParenthesesVisitor<'a> {

Pattern::MatchValue(value) => match &*value.value {
Expr::StringLiteral(string) => {
self.update_max_precedence(OperatorPrecedence::String, string.value.len());
// TODO update?

// self.update_max_precedence(OperatorPrecedence::String, string.value.len());
}
Expr::BytesLiteral(bytes) => {
self.update_max_precedence(OperatorPrecedence::String, bytes.value.len());
// TODO update?
// self.update_max_precedence(OperatorPrecedence::String, bytes.value.len());
}
// F-strings are allowed according to python's grammar but fail with a syntax error at runtime.
// That's why we need to support them for formatting.
Expr::FString(string) => {
self.update_max_precedence(
OperatorPrecedence::String,
string.value.as_slice().len(),
);
// TODO update?
// self.update_max_precedence(
// OperatorPrecedence::String,
// string.value.as_slice().len(),
// );
}

Expr::NumberLiteral(_) | Expr::Attribute(_) | Expr::UnaryOp(_) => {
Expand Down
8 changes: 8 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,11 @@ pub(crate) fn is_docstring_code_block_in_docstring_indent_enabled(
) -> bool {
context.is_preview()
}

/// Returns `true` if implicitly concatenated strings should be joined if they all fit on a single line.
/// See [#9457](https://github.com/astral-sh/ruff/issues/9457)
/// WARNING: This preview style depends on `is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled`
/// because it relies on the new semantic of `IfBreaksParenthesized`.
pub(crate) fn is_join_implicit_concatenated_string_enabled(context: &PyFormatContext) -> bool {
context.is_preview()
}
Loading

0 comments on commit ae9c6a9

Please sign in to comment.