Skip to content

Commit

Permalink
Account for trailing comments in span handling
Browse files Browse the repository at this point in the history
Fixes tamasfe#464

When using taplo rules to limit configuration to specific portions of a
document, trailing comments would break formatting.

`Node#text_ranges`, when considering tables, was returning ranges
excluding trailing comments. When the formatter walks the code and is
trying to determine which rules apply to a table, it checks whether the
table is a subset of the rule "span". When there is a trailing comment,
this check fails, and the rule will never apply.

Since the dom `Node` for tables contains only a reference to the table
header, we can get the text range of the entire table, including
trailing or interior comments, by traversing up to the parent and taking
its text range. If this is unavailable, the previous behavior is used.

Note that this modifies the behavior of `crate::dom::Node::text_ranges`,
which is part of the public API.

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
  • Loading branch information
clarkf committed Jan 3, 2024
1 parent 268c8b1 commit 6cb4ce6
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 1 deletion.
46 changes: 45 additions & 1 deletion crates/taplo/src/dom/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ impl Node {
Ok(all.into_iter())
}

/// Determine the portions of the source that correspond to this
/// `Node`.
///
/// Since a symbol may appear in multiple places, a single node
/// may correspond to multiple `TextRange`s.
pub fn text_ranges(&self) -> impl ExactSizeIterator<Item = TextRange> {
let mut ranges = Vec::with_capacity(1);

Expand All @@ -256,7 +261,19 @@ impl Node {
ranges.extend(entry.text_ranges());
}

if let Some(mut r) = v.syntax().map(|s| s.text_range()) {
// consider both v.syntax() and its parent node when
// broadening the range. v.syntax() is only the
// TableHeader, where its parent is the full Table
// syntax node. Take the broader of the two.
let largest_syntax_range = [
v.syntax().map(|s| s.text_range()),
v.syntax().and_then(|s| s.parent()).map(|n| n.text_range()),
]
.into_iter()
.flatten()
.max_by(|a, b| a.len().cmp(&b.len()));

if let Some(mut r) = largest_syntax_range {
for range in &ranges {
r = r.cover(*range);
}
Expand Down Expand Up @@ -659,3 +676,30 @@ impl From<Invalid> for Node {
Self::Invalid(v)
}
}

#[cfg(test)]
mod tests {
#[test]
fn test_dom_find_all_matches_includes_comments() {
let source = r#"
[table]
key = "value" # comment
"#;
let dom = crate::parser::parse(source).into_dom();
let matches = dom
.find_all_matches("table".parse().unwrap(), false)
.unwrap()
.collect::<Vec<_>>();
assert_eq!(1, matches.len());
let (_, node) = matches.first().unwrap();

// Ensure that at least one of the resultant spans includes
// the comment.
let spans = node.text_ranges().map(|r| &source[r]).collect::<Vec<_>>();
assert!(
spans.iter().any(|s| s.contains('#')),
"expected one of {:?} to contain a comment",
&spans
);
}
}
35 changes: 35 additions & 0 deletions crates/taplo/src/tests/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,41 @@ foo = [
assert_format!(expected, &formatted);
}

/// See https://github.com/tamasfe/taplo/issues/464
#[test]
fn test_reorder_keys_trailing_comment() {
let src = r#"
[mytable]
a = "a"
c = "c"
b = "b" # ...
"#;

let expected = r#"
[mytable]
a = "a"
b = "b" # ...
c = "c"
"#;

let p = crate::parser::parse(src);
let formatted = crate::formatter::format_with_path_scopes(
p.into_dom(),
Default::default(),
&[],
vec![(
"mytable",
formatter::OptionsIncomplete {
reorder_keys: Some(true),
..Default::default()
},
)],
)
.unwrap();

assert_format!(expected, &formatted);
}

#[test]
fn test_single_comment_no_alignment() {
let src = r#"
Expand Down

0 comments on commit 6cb4ce6

Please sign in to comment.