Consistently name comment own line/end-of-line line_position() (#5215)

## Summary

Previously, `DecoratedComment` used `text_position()` and
`SourceComment` used `position()`. This PR unifies this to
`line_position` everywhere.

## Test Plan

This is a rename refactoring.
This commit is contained in:
konstin 2023-06-21 11:04:56 +02:00 committed by GitHub
parent 1336ca601b
commit db301c14bd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 53 additions and 52 deletions

View file

@ -26,7 +26,7 @@ impl Debug for DebugComment<'_> {
strut strut
.field("text", &self.comment.slice.text(self.source_code)) .field("text", &self.comment.slice.text(self.source_code))
.field("position", &self.comment.position); .field("position", &self.comment.line_position);
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
strut.field("formatted", &self.comment.formatted.get()); strut.field("formatted", &self.comment.formatted.get());
@ -177,7 +177,7 @@ impl Debug for DebugNodeCommentSlice<'_> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::comments::map::MultiMap; use crate::comments::map::MultiMap;
use crate::comments::{CommentTextPosition, Comments, CommentsMap, SourceComment}; use crate::comments::{CommentLinePosition, Comments, CommentsMap, SourceComment};
use insta::assert_debug_snapshot; use insta::assert_debug_snapshot;
use ruff_formatter::SourceCode; use ruff_formatter::SourceCode;
use ruff_python_ast::node::AnyNode; use ruff_python_ast::node::AnyNode;
@ -208,7 +208,7 @@ break;
continue_statement.as_ref().into(), continue_statement.as_ref().into(),
SourceComment::new( SourceComment::new(
source_code.slice(TextRange::at(TextSize::new(0), TextSize::new(17))), source_code.slice(TextRange::at(TextSize::new(0), TextSize::new(17))),
CommentTextPosition::OwnLine, CommentLinePosition::OwnLine,
), ),
); );
@ -216,7 +216,7 @@ break;
continue_statement.as_ref().into(), continue_statement.as_ref().into(),
SourceComment::new( SourceComment::new(
source_code.slice(TextRange::at(TextSize::new(28), TextSize::new(10))), source_code.slice(TextRange::at(TextSize::new(28), TextSize::new(10))),
CommentTextPosition::EndOfLine, CommentLinePosition::EndOfLine,
), ),
); );
@ -224,7 +224,7 @@ break;
break_statement.as_ref().into(), break_statement.as_ref().into(),
SourceComment::new( SourceComment::new(
source_code.slice(TextRange::at(TextSize::new(39), TextSize::new(15))), source_code.slice(TextRange::at(TextSize::new(39), TextSize::new(15))),
CommentTextPosition::OwnLine, CommentLinePosition::OwnLine,
), ),
); );

View file

@ -136,7 +136,7 @@ impl Format<PyFormatContext<'_>> for FormatTrailingComments<'_> {
{ {
let slice = trailing.slice(); let slice = trailing.slice();
has_trailing_own_line_comment |= trailing.position().is_own_line(); has_trailing_own_line_comment |= trailing.line_position().is_own_line();
if has_trailing_own_line_comment { if has_trailing_own_line_comment {
let lines_before_comment = lines_before(slice.start(), f.context().contents()); let lines_before_comment = lines_before(slice.start(), f.context().contents());
@ -208,7 +208,7 @@ impl Format<PyFormatContext<'_>> for FormatDanglingComments<'_> {
.iter() .iter()
.filter(|comment| comment.is_unformatted()) .filter(|comment| comment.is_unformatted())
{ {
if first && comment.position().is_end_of_line() { if first && comment.line_position().is_end_of_line() {
write!(f, [space(), space()])?; write!(f, [space(), space()])?;
} }

View file

@ -117,14 +117,14 @@ pub(crate) struct SourceComment {
slice: SourceCodeSlice, slice: SourceCodeSlice,
/// Whether the comment has been formatted or not. /// Whether the comment has been formatted or not.
formatted: Cell<bool>, formatted: Cell<bool>,
position: CommentTextPosition, line_position: CommentLinePosition,
} }
impl SourceComment { impl SourceComment {
fn new(slice: SourceCodeSlice, position: CommentTextPosition) -> Self { fn new(slice: SourceCodeSlice, position: CommentLinePosition) -> Self {
Self { Self {
slice, slice,
position, line_position: position,
formatted: Cell::new(false), formatted: Cell::new(false),
} }
} }
@ -135,8 +135,8 @@ impl SourceComment {
&self.slice &self.slice
} }
pub(crate) const fn position(&self) -> CommentTextPosition { pub(crate) const fn line_position(&self) -> CommentLinePosition {
self.position self.line_position
} }
/// Marks the comment as formatted /// Marks the comment as formatted
@ -163,7 +163,7 @@ impl SourceComment {
/// The position of a comment in the source text. /// The position of a comment in the source text.
#[derive(Debug, Copy, Clone, Eq, PartialEq)] #[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub(crate) enum CommentTextPosition { pub(crate) enum CommentLinePosition {
/// A comment that is on the same line as the preceding token and is separated by at least one line break from the following token. /// A comment that is on the same line as the preceding token and is separated by at least one line break from the following token.
/// ///
/// # Examples /// # Examples
@ -176,7 +176,7 @@ pub(crate) enum CommentTextPosition {
/// ``` /// ```
/// ///
/// `# comment` is an end of line comments because it is separated by at least one line break from the following token `b`. /// `# comment` is an end of line comments because it is separated by at least one line break from the following token `b`.
/// Comments that not only end, but also start on a new line are [`OwnLine`](CommentTextPosition::OwnLine) comments. /// Comments that not only end, but also start on a new line are [`OwnLine`](CommentLinePosition::OwnLine) comments.
EndOfLine, EndOfLine,
/// A Comment that is separated by at least one line break from the preceding token. /// A Comment that is separated by at least one line break from the preceding token.
@ -193,13 +193,13 @@ pub(crate) enum CommentTextPosition {
OwnLine, OwnLine,
} }
impl CommentTextPosition { impl CommentLinePosition {
pub(crate) const fn is_own_line(self) -> bool { pub(crate) const fn is_own_line(self) -> bool {
matches!(self, CommentTextPosition::OwnLine) matches!(self, CommentLinePosition::OwnLine)
} }
pub(crate) const fn is_end_of_line(self) -> bool { pub(crate) const fn is_end_of_line(self) -> bool {
matches!(self, CommentTextPosition::EndOfLine) matches!(self, CommentLinePosition::EndOfLine)
} }
} }
@ -335,7 +335,7 @@ impl<'a> Comments<'a> {
{ {
self.trailing_comments(node) self.trailing_comments(node)
.iter() .iter()
.any(|comment| comment.position().is_own_line()) .any(|comment| comment.line_position().is_own_line())
} }
/// Returns an iterator over the [leading](self#leading-comments) and [trailing comments](self#trailing-comments) of `node`. /// Returns an iterator over the [leading](self#leading-comments) and [trailing comments](self#trailing-comments) of `node`.

View file

@ -9,7 +9,7 @@ use ruff_python_ast::whitespace;
use ruff_python_whitespace::{PythonWhitespace, UniversalNewlines}; use ruff_python_whitespace::{PythonWhitespace, UniversalNewlines};
use crate::comments::visitor::{CommentPlacement, DecoratedComment}; use crate::comments::visitor::{CommentPlacement, DecoratedComment};
use crate::comments::CommentTextPosition; use crate::comments::CommentLinePosition;
use crate::trivia::{SimpleTokenizer, Token, TokenKind}; use crate::trivia::{SimpleTokenizer, Token, TokenKind};
/// Implements the custom comment placement logic. /// Implements the custom comment placement logic.
@ -49,7 +49,7 @@ fn handle_match_comment<'a>(
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
// Must be an own line comment after the last statement in a match case // Must be an own line comment after the last statement in a match case
if comment.text_position().is_end_of_line() || comment.following_node().is_some() { if comment.line_position().is_end_of_line() || comment.following_node().is_some() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -147,7 +147,7 @@ fn handle_in_between_except_handlers_or_except_handler_and_else_or_finally_comme
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
if comment.text_position().is_end_of_line() || comment.following_node().is_none() { if comment.line_position().is_end_of_line() || comment.following_node().is_none() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -201,7 +201,7 @@ fn handle_in_between_bodies_own_line_comment<'a>(
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
if !comment.text_position().is_own_line() { if !comment.line_position().is_own_line() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -310,7 +310,7 @@ fn handle_in_between_bodies_end_of_line_comment<'a>(
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
if !comment.text_position().is_end_of_line() { if !comment.line_position().is_end_of_line() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -393,7 +393,7 @@ fn handle_trailing_body_comment<'a>(
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
if comment.text_position().is_end_of_line() { if comment.line_position().is_end_of_line() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -485,7 +485,7 @@ fn handle_trailing_body_comment<'a>(
/// ``` /// ```
fn handle_trailing_end_of_line_body_comment(comment: DecoratedComment<'_>) -> CommentPlacement<'_> { fn handle_trailing_end_of_line_body_comment(comment: DecoratedComment<'_>) -> CommentPlacement<'_> {
// Must be an end of line comment // Must be an end of line comment
if comment.text_position().is_own_line() { if comment.line_position().is_own_line() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -526,7 +526,7 @@ fn handle_trailing_end_of_line_condition_comment<'a>(
use ruff_python_ast::prelude::*; use ruff_python_ast::prelude::*;
// Must be an end of line comment // Must be an end of line comment
if comment.text_position().is_own_line() { if comment.line_position().is_own_line() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -640,15 +640,15 @@ fn handle_positional_only_arguments_separator_comment<'a>(
if let Some(slash_offset) = find_pos_only_slash_offset(trivia_range, locator) { if let Some(slash_offset) = find_pos_only_slash_offset(trivia_range, locator) {
let comment_start = comment.slice().range().start(); let comment_start = comment.slice().range().start();
let is_slash_comment = match comment.text_position() { let is_slash_comment = match comment.line_position() {
CommentTextPosition::EndOfLine => { CommentLinePosition::EndOfLine => {
let preceding_end_line = locator.line_end(last_argument_or_default.end()); let preceding_end_line = locator.line_end(last_argument_or_default.end());
let slash_comments_start = preceding_end_line.min(slash_offset); let slash_comments_start = preceding_end_line.min(slash_offset);
comment_start >= slash_comments_start comment_start >= slash_comments_start
&& locator.line_end(slash_offset) > comment_start && locator.line_end(slash_offset) > comment_start
} }
CommentTextPosition::OwnLine => comment_start < slash_offset, CommentLinePosition::OwnLine => comment_start < slash_offset,
}; };
if is_slash_comment { if is_slash_comment {
@ -711,7 +711,7 @@ fn handle_trailing_binary_expression_left_or_operator_comment<'a>(
// ) // )
// ``` // ```
CommentPlacement::trailing(AnyNodeRef::from(binary_expression.left.as_ref()), comment) CommentPlacement::trailing(AnyNodeRef::from(binary_expression.left.as_ref()), comment)
} else if comment.text_position().is_end_of_line() { } else if comment.line_position().is_end_of_line() {
// Is the operator on its own line. // Is the operator on its own line.
if locator.contains_line_break(TextRange::new( if locator.contains_line_break(TextRange::new(
binary_expression.left.end(), binary_expression.left.end(),
@ -800,7 +800,7 @@ fn handle_module_level_own_line_comment_before_class_or_function_comment<'a>(
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
// Only applies for own line comments on the module level... // Only applies for own line comments on the module level...
if !comment.text_position().is_own_line() || !comment.enclosing_node().is_module() { if !comment.line_position().is_own_line() || !comment.enclosing_node().is_module() {
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
@ -877,7 +877,7 @@ fn handle_leading_function_with_decorators_comment(comment: DecoratedComment) ->
.following_node() .following_node()
.map_or(false, |node| node.is_arguments()); .map_or(false, |node| node.is_arguments());
if comment.text_position().is_own_line() && is_preceding_decorator && is_following_arguments { if comment.line_position().is_own_line() && is_preceding_decorator && is_following_arguments {
CommentPlacement::dangling(comment.enclosing_node(), comment) CommentPlacement::dangling(comment.enclosing_node(), comment)
} else { } else {
CommentPlacement::Default(comment) CommentPlacement::Default(comment)

View file

@ -1,6 +1,6 @@
use crate::comments::node_key::NodeRefEqualityKey; use crate::comments::node_key::NodeRefEqualityKey;
use crate::comments::placement::place_comment; use crate::comments::placement::place_comment;
use crate::comments::{CommentTextPosition, CommentsMap, SourceComment}; use crate::comments::{CommentLinePosition, CommentsMap, SourceComment};
use ruff_formatter::{SourceCode, SourceCodeSlice}; use ruff_formatter::{SourceCode, SourceCodeSlice};
use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::prelude::*; use ruff_python_ast::prelude::*;
@ -66,7 +66,7 @@ impl<'a> CommentsVisitor<'a> {
preceding: self.preceding_node, preceding: self.preceding_node,
following: Some(node), following: Some(node),
parent: self.parents.iter().rev().nth(1).copied(), parent: self.parents.iter().rev().nth(1).copied(),
text_position: text_position(*comment_range, self.source_code), line_position: text_position(*comment_range, self.source_code),
slice: self.source_code.slice(*comment_range), slice: self.source_code.slice(*comment_range),
}; };
@ -125,7 +125,7 @@ impl<'a> CommentsVisitor<'a> {
preceding: self.preceding_node, preceding: self.preceding_node,
parent: self.parents.last().copied(), parent: self.parents.last().copied(),
following: None, following: None,
text_position: text_position(*comment_range, self.source_code), line_position: text_position(*comment_range, self.source_code),
slice: self.source_code.slice(*comment_range), slice: self.source_code.slice(*comment_range),
}; };
@ -280,7 +280,7 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> {
} }
} }
fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentTextPosition { fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentLinePosition {
let before = &source_code.as_str()[TextRange::up_to(comment_range.start())]; let before = &source_code.as_str()[TextRange::up_to(comment_range.start())];
for c in before.chars().rev() { for c in before.chars().rev() {
@ -289,11 +289,11 @@ fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentTe
break; break;
} }
c if is_python_whitespace(c) => continue, c if is_python_whitespace(c) => continue,
_ => return CommentTextPosition::EndOfLine, _ => return CommentLinePosition::EndOfLine,
} }
} }
CommentTextPosition::OwnLine CommentLinePosition::OwnLine
} }
/// A comment decorated with additional information about its surrounding context in the source document. /// A comment decorated with additional information about its surrounding context in the source document.
@ -305,7 +305,7 @@ pub(super) struct DecoratedComment<'a> {
preceding: Option<AnyNodeRef<'a>>, preceding: Option<AnyNodeRef<'a>>,
following: Option<AnyNodeRef<'a>>, following: Option<AnyNodeRef<'a>>,
parent: Option<AnyNodeRef<'a>>, parent: Option<AnyNodeRef<'a>>,
text_position: CommentTextPosition, line_position: CommentLinePosition,
slice: SourceCodeSlice, slice: SourceCodeSlice,
} }
@ -443,14 +443,14 @@ impl<'a> DecoratedComment<'a> {
} }
/// The position of the comment in the text. /// The position of the comment in the text.
pub(super) fn text_position(&self) -> CommentTextPosition { pub(super) fn line_position(&self) -> CommentLinePosition {
self.text_position self.line_position
} }
} }
impl From<DecoratedComment<'_>> for SourceComment { impl From<DecoratedComment<'_>> for SourceComment {
fn from(decorated: DecoratedComment) -> Self { fn from(decorated: DecoratedComment) -> Self {
Self::new(decorated.slice, decorated.text_position) Self::new(decorated.slice, decorated.line_position)
} }
} }
@ -659,8 +659,8 @@ impl<'a> CommentsBuilder<'a> {
self.push_dangling_comment(node, comment); self.push_dangling_comment(node, comment);
} }
CommentPlacement::Default(comment) => { CommentPlacement::Default(comment) => {
match comment.text_position() { match comment.line_position() {
CommentTextPosition::EndOfLine => { CommentLinePosition::EndOfLine => {
match (comment.preceding_node(), comment.following_node()) { match (comment.preceding_node(), comment.following_node()) {
(Some(preceding), Some(_)) => { (Some(preceding), Some(_)) => {
// Attach comments with both preceding and following node to the preceding // Attach comments with both preceding and following node to the preceding
@ -682,7 +682,7 @@ impl<'a> CommentsBuilder<'a> {
} }
} }
} }
CommentTextPosition::OwnLine => { CommentLinePosition::OwnLine => {
match (comment.preceding_node(), comment.following_node()) { match (comment.preceding_node(), comment.following_node()) {
// Following always wins for a leading comment // Following always wins for a leading comment
// ```python // ```python

View file

@ -1,4 +1,4 @@
use crate::comments::{dangling_comments, CommentTextPosition, Comments}; use crate::comments::{dangling_comments, CommentLinePosition, Comments};
use crate::expression::parentheses::{ use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
}; };
@ -30,11 +30,12 @@ impl FormatNodeRule<ExprList> for FormatExprList {
// ``` // ```
// In all other cases comments get assigned to a list element // In all other cases comments get assigned to a list element
if elts.is_empty() { if elts.is_empty() {
let end_of_line_split = dangling let end_of_line_split = dangling.partition_point(|comment| {
.partition_point(|comment| comment.position() == CommentTextPosition::EndOfLine); comment.line_position() == CommentLinePosition::EndOfLine
});
debug_assert!(dangling[end_of_line_split..] debug_assert!(dangling[end_of_line_split..]
.iter() .iter()
.all(|comment| comment.position() == CommentTextPosition::OwnLine)); .all(|comment| comment.line_position() == CommentLinePosition::OwnLine));
return write!( return write!(
f, f,
[group(&format_args![ [group(&format_args![

View file

@ -39,7 +39,7 @@ impl FormatRule<AnyFunctionDefinition<'_>, PyFormatContext<'_>> for FormatAnyFun
let dangling_comments = comments.dangling_comments(item); let dangling_comments = comments.dangling_comments(item);
let trailing_definition_comments_start = let trailing_definition_comments_start =
dangling_comments.partition_point(|comment| comment.position().is_own_line()); dangling_comments.partition_point(|comment| comment.line_position().is_own_line());
let (leading_function_definition_comments, trailing_definition_comments) = let (leading_function_definition_comments, trailing_definition_comments) =
dangling_comments.split_at(trailing_definition_comments_start); dangling_comments.split_at(trailing_definition_comments_start);

View file

@ -73,7 +73,7 @@ impl FormatNodeRule<StmtIf> for FormatStmtIf {
if !orelse.is_empty() { if !orelse.is_empty() {
// Leading comments are always own line comments // Leading comments are always own line comments
let leading_else_comments_end = let leading_else_comments_end =
else_comments.partition_point(|comment| comment.position().is_own_line()); else_comments.partition_point(|comment| comment.line_position().is_own_line());
let (else_leading, else_trailing) = else_comments.split_at(leading_else_comments_end); let (else_leading, else_trailing) = else_comments.split_at(leading_else_comments_end);
write!( write!(

View file

@ -44,7 +44,7 @@ impl FormatNodeRule<StmtWhile> for FormatStmtWhile {
// Split between leading comments before the `else` keyword and end of line comments at the end of // Split between leading comments before the `else` keyword and end of line comments at the end of
// the `else:` line. // the `else:` line.
let trailing_start = let trailing_start =
or_else_comments.partition_point(|comment| comment.position().is_own_line()); or_else_comments.partition_point(|comment| comment.line_position().is_own_line());
let (leading, trailing) = or_else_comments.split_at(trailing_start); let (leading, trailing) = or_else_comments.split_at(trailing_start);
write!( write!(