From 39beeb61f75938ce0ddcc40eec0dd468f6a2a1c1 Mon Sep 17 00:00:00 2001 From: konsti Date: Thu, 10 Aug 2023 09:19:27 +0200 Subject: [PATCH] Track formatting all comments We currently don't format all comments as match statements are not yet implemented. We can work around this for the top level match statement by setting them manually formatted but the mocked-out top level match doesn't call into its children so they would still have unformatted comments --- .../ruff_python_formatter/src/comments/mod.rs | 31 +++++++++ .../src/expression/expr_formatted_value.rs | 8 +-- crates/ruff_python_formatter/src/lib.rs | 63 +++++++++++++------ .../src/other/match_case.rs | 14 ++++- 4 files changed, 91 insertions(+), 25 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 141a7e2c3e..ae855c82b6 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -101,6 +101,7 @@ pub(crate) use format::{ }; use ruff_formatter::{SourceCode, SourceCodeSlice}; use ruff_python_ast::node::AnyNodeRef; +use ruff_python_ast::visitor::preorder::{PreorderVisitor, TraversalSignal}; use ruff_python_index::CommentRanges; use crate::comments::debug::{DebugComment, DebugComments}; @@ -401,6 +402,24 @@ impl<'a> Comments<'a> { ); } + #[inline(always)] + #[cfg(not(debug_assertions))] + pub(crate) fn mark_verbatim_node_comments_formatted(&self, node: AnyNodeRef) {} + + /// Marks the comments of a node printed in verbatim (suppressed) as formatted. + /// + /// Marks the dangling comments and the comments of all descendants as formatted. + /// The leading and trailing comments are not marked as formatted because they are formatted + /// normally if `node` is the first or last node of a suppression range. + #[cfg(debug_assertions)] + pub(crate) fn mark_verbatim_node_comments_formatted(&self, node: AnyNodeRef) { + for dangling in self.dangling_comments(node) { + dangling.mark_formatted(); + } + + node.visit_preorder(&mut MarkVerbatimCommentsAsFormattedVisitor(self)); + } + /// Returns an object that implements [Debug] for nicely printing the [`Comments`]. pub(crate) fn debug(&'a self, source_code: SourceCode<'a>) -> DebugComments<'a> { DebugComments::new(&self.data.comments, source_code) @@ -412,6 +431,18 @@ struct CommentsData<'a> { comments: CommentsMap<'a>, } +struct MarkVerbatimCommentsAsFormattedVisitor<'a>(&'a Comments<'a>); + +impl<'a> PreorderVisitor<'a> for MarkVerbatimCommentsAsFormattedVisitor<'a> { + fn enter_node(&mut self, node: AnyNodeRef<'a>) -> TraversalSignal { + for comment in self.0.leading_dangling_trailing_comments(node) { + comment.mark_formatted(); + } + + TraversalSignal::Traverse + } +} + #[cfg(test)] mod tests { use insta::assert_debug_snapshot; diff --git a/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs b/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs index 34b7fb1f65..12c1a8e0e5 100644 --- a/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs +++ b/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs @@ -1,7 +1,7 @@ use crate::context::PyFormatContext; use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses}; -use crate::{not_yet_implemented, FormatNodeRule, PyFormatter}; -use ruff_formatter::{write, Buffer, FormatResult}; +use crate::{FormatNodeRule, PyFormatter}; +use ruff_formatter::FormatResult; use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::ExprFormattedValue; @@ -9,8 +9,8 @@ use ruff_python_ast::ExprFormattedValue; pub struct FormatExprFormattedValue; impl FormatNodeRule for FormatExprFormattedValue { - fn fmt_fields(&self, item: &ExprFormattedValue, f: &mut PyFormatter) -> FormatResult<()> { - write!(f, [not_yet_implemented(item)]) + fn fmt_fields(&self, _item: &ExprFormattedValue, _f: &mut PyFormatter) -> FormatResult<()> { + unreachable!("Handled inside of `FormatExprFString"); } } diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index 0ade2fad54..5e3213ae95 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -12,7 +12,7 @@ use ruff_formatter::{ PrintError, }; use ruff_formatter::{Formatted, Printed, SourceCode}; -use ruff_python_ast::node::{AnyNodeRef, AstNode, NodeKind}; +use ruff_python_ast::node::{AnyNodeRef, AstNode}; use ruff_python_ast::{Mod, Ranged}; use ruff_python_index::{CommentRanges, CommentRangesBuilder}; use ruff_python_parser::lexer::{lex, LexicalError}; @@ -150,25 +150,30 @@ pub fn format_node<'a>( let locator = Locator::new(source); - format!( + let formatted = format!( PyFormatContext::new(options, locator.contents(), comments), [root.format()] - ) + )?; + formatted + .context() + .comments() + .assert_formatted_all_comments(SourceCode::new(source)); + Ok(formatted) } -pub(crate) struct NotYetImplemented(NodeKind); +pub(crate) struct NotYetImplemented<'a>(AnyNodeRef<'a>); /// Formats a placeholder for nodes that have not yet been implemented -pub(crate) fn not_yet_implemented<'a, T>(node: T) -> NotYetImplemented +pub(crate) fn not_yet_implemented<'a, T>(node: T) -> NotYetImplemented<'a> where T: Into>, { - NotYetImplemented(node.into().kind()) + NotYetImplemented(node.into()) } -impl Format> for NotYetImplemented { +impl Format> for NotYetImplemented<'_> { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { - let text = std::format!("NOT_YET_IMPLEMENTED_{:?}", self.0); + let text = std::format!("NOT_YET_IMPLEMENTED_{:?}", self.0.kind()); f.write_element(FormatElement::Tag(Tag::StartVerbatim( tag::VerbatimKind::Verbatim { @@ -181,31 +186,51 @@ impl Format> for NotYetImplemented { })?; f.write_element(FormatElement::Tag(Tag::EndVerbatim))?; + + f.context() + .comments() + .mark_verbatim_node_comments_formatted(self.0); + Ok(()) } } -pub(crate) struct NotYetImplementedCustomText(&'static str); - -/// Formats a placeholder for nodes that have not yet been implemented -#[allow(dead_code)] -pub(crate) const fn not_yet_implemented_custom_text( +pub(crate) struct NotYetImplementedCustomText<'a> { text: &'static str, -) -> NotYetImplementedCustomText { - NotYetImplementedCustomText(text) + node: AnyNodeRef<'a>, } -impl Format> for NotYetImplementedCustomText { +/// Formats a placeholder for nodes that have not yet been implemented +pub(crate) fn not_yet_implemented_custom_text<'a, T>( + text: &'static str, + node: T, +) -> NotYetImplementedCustomText<'a> +where + T: Into>, +{ + NotYetImplementedCustomText { + text, + node: node.into(), + } +} + +impl Format> for NotYetImplementedCustomText<'_> { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { f.write_element(FormatElement::Tag(Tag::StartVerbatim( tag::VerbatimKind::Verbatim { - length: self.0.text_len(), + length: self.text.text_len(), }, )))?; - text(self.0).fmt(f)?; + text(self.text).fmt(f)?; - f.write_element(FormatElement::Tag(Tag::EndVerbatim)) + f.write_element(FormatElement::Tag(Tag::EndVerbatim))?; + + f.context() + .comments() + .mark_verbatim_node_comments_formatted(self.node); + + Ok(()) } } diff --git a/crates/ruff_python_formatter/src/other/match_case.rs b/crates/ruff_python_formatter/src/other/match_case.rs index 0bbe0572bc..f2dc80f93a 100644 --- a/crates/ruff_python_formatter/src/other/match_case.rs +++ b/crates/ruff_python_formatter/src/other/match_case.rs @@ -14,7 +14,7 @@ impl FormatNodeRule for FormatMatchCase { fn fmt_fields(&self, item: &MatchCase, f: &mut PyFormatter) -> FormatResult<()> { let MatchCase { range: _, - pattern: _, + pattern, guard, body, } = item; @@ -24,7 +24,17 @@ impl FormatNodeRule for FormatMatchCase { [ text("case"), space(), - not_yet_implemented_custom_text("NOT_YET_IMPLEMENTED_Pattern"), + format_with(|f: &mut PyFormatter| { + let comments = f.context().comments(); + + for comment in comments.leading_trailing_comments(pattern) { + // This is a lie, but let's go with it. + comment.mark_formatted(); + } + + // Replace the whole `format_with` with `pattern.format()` once pattern formatting is implemented. + not_yet_implemented_custom_text("NOT_YET_IMPLEMENTED_Pattern", pattern).fmt(f) + }), ] )?;