Skip to content

Commit

Permalink
Memoize text width
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Aug 14, 2023
1 parent 493a3b6 commit 9b9db1b
Show file tree
Hide file tree
Showing 10 changed files with 277 additions and 123 deletions.
22 changes: 17 additions & 5 deletions crates/ruff_formatter/src/arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ mod tests {

#[test]
fn test_nesting() {
let mut context = FormatState::new(());
let mut context = FormatState::new(SimpleFormatContext::default());
let mut buffer = VecBuffer::new(&mut context);

write!(
Expand All @@ -151,14 +151,26 @@ mod tests {
assert_eq!(
buffer.into_vec(),
vec![
FormatElement::StaticText { text: "function" },
FormatElement::StaticText {
text: "function",
text_width: TextWidth::new_width(8)
},
FormatElement::Space,
FormatElement::StaticText { text: "a" },
FormatElement::StaticText {
text: "a",
text_width: TextWidth::new_width(1)
},
FormatElement::Space,
// Group
FormatElement::Tag(Tag::StartGroup(tag::Group::new())),
FormatElement::StaticText { text: "(" },
FormatElement::StaticText { text: ")" },
FormatElement::StaticText {
text: "(",
text_width: TextWidth::new_width(1)
},
FormatElement::StaticText {
text: ")",
text_width: TextWidth::new_width(1)
},
FormatElement::Tag(Tag::EndGroup)
]
);
Expand Down
35 changes: 23 additions & 12 deletions crates/ruff_formatter/src/builders.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::format_element::tag::{Condition, Tag};
use crate::prelude::tag::{DedentMode, GroupMode, LabelId};
use crate::prelude::*;
use crate::{format_element, write, Argument, Arguments, FormatContext, GroupId, TextSize};
use crate::{
format_element, write, Argument, Arguments, FormatContext, FormatOptions, GroupId, TextSize,
};
use crate::{Buffer, VecBuffer};

use ruff_text_size::TextRange;
Expand Down Expand Up @@ -264,9 +266,15 @@ pub struct StaticText {
text: &'static str,
}

impl<Context> Format<Context> for StaticText {
impl<Context> Format<Context> for StaticText
where
Context: FormatContext,
{
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
f.write_element(FormatElement::StaticText { text: self.text })
f.write_element(FormatElement::StaticText {
text: self.text,
text_width: TextWidth::from_text(self.text, f.options().tab_width()),
})
}
}

Expand Down Expand Up @@ -343,14 +351,18 @@ pub struct DynamicText<'a> {
position: Option<TextSize>,
}

impl<Context> Format<Context> for DynamicText<'_> {
impl<Context> Format<Context> for DynamicText<'_>
where
Context: FormatContext,
{
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
if let Some(source_position) = self.position {
f.write_element(FormatElement::SourcePosition(source_position))?;
}

f.write_element(FormatElement::DynamicText {
text: self.text.to_string().into_boxed_str(),
text_width: TextWidth::from_text(self.text, f.options().tab_width()),
})
}
}
Expand Down Expand Up @@ -398,28 +410,27 @@ where
let slice = source_code.slice(self.range);
debug_assert_no_newlines(slice.text(source_code));

let contains_newlines = match self.new_lines {
let text_width = match self.new_lines {
ContainsNewlines::Yes => {
debug_assert!(
slice.text(source_code).contains('\n'),
"Text contains no new line characters but the caller specified that it does."
);
true
TextWidth::Multiline
}
ContainsNewlines::No => {
debug_assert!(
!slice.text(source_code).contains('\n'),
"Text contains new line characters but the caller specified that it does not."
);
false
TextWidth::from_text(slice.text(source_code), f.context().options().tab_width())
}
ContainsNewlines::Detect => {
TextWidth::from_text(slice.text(source_code), f.context().options().tab_width())
}
ContainsNewlines::Detect => slice.text(source_code).contains('\n'),
};

f.write_element(FormatElement::SourceCodeSlice {
slice,
contains_newlines,
})
f.write_element(FormatElement::SourceCodeSlice { slice, text_width })
}
}

Expand Down
77 changes: 61 additions & 16 deletions crates/ruff_formatter/src/format_element.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ pub mod tag;

use std::borrow::Cow;
use std::hash::{Hash, Hasher};
use std::num::NonZeroU32;
use std::ops::Deref;
use std::rc::Rc;
use unicode_width::UnicodeWidthChar;

use crate::format_element::tag::{GroupMode, LabelId, Tag};
use crate::source_code::SourceCodeSlice;
use crate::TagKind;
use crate::{TabWidth, TagKind};
use ruff_text_size::TextSize;

/// Language agnostic IR for formatting source code.
Expand All @@ -31,20 +33,23 @@ pub enum FormatElement {
SourcePosition(TextSize),

/// Token constructed by the formatter from a static string
StaticText { text: &'static str },
StaticText {
text: &'static str,
text_width: TextWidth,
},

/// Token constructed from the input source as a dynamic
/// string.
DynamicText {
/// There's no need for the text to be mutable, using `Box<str>` safes 8 bytes over `String`.
text: Box<str>,
text_width: TextWidth,
},

/// Text that gets emitted as it is in the source code. Optimized to avoid any allocations.
SourceCodeSlice {
slice: SourceCodeSlice,
/// Whether the string contains any new line characters
contains_newlines: bool,
text_width: TextWidth,
},

/// Prevents that line suffixes move past this boundary. Forces the printer to print any pending
Expand All @@ -69,19 +74,18 @@ impl std::fmt::Debug for FormatElement {
FormatElement::Space => write!(fmt, "Space"),
FormatElement::Line(mode) => fmt.debug_tuple("Line").field(mode).finish(),
FormatElement::ExpandParent => write!(fmt, "ExpandParent"),
FormatElement::StaticText { text } => {
fmt.debug_tuple("StaticText").field(text).finish()
}
FormatElement::StaticText { text, text_width } => fmt
.debug_tuple("StaticText")
.field(text)
.field(text_width)
.finish(),
FormatElement::DynamicText { text, .. } => {
fmt.debug_tuple("DynamicText").field(text).finish()
}
FormatElement::SourceCodeSlice {
slice,
contains_newlines,
} => fmt
FormatElement::SourceCodeSlice { slice, text_width } => fmt
.debug_tuple("Text")
.field(slice)
.field(contains_newlines)
.field(text_width)
.finish(),
FormatElement::LineSuffixBoundary => write!(fmt, "LineSuffixBoundary"),
FormatElement::BestFitting { variants } => fmt
Expand Down Expand Up @@ -256,11 +260,12 @@ impl FormatElements for FormatElement {
FormatElement::ExpandParent => true,
FormatElement::Tag(Tag::StartGroup(group)) => !group.mode().is_flat(),
FormatElement::Line(line_mode) => matches!(line_mode, LineMode::Hard | LineMode::Empty),
FormatElement::StaticText { text } => text.contains('\n'),
FormatElement::StaticText {
text: _,
text_width,
} => text_width.is_multiline(),
FormatElement::DynamicText { text, .. } => text.contains('\n'),
FormatElement::SourceCodeSlice {
contains_newlines, ..
} => *contains_newlines,
FormatElement::SourceCodeSlice { text_width, .. } => text_width.is_multiline(),
FormatElement::Interned(interned) => interned.will_break(),
// Traverse into the most flat version because the content is guaranteed to expand when even
// the most flat version contains some content that forces a break.
Expand Down Expand Up @@ -380,6 +385,46 @@ pub trait FormatElements {
fn end_tag(&self, kind: TagKind) -> Option<&Tag>;
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum TextWidth {
Width(NonZeroU32),
Multiline,
}

impl TextWidth {
pub(crate) const fn new_width(width: u32) -> Self {
// SAFETY: 1 + x is guaranteed to be non zero
#[allow(unsafe_code)]
TextWidth::Width(unsafe { NonZeroU32::new_unchecked(width.saturating_add(1)) })
}

pub(crate) fn from_text(text: &str, tab_width: TabWidth) -> TextWidth {
let mut width = 0u32;

for c in text.chars() {
let char_width = match c {
'\t' => tab_width.value(),
'\n' => return TextWidth::Multiline,
c => c.width().unwrap_or(0) as u32,
};
width += char_width as u32;
}

Self::new_width(width)
}

pub(crate) const fn width(self) -> Option<u32> {
match self {
TextWidth::Width(width) => Some(width.get() - 1),
TextWidth::Multiline => None,
}
}

pub(crate) const fn is_multiline(self) -> bool {
matches!(self, TextWidth::Multiline)
}
}

#[cfg(test)]
mod tests {

Expand Down
45 changes: 31 additions & 14 deletions crates/ruff_formatter/src/format_element/document.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::prelude::tag::GroupMode;
use crate::prelude::*;
use crate::printer::LineEnding;
use crate::source_code::SourceCode;
use crate::{format, write};
use crate::{format, write, TabWidth};
use crate::{
BufferExtensions, Format, FormatContext, FormatElement, FormatOptions, FormatResult, Formatter,
IndentStyle, LineWidth, PrinterOptions,
Expand Down Expand Up @@ -103,11 +103,12 @@ impl Document {
expands = false;
continue;
}
FormatElement::StaticText { text } => text.contains('\n'),
FormatElement::StaticText {
text: _,
text_width,
} => text_width.is_multiline(),
FormatElement::DynamicText { text, .. } => text.contains('\n'),
FormatElement::SourceCodeSlice {
contains_newlines, ..
} => *contains_newlines,
FormatElement::SourceCodeSlice { text_width, .. } => text_width.is_multiline(),
FormatElement::ExpandParent
| FormatElement::Line(LineMode::Hard | LineMode::Empty) => true,
_ => false,
Expand Down Expand Up @@ -211,13 +212,17 @@ impl FormatOptions for IrFormatOptions {
IndentStyle::Space(2)
}

fn tab_width(&self) -> TabWidth {
TabWidth::default()
}

fn line_width(&self) -> LineWidth {
LineWidth(80)
}

fn as_print_options(&self) -> PrinterOptions {
PrinterOptions {
tab_width: 2,
tab_width: self.tab_width(),
print_width: self.line_width().into(),
line_ending: LineEnding::LineFeed,
indent_style: IndentStyle::Space(2),
Expand Down Expand Up @@ -255,18 +260,21 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
element: &FormatElement,
f: &mut Formatter<IrFormatContext>,
) -> FormatResult<()> {
let text = match element {
FormatElement::StaticText { text } => text,
FormatElement::DynamicText { text } => text.as_ref(),
FormatElement::SourceCodeSlice { slice, .. } => {
slice.text(f.context().source_code())
let (text, text_width) = match element {
FormatElement::StaticText { text, text_width } => (*text, *text_width),
FormatElement::DynamicText { text, text_width } => {
(text.as_ref(), *text_width)
}
FormatElement::SourceCodeSlice { slice, text_width } => {
(slice.text(f.context().source_code()), *text_width)
}
_ => unreachable!(),
};

if text.contains('"') {
f.write_element(FormatElement::DynamicText {
text: text.replace('"', r#"\""#).into(),
text_width,
})
} else {
f.write_element(element.clone())
Expand Down Expand Up @@ -876,16 +884,25 @@ mod tests {
use Tag::*;

let document = Document::from(vec![
FormatElement::StaticText { text: "[" },
FormatElement::StaticText {
text: "[",
text_width: TextWidth::new_width(1),
},
FormatElement::Tag(StartGroup(tag::Group::new())),
FormatElement::Tag(StartIndent),
FormatElement::Line(LineMode::Soft),
FormatElement::StaticText { text: "a" },
FormatElement::StaticText {
text: "a",
text_width: TextWidth::new_width(1),
},
// Close group instead of indent
FormatElement::Tag(EndGroup),
FormatElement::Line(LineMode::Soft),
FormatElement::Tag(EndIndent),
FormatElement::StaticText { text: "]" },
FormatElement::StaticText {
text: "]",
text_width: TextWidth::new_width(1),
},
// End tag without start
FormatElement::Tag(EndIndent),
// Start tag without an end
Expand Down
Loading

0 comments on commit 9b9db1b

Please sign in to comment.