Skip to content
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions crates/ruff_benchmark/benches/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ruff_benchmark::{
};
use ruff_python_formatter::{PreviewMode, PyFormatOptions, format_module_ast};
use ruff_python_parser::{Mode, ParseOptions, parse};
use ruff_python_trivia::CommentRanges;
use ruff_python_trivia::TriviaRanges;

#[cfg(target_os = "windows")]
#[global_allocator]
Expand Down Expand Up @@ -53,11 +53,11 @@ fn benchmark_formatter(criterion: &mut Criterion) {
.expect("Input should be a valid Python code");

b.iter(|| {
let comment_ranges = CommentRanges::from(parsed.tokens());
let trivia_ranges = TriviaRanges::from(parsed.tokens());
let options = PyFormatOptions::from_extension(Path::new(case.name()))
.with_preview(PreviewMode::Enabled);
let formatted =
format_module_ast(&parsed, &comment_ranges, case.code(), options)
format_module_ast(&parsed, &trivia_ranges, case.code(), options)
.expect("Formatting to succeed");

formatted.print().expect("Printing to succeed")
Expand Down
50 changes: 49 additions & 1 deletion crates/ruff_python_ast/src/token/tokens.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::{iter::FusedIterator, ops::Deref};

use super::{Token, TokenKind};
use ruff_python_trivia::CommentRanges;
use ruff_python_trivia::{CommentRanges, ParenthesizedExpressions, TriviaRanges};
use ruff_text_size::{Ranged as _, TextRange, TextSize};
use rustc_hash::FxHashSet;

/// Tokens represents a vector of lexed [`Token`].
#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -279,15 +280,62 @@ impl FusedIterator for TokenAt {}
impl From<&Tokens> for CommentRanges {
fn from(tokens: &Tokens) -> Self {
let mut ranges = vec![];

for token in tokens {
if token.kind() == TokenKind::Comment {
ranges.push(token.range());
}
}

CommentRanges::new(ranges)
}
}

impl From<&Tokens> for TriviaRanges {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for expanding scope, it's completely fine not to do this in this PR. Should we also replace the function that we use in the linter to use the new trivia ranges?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good question, I think that should be a separate change.

fn from(tokens: &Tokens) -> Self {
let mut comments = vec![];
let mut parenthesized = FxHashSet::default();
let mut stack = Vec::<Option<TextSize>>::new();
let mut previous_end = None;

for token in tokens {
if token.kind() == TokenKind::Comment {
comments.push(token.range());
}

if token.kind().is_trivia() {
continue;
}

match token.kind() {
TokenKind::Lpar => {
if let Some(start) = stack.last_mut() {
start.get_or_insert(token.start());
}
stack.push(None);
}
TokenKind::Rpar => {
if let (Some(Some(start)), Some(end)) = (stack.pop(), previous_end) {
parenthesized.insert(TextRange::new(start, end));
}
}
_ => {

@MichaReiser MichaReiser Jun 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This opens an interesting question. Is it intentional that we set start if the current token is a comment?

Edit: We actually don't do this... I should not review code this late :)

if let Some(start) = stack.last_mut() {
start.get_or_insert(token.start());
}
}
}

previous_end = Some(token.end());
}

TriviaRanges::new(
CommentRanges::new(comments),
ParenthesizedExpressions::new(parenthesized),
)
}
}

/// An iterator over the [`Token`]s with context.
///
/// This struct is created by the [`iter_with_context`] method on [`Tokens`]. Refer to its
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_python_formatter/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::{Parser, ValueEnum};
use ruff_formatter::SourceCode;
use ruff_python_ast::{PySourceType, PythonVersion};
use ruff_python_parser::{ParseOptions, parse};
use ruff_python_trivia::CommentRanges;
use ruff_python_trivia::TriviaRanges;
use ruff_text_size::Ranged;

use crate::comments::collect_comments;
Expand Down Expand Up @@ -70,15 +70,15 @@ pub fn format_and_debug_print(source: &str, cli: &Cli, source_path: &Path) -> Re
.with_target_version(cli.target_version);

let source_code = SourceCode::new(source);
let comment_ranges = CommentRanges::from(parsed.tokens());
let formatted = format_module_ast(&parsed, &comment_ranges, source, options)
.context("Failed to format node")?;
let trivia = TriviaRanges::from(parsed.tokens());
let formatted =
format_module_ast(&parsed, &trivia, source, options).context("Failed to format node")?;
if cli.print_ir {
println!("{}", formatted.document().display(source_code));
}
if cli.print_comments {
// Print preceding, following and enclosing nodes
let decorated_comments = collect_comments(parsed.syntax(), source_code, &comment_ranges);
let decorated_comments = collect_comments(parsed.syntax(), source_code, trivia.comments());
if !decorated_comments.is_empty() {
println!("# Comment decoration: Range, Preceding, Following, Enclosing, Comment");
}
Expand Down
5 changes: 2 additions & 3 deletions crates/ruff_python_formatter/src/comments/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ mod tests {
use ruff_formatter::SourceCode;
use ruff_python_ast::{AnyNodeRef, AtomicNodeIndex};
use ruff_python_ast::{StmtBreak, StmtContinue};
use ruff_python_trivia::{CommentLinePosition, CommentRanges};
use ruff_python_trivia::CommentLinePosition;
use ruff_text_size::{TextRange, TextSize};

use crate::comments::map::MultiMap;
Expand Down Expand Up @@ -238,8 +238,7 @@ break;
),
);

let comment_ranges = CommentRanges::default();
let comments = Comments::new(comments_map, &comment_ranges);
let comments = Comments::new(comments_map);

assert_debug_snapshot!(comments.debug(source_code));
}
Expand Down
51 changes: 18 additions & 33 deletions crates/ruff_python_formatter/src/comments/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub(crate) use format::{
};
use ruff_formatter::{SourceCode, SourceCodeSlice};
use ruff_python_ast::AnyNodeRef;
use ruff_python_trivia::{CommentLinePosition, CommentRanges, SuppressionKind};
use ruff_python_trivia::{CommentLinePosition, SuppressionKind, TriviaRanges};
use ruff_text_size::{Ranged, TextRange};
pub(crate) use visitor::collect_comments;

Expand Down Expand Up @@ -219,53 +219,41 @@ pub(crate) struct Comments<'a> {
}

impl<'a> Comments<'a> {
fn new(comments: CommentsMap<'a>, comment_ranges: &'a CommentRanges) -> Self {
fn new(comments: CommentsMap<'a>) -> Self {
Self {
data: Rc::new(CommentsData {
comments,
comment_ranges,
}),
data: Rc::new(CommentsData { comments }),
}
}

/// Effectively a [`Default`] implementation that works around the lifetimes for tests
/// Creates an empty comment map for tests.
#[cfg(test)]
pub(crate) fn from_ranges(comment_ranges: &'a CommentRanges) -> Self {
Self {
data: Rc::new(CommentsData {
comments: CommentsMap::default(),
comment_ranges,
}),
}
}

pub(crate) fn ranges(&self) -> &'a CommentRanges {
self.data.comment_ranges
pub(crate) fn empty() -> Self {
Self::new(CommentsMap::default())
}

/// Extracts the comments from the AST.
pub(crate) fn from_ast(
root: impl Into<AnyNodeRef<'a>>,
source_code: SourceCode<'a>,
comment_ranges: &'a CommentRanges,
trivia: &'a TriviaRanges,
) -> Self {
fn collect_comments<'a>(
root: AnyNodeRef<'a>,
source_code: SourceCode<'a>,
comment_ranges: &'a CommentRanges,
trivia: &'a TriviaRanges,
) -> Comments<'a> {
let map = if comment_ranges.is_empty() {
let map = if trivia.comments().is_empty() {
CommentsMap::new()
} else {
let mut builder = CommentsMapBuilder::new(source_code.as_str(), comment_ranges);
CommentsVisitor::new(source_code, comment_ranges, &mut builder).visit(root);
let mut builder = CommentsMapBuilder::new(source_code.as_str(), trivia);
CommentsVisitor::new(source_code, trivia.comments(), &mut builder).visit(root);
builder.finish()
};

Comments::new(map, comment_ranges)
Comments::new(map)
}

collect_comments(root.into(), source_code, comment_ranges)
collect_comments(root.into(), source_code, trivia)
}

/// Returns `true` if the given `node` has any comments.
Expand Down Expand Up @@ -496,9 +484,6 @@ impl LeadingDanglingTrailingComments<'_> {
#[derive(Debug)]
struct CommentsData<'a> {
comments: CommentsMap<'a>,

/// We need those for backwards lexing
comment_ranges: &'a CommentRanges,
}

pub(crate) fn has_skip_comment(trailing_comments: &[SourceComment], source: &str) -> bool {
Expand All @@ -518,13 +503,13 @@ mod tests {
use ruff_formatter::SourceCode;
use ruff_python_ast::{Mod, PySourceType};
use ruff_python_parser::{ParseOptions, Parsed, parse};
use ruff_python_trivia::CommentRanges;
use ruff_python_trivia::TriviaRanges;

use crate::comments::Comments;

struct CommentsTestCase<'a> {
parsed: Parsed<Mod>,
comment_ranges: CommentRanges,
trivia: TriviaRanges,
source_code: SourceCode<'a>,
}

Expand All @@ -534,17 +519,17 @@ mod tests {
let source_type = PySourceType::Python;
let parsed = parse(source, ParseOptions::from(source_type))
.expect("Expect source to be valid Python");
let comment_ranges = CommentRanges::from(parsed.tokens());
let trivia = TriviaRanges::from(parsed.tokens());

CommentsTestCase {
parsed,
comment_ranges,
trivia,
source_code,
}
}

fn to_comments(&self) -> Comments<'_> {
Comments::from_ast(self.parsed.syntax(), self.source_code, &self.comment_ranges)
Comments::from_ast(self.parsed.syntax(), self.source_code, &self.trivia)
}
}

Expand Down
Loading
Loading