From ba4c2ec1e454b0a0776344bbc1109e82cb0d3927 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 27 Sep 2026 16:50:26 -0400 Subject: [PATCH 1/3] feat: support range formatting Formats only the object properties or array elements that a range touches within the innermost object or array containing it, leaving the rest of the file as written. The whole object or array is formatted when its members would be reordered (ex. package.json conventions) or the line breaks around them change, and the whole file when the range reaches the root value's brackets. --- Cargo.lock | 116 +++------- Cargo.toml | 3 +- src/format_range.rs | 275 ++++++++++++++++++++++++ src/format_text.rs | 6 +- src/lib.rs | 2 + src/wasm_plugin.rs | 9 +- tests/specs/range/Range_Containers.txt | 42 ++++ tests/specs/range/Range_Members.txt | 117 ++++++++++ tests/specs/range/Range_PackageJson.txt | 31 +++ tests/test.rs | 8 +- 10 files changed, 511 insertions(+), 98 deletions(-) create mode 100644 src/format_range.rs create mode 100644 tests/specs/range/Range_Containers.txt create mode 100644 tests/specs/range/Range_Members.txt create mode 100644 tests/specs/range/Range_PackageJson.txt diff --git a/Cargo.lock b/Cargo.lock index b384947..c84f29a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -81,15 +81,14 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "console" -version = "0.15.7" +version = "0.16.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c926e00cc70edefdc64d3a5ff31cc65bb97a3460097762bd23afb4d8145fccf8" +checksum = "e96a4956774c13c126a8b5af4daa79384f4d826534c95a02d76afb39e2ab64e3" dependencies = [ "encode_unicode", - "lazy_static", "libc", - "unicode-width 0.1.10", - "windows-sys 0.45.0", + "unicode-width", + "windows-sys 0.61.2", ] [[package]] @@ -160,7 +159,7 @@ dependencies = [ "serde", "serde_json", "thiserror", - "unicode-width 0.2.0", + "unicode-width", ] [[package]] @@ -175,11 +174,10 @@ dependencies = [ [[package]] name = "dprint-development" -version = "0.10.2" +version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19cc5c05c28cb1bfc6c4a22174e634c0096a99288b2844518b7e3da77337b9ff" +checksum = "b380bab000ac6a5321d235375cfb1a727b5ff48a27891dd018dc9f741e7cba9a" dependencies = [ - "anyhow", "console", "file_test_runner", "serde_json", @@ -190,7 +188,6 @@ dependencies = [ name = "dprint-plugin-json" version = "0.24.0" dependencies = [ - "anyhow", "debug-here", "dprint-core", "dprint-core-macros", @@ -210,9 +207,9 @@ checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" [[package]] name = "encode_unicode" -version = "0.3.6" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" [[package]] name = "equivalent" @@ -231,9 +228,9 @@ dependencies = [ [[package]] name = "file_test_runner" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "182e4f4d9359dae21754e0df8357b76d4df81ee881688deb5bca41fb547f0260" +checksum = "aab2ea62262e650557af93e48bfb0ffbcf3b6d86af4b9fea1aa00f93821b69a9" dependencies = [ "anyhow", "crossbeam-channel", @@ -297,7 +294,7 @@ version = "0.33.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ff5a48f48971be8e762a6ff955725a0802b6e46c441057992da5a673db9fd3a" dependencies = [ - "unicode-width 0.2.0", + "unicode-width", ] [[package]] @@ -370,7 +367,7 @@ dependencies = [ "libc", "redox_syscall", "smallvec", - "windows-targets 0.52.5", + "windows-targets", ] [[package]] @@ -586,12 +583,6 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4f5b37a154999a8f3f98cc23a628d850e154479cd94decf3414696e12e31aaf" -[[package]] -name = "unicode-width" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" - [[package]] name = "unicode-width" version = "0.2.0" @@ -640,13 +631,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] -name = "windows-sys" -version = "0.45.0" +name = "windows-link" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" -dependencies = [ - "windows-targets 0.42.2", -] +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" [[package]] name = "windows-sys" @@ -654,22 +642,16 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.5", + "windows-targets", ] [[package]] -name = "windows-targets" -version = "0.42.2" +name = "windows-sys" +version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e5180c00cd44c9b1c88adb3693291f1cd93605ded80c250a75d472756b4d071" +checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" dependencies = [ - "windows_aarch64_gnullvm 0.42.2", - "windows_aarch64_msvc 0.42.2", - "windows_i686_gnu 0.42.2", - "windows_i686_msvc 0.42.2", - "windows_x86_64_gnu 0.42.2", - "windows_x86_64_gnullvm 0.42.2", - "windows_x86_64_msvc 0.42.2", + "windows-link", ] [[package]] @@ -678,46 +660,28 @@ version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6f0713a46559409d202e70e28227288446bf7841d3211583a4b53e3f6d96e7eb" dependencies = [ - "windows_aarch64_gnullvm 0.52.5", - "windows_aarch64_msvc 0.52.5", - "windows_i686_gnu 0.52.5", + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", "windows_i686_gnullvm", - "windows_i686_msvc 0.52.5", - "windows_x86_64_gnu 0.52.5", - "windows_x86_64_gnullvm 0.52.5", - "windows_x86_64_msvc 0.52.5", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", ] -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8" - [[package]] name = "windows_aarch64_gnullvm" version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7088eed71e8b8dda258ecc8bac5fb1153c5cffaf2578fc8ff5d61e23578d3263" -[[package]] -name = "windows_aarch64_msvc" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43" - [[package]] name = "windows_aarch64_msvc" version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9985fd1504e250c615ca5f281c3f7a6da76213ebd5ccc9561496568a2752afb6" -[[package]] -name = "windows_i686_gnu" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f" - [[package]] name = "windows_i686_gnu" version = "0.52.5" @@ -730,48 +694,24 @@ version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "87f4261229030a858f36b459e748ae97545d6f1ec60e5e0d6a3d32e0dc232ee9" -[[package]] -name = "windows_i686_msvc" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060" - [[package]] name = "windows_i686_msvc" version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "db3c2bf3d13d5b658be73463284eaf12830ac9a26a90c717b7f771dfe97487bf" -[[package]] -name = "windows_x86_64_gnu" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36" - [[package]] name = "windows_x86_64_gnu" version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4e4246f76bdeff09eb48875a0fd3e2af6aada79d409d33011886d3e1581517d9" -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3" - [[package]] name = "windows_x86_64_gnullvm" version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "852298e482cd67c356ddd9570386e2862b5673c85bd5f88df9ab6802b334c596" -[[package]] -name = "windows_x86_64_msvc" -version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0" - [[package]] name = "windows_x86_64_msvc" version = "0.52.5" diff --git a/Cargo.toml b/Cargo.toml index 37b9f5b..e0c0bf0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,6 @@ text_lines = "0.6.0" thiserror = "2" [dev-dependencies] -anyhow = "1.0.64" debug-here = "0.2" -dprint-development = "0.10.2" +dprint-development = "0.12.0" serde_json = { version = "1.0" } diff --git a/src/format_range.rs b/src/format_range.rs new file mode 100644 index 0000000..78bee72 --- /dev/null +++ b/src/format_range.rs @@ -0,0 +1,275 @@ +use std::ops::Range; +use std::path::Path; + +use jsonc_parser::ast::Value; +use jsonc_parser::common::Ranged; + +use super::configuration::Configuration; +use super::format_text::FormatError; +use super::format_text::format_text; +use super::format_text::format_text_inner; +use super::format_text::parse; +use super::format_text::strip_bom; + +/// Formats only the part of the text within the provided byte range. +/// +/// The range is widened to the object properties or array elements it touches in the innermost +/// object or array that contains it, and the text outside of those is left as it was. When those +/// members don't keep their order once formatted (ex. the `package.json` conventions reorder +/// them) or their object or array switches between single and multi-line, the whole object or +/// array is formatted instead. A range that reaches the brackets of the root value formats the +/// whole file. +pub fn format_text_range( + path: &Path, + text: &str, + range: Range, + config: &Configuration, +) -> Result, FormatError> { + let body = strip_bom(text); + let bom_len = text.len() - body.len(); + let end = range.end.saturating_sub(bom_len).min(body.len()); + let range = range.start.saturating_sub(bom_len).min(end)..end; + if range.start == 0 && range.end == body.len() { + return format_text(path, text, config); + } + + let parse_result = parse(body)?; + let Some(root) = &parse_result.value else { + return format_text(path, text, config); + }; + let selection = match find_target(root, &range) { + Target::File => return format_text(path, text, config), + Target::Nothing => return Ok(None), + Target::Members(selection) => selection, + }; + + let formatted = format_text_inner(path, body, config)?; + let formatted_parse_result = parse(&formatted)?; + let Some(formatted_root) = &formatted_parse_result.value else { + return format_text(path, text, config); + }; + let Some((original_range, formatted_range)) = find_replacement(&selection, body, formatted_root, &formatted) else { + return format_text(path, text, config); + }; + + let result = format!( + "{}{}{}", + &text[..bom_len + original_range.start], + &formatted[formatted_range], + &text[bom_len + original_range.end..] + ); + if result == text { Ok(None) } else { Ok(Some(result)) } +} + +enum Target<'a> { + /// The range reaches the root value's brackets or the root isn't an object or array. + File, + /// The range only touches whitespace or comments between members. + Nothing, + Members(Selection<'a>), +} + +struct Selection<'a> { + /// Steps from the root to the object or array holding the members. + path: Vec>, + container: &'a Value<'a>, + /// Steps to each touched member within the container, in order. + members: Vec>, + /// Range from the start of the first touched member to the end of the last. + range: Range, +} + +/// Identifies a member by what it is rather than where it is, so it can be found again in the +/// formatted text even after its properties were reordered. +#[derive(Clone, Copy, PartialEq)] +enum Step<'a> { + /// Property name along with how many properties with the same name come before it. + Prop(&'a str, usize), + Element(usize), +} + +struct Member<'a> { + step: Step<'a>, + range: Range, + value: &'a Value<'a>, +} + +fn find_target<'a>(root: &'a Value<'a>, range: &Range) -> Target<'a> { + if !is_within_brackets(root, range) { + return Target::File; + } + let mut path = Vec::new(); + let mut container = root; + loop { + let Some(members) = members(container) else { + return Target::File; + }; + let touched = members + .into_iter() + .filter(|member| touches(&member.range, range)) + .collect::>(); + match touched.as_slice() { + [] => return Target::Nothing, + [member] if is_container(member.value) && is_within_brackets(member.value, range) => { + path.push(member.step); + container = member.value; + } + [first, .., last] | [first @ last] => { + return Target::Members(Selection { + range: first.range.start..last.range.end, + members: touched.iter().map(|member| member.step).collect(), + path, + container, + }); + } + } + } +} + +/// Finds the range in the original text to replace and the formatted text to replace it with. +fn find_replacement( + selection: &Selection, + text: &str, + formatted_root: &Value, + formatted_text: &str, +) -> Option<(Range, Range)> { + let mut formatted_container = formatted_root; + for step in &selection.path { + formatted_container = members(formatted_container)? + .into_iter() + .find(|member| member.step == *step)? + .value; + } + + let formatted_members = members(formatted_container)?; + let indexes = selection + .members + .iter() + .map(|step| formatted_members.iter().position(|member| member.step == *step)) + .collect::>>()?; + let keeps_order = indexes.windows(2).all(|pair| pair[1] == pair[0] + 1); + let first_index = indexes[0]; + let last_index = *indexes.last()?; + // the text around the spliced members is kept, so the line breaks there need to already be + // what the formatter wants or the result would be half single and half multi-line + let original_members = members(selection.container)?; + let original_first_index = original_members + .iter() + .position(|member| member.step == selection.members[0])?; + let original_last_index = original_first_index + selection.members.len() - 1; + let keeps_lines = line_breaks_around( + selection.container, + &original_members, + original_first_index..original_last_index, + text, + ) == line_breaks_around( + formatted_container, + &formatted_members, + first_index..last_index, + formatted_text, + ); + if keeps_order && keeps_lines { + let start = formatted_members[first_index].range.start; + let end = formatted_members[last_index].range.end; + Some((selection.range.clone(), start..end)) + } else if selection.path.is_empty() { + // the container is the root, so the whole file needs formatting + None + } else { + Some((range_of(selection.container), range_of(formatted_container))) + } +} + +fn members<'a>(value: &'a Value<'a>) -> Option>> { + match value { + Value::Object(object) => { + let mut members: Vec> = Vec::with_capacity(object.properties.len()); + for prop in &object.properties { + let name = prop.name.as_str(); + let occurrence = members + .iter() + .filter(|member| matches!(member.step, Step::Prop(other, _) if other == name)) + .count(); + members.push(Member { + step: Step::Prop(name, occurrence), + range: prop.range.start..prop.range.end, + value: &prop.value, + }); + } + Some(members) + } + Value::Array(array) => Some( + array + .elements + .iter() + .enumerate() + .map(|(index, element)| Member { + step: Step::Element(index), + range: range_of(element), + value: element, + }) + .collect(), + ), + _ => None, + } +} + +fn is_container(value: &Value) -> bool { + matches!(value, Value::Object(_) | Value::Array(_)) +} + +fn is_within_brackets(value: &Value, range: &Range) -> bool { + is_container(value) && range.start > value.start() && range.end < value.end() +} + +fn touches(member: &Range, range: &Range) -> bool { + if range.is_empty() { + member.start <= range.start && range.start <= member.end + } else { + range.start < member.end && range.end > member.start + } +} + +/// Gets whether there's a line break before the first member and after the last member of the +/// provided inclusive range of member indexes. +fn line_breaks_around(container: &Value, members: &[Member], indexes: Range, text: &str) -> (bool, bool) { + let first = &members[indexes.start]; + let last = &members[indexes.end]; + let before_start = match indexes.start { + 0 => container.start(), + index => members[index - 1].range.end, + }; + let after_end = members + .get(indexes.end + 1) + .map(|member| member.range.start) + .unwrap_or(container.end()); + ( + text[before_start..first.range.start].contains('\n'), + text[last.range.end..after_end].contains('\n'), + ) +} + +fn range_of(value: &Value) -> Range { + value.start()..value.end() +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use crate::configuration::ConfigurationBuilder; + + use super::*; + + #[test] + fn keeps_bom_outside_range() { + // the spec files can't express this since editors strip the bom + let config = ConfigurationBuilder::new().build(); + let text = "\u{FEFF}{\n \"a\":1,\n \"b\":2\n}\n"; + let start = text.find("\"b\"").unwrap(); + let output = format_text_range(Path::new("/file.json"), text, start..start + 5, &config) + .unwrap() + .unwrap(); + assert_eq!(output, "\u{FEFF}{\n \"a\":1,\n \"b\": 2\n}\n"); + } +} diff --git a/src/format_text.rs b/src/format_text.rs index f532143..d10645e 100644 --- a/src/format_text.rs +++ b/src/format_text.rs @@ -44,7 +44,7 @@ pub fn format_text(path: &Path, text: &str, config: &Configuration) -> Result Result { +pub(crate) fn format_text_inner(path: &Path, text: &str, config: &Configuration) -> Result { let text = strip_bom(text); let text = if config.package_json_apply_conventions && package_json::is_package_json_file(path) { package_json::apply_conventions(text, config) @@ -70,11 +70,11 @@ pub fn trace_file(text: &str, config: &Configuration) -> dprint_core::formatting ) } -fn strip_bom(text: &str) -> &str { +pub(crate) fn strip_bom(text: &str) -> &str { text.strip_prefix("\u{FEFF}").unwrap_or(text) } -fn parse(text: &str) -> Result, FormatError> { +pub(crate) fn parse(text: &str) -> Result, FormatError> { let parse_result = parse_to_ast( text, &CollectOptions { diff --git a/src/lib.rs b/src/lib.rs index 26a5785..0ea25fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,9 +1,11 @@ pub mod configuration; +mod format_range; mod format_text; mod generation; mod glob; mod package_json; +pub use format_range::format_text_range; pub use format_text::format_text; #[cfg(feature = "tracing")] diff --git a/src/wasm_plugin.rs b/src/wasm_plugin.rs index a0a36e5..a7090ca 100644 --- a/src/wasm_plugin.rs +++ b/src/wasm_plugin.rs @@ -63,9 +63,12 @@ impl SyncPluginHandler for JsonPluginHandler { _format_with_host: impl FnMut(SyncHostFormatRequest) -> FormatResult, ) -> FormatResult { let file_text = String::from_utf8(request.file_bytes)?; - super::format_text(request.file_path, &file_text, request.config) - .map(|maybe_text| maybe_text.map(|t| t.into_bytes())) - .map_err(FormatError::new) + match request.range { + Some(range) => super::format_text_range(request.file_path, &file_text, range, request.config), + None => super::format_text(request.file_path, &file_text, request.config), + } + .map(|maybe_text| maybe_text.map(|t| t.into_bytes())) + .map_err(FormatError::new) } } diff --git a/tests/specs/range/Range_Containers.txt b/tests/specs/range/Range_Containers.txt new file mode 100644 index 0000000..8b4e32f --- /dev/null +++ b/tests/specs/range/Range_Containers.txt @@ -0,0 +1,42 @@ +== should format the whole object when the range reaches its brackets == +{ + "a":1, + "b":[|{"c":1, +"d":2}|] +} + +[expect] +{ + "a":1, + [|"b": { "c": 1, "d": 2 }|] +} + +== should format the whole object when the line breaks around the members change == +{ + "a":1, + "b":{ +"c":[||]1, "d":2} +} + +[expect] +{ + "a":1, + "b":{ + "c": 1, + "d": 2 + } +} + +== should format the file when the range reaches the root brackets == +[|{"a":1, +"b":2}|] + +[expect] +{ "a": 1, "b": 2 } + +== should format the file when the range starts at the root bracket == +[|{"a"|]:1, +"b":2} + +[expect] +{ "a": 1, "b": 2 } diff --git a/tests/specs/range/Range_Members.txt b/tests/specs/range/Range_Members.txt new file mode 100644 index 0000000..202f6d2 --- /dev/null +++ b/tests/specs/range/Range_Members.txt @@ -0,0 +1,117 @@ +== should format only the touched property == +{ + "a":1, + [|"b":2|], + "c":3 +} + +[expect] +{ + "a":1, + [|"b": 2|], + "c":3 +} + +== should format all the touched properties == +{ + "a":1, + "b"[|:2, + "c"|]:3, + "d":4 +} + +[expect] +{ + "a":1, + [|"b": 2, + "c": 3|], + "d":4 +} + +== should widen a partial selection to the whole member == +{ + "a": [|[1,2]|], + "b":2 +} + +[expect] +{ + [|"a": [1, 2]|], + "b":2 +} + +== should format the touched array elements == +[ + {"a":1}, + [|{"b":2}|], + {"c":3} +] + +[expect] +[ + {"a":1}, + [|{ "b": 2 }|], + {"c":3} +] + +== should descend into nested objects == +{ + "a":1, + "b": { + "c":1, + [|"d":[1,2]|] + } +} + +[expect] +{ + "a":1, + "b": { + "c":1, + [|"d": [1, 2]|] + } +} + +== should format the member at the cursor == +{ + "a":1, + "b": { + "c":1, + "d":[||][1,2] + } +} + +[expect] +{ + "a":1, + "b": { + "c":1, + "d": [1, 2] + } +} + +== should not change anything for a range between members == +{ + "a":1, +[||] + "b":2 +} + +[expect] +{ + "a":1, + + "b":2 +} + +== should find the property when there are duplicate names == +{ + "a":1, + [|"a":2|] +} + +[expect] +{ + "a":1, + [|"a": 2|] +} diff --git a/tests/specs/range/Range_PackageJson.txt b/tests/specs/range/Range_PackageJson.txt new file mode 100644 index 0000000..6aa6de4 --- /dev/null +++ b/tests/specs/range/Range_PackageJson.txt @@ -0,0 +1,31 @@ +-- package.json -- +== should find a property that the conventions move == +{ + [|"version":"1.0.0"|], + "dependencies": {"b":"1", + "a":"1"}, + "name":"a" +} + +[expect] +{ + [|"version": "1.0.0"|], + "dependencies": {"b":"1", + "a":"1"}, + "name":"a" +} + +== should format the whole object when the conventions reorder its members == +{ + "version":"1.0.0", + "dependencies": {[|"b":"1", + "a"|]:"1"}, + "name":"a" +} + +[expect] +{ + "version":"1.0.0", + "dependencies": { "a": "1", "b": "1" }, + "name":"a" +} diff --git a/tests/test.rs b/tests/test.rs index 541bbf9..2c800d3 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -28,12 +28,16 @@ fn test_specs() { }, { let global_config = global_config.clone(); - Arc::new(move |path, file_text, spec_config| { + Arc::new(move |path, file_text, range, spec_config| { let spec_config: ConfigKeyMap = serde_json::from_value(spec_config.clone().into()).unwrap(); let config_result = resolve_config(spec_config, &global_config); ensure_no_diagnostics(&config_result.diagnostics); - format_text(&path, &file_text, &config_result.config).map_err(anyhow::Error::from) + let result = match range { + Some(range) => format_text_range(path, file_text, range, &config_result.config), + None => format_text(path, file_text, &config_result.config), + }; + result.map_err(|err| err.into()) }) }, Arc::new(move |_, _file_text, _spec_config| { From 3db88cd0ecca90726be5c9a79f4e82e45231aff5 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 27 Sep 2026 17:10:02 -0400 Subject: [PATCH 2/3] fix: replace whole lines when range formatting Replacing only the touched members left their indentation, separators and trailing comments unformatted, so a selected line kept a wrong indent, a multi-line member came out indented inconsistently and trailing commas weren't added or removed. The lines of the members are replaced instead, widening to the member holding their object or array when the line breaks around them change and keeping the file's line endings. Also leaves a range outside the root value unformatted. --- src/format_range.rs | 383 +++++++++++------- tests/specs/range/Range_Containers.txt | 4 +- tests/specs/range/Range_Lines.txt | 118 ++++++ tests/specs/range/Range_PackageJson.txt | 25 +- .../range/Range_TrailingCommas_Always.txt | 12 + .../range/Range_TrailingCommas_Never.txt | 18 + 6 files changed, 417 insertions(+), 143 deletions(-) create mode 100644 tests/specs/range/Range_Lines.txt create mode 100644 tests/specs/range/Range_TrailingCommas_Always.txt create mode 100644 tests/specs/range/Range_TrailingCommas_Never.txt diff --git a/src/format_range.rs b/src/format_range.rs index 78bee72..804de41 100644 --- a/src/format_range.rs +++ b/src/format_range.rs @@ -1,8 +1,14 @@ +use std::borrow::Cow; +use std::collections::HashMap; use std::ops::Range; +use std::ops::RangeInclusive; use std::path::Path; +use dprint_core::configuration::NewLineKind; +use dprint_core::configuration::resolve_new_line_kind; use jsonc_parser::ast::Value; use jsonc_parser::common::Ranged; +use jsonc_parser::tokens::Token; use super::configuration::Configuration; use super::format_text::FormatError; @@ -13,12 +19,12 @@ use super::format_text::strip_bom; /// Formats only the part of the text within the provided byte range. /// -/// The range is widened to the object properties or array elements it touches in the innermost -/// object or array that contains it, and the text outside of those is left as it was. When those -/// members don't keep their order once formatted (ex. the `package.json` conventions reorder -/// them) or their object or array switches between single and multi-line, the whole object or -/// array is formatted instead. A range that reaches the brackets of the root value formats the -/// whole file. +/// The range is widened to the lines of the object properties or array elements it touches in the +/// innermost object or array that contains it, and the text outside of those is left as it was. +/// When those members don't keep their order once formatted (ex. the `package.json` conventions +/// reorder them) or the line breaks around them change, the member holding their object or array +/// is formatted instead, and so on out to the whole file. A range that reaches the brackets of the +/// root value formats the whole file and one outside of the root value formats nothing. pub fn format_text_range( path: &Path, text: &str, @@ -37,46 +43,60 @@ pub fn format_text_range( let Some(root) = &parse_result.value else { return format_text(path, text, config); }; - let selection = match find_target(root, &range) { + let levels = match find_levels(root, &range) { Target::File => return format_text(path, text, config), Target::Nothing => return Ok(None), - Target::Members(selection) => selection, + Target::Levels(levels) => levels, }; let formatted = format_text_inner(path, body, config)?; let formatted_parse_result = parse(&formatted)?; - let Some(formatted_root) = &formatted_parse_result.value else { - return format_text(path, text, config); - }; - let Some((original_range, formatted_range)) = find_replacement(&selection, body, formatted_root, &formatted) else { - return format_text(path, text, config); + let formatted_root = formatted_parse_result + .value + .as_ref() + .expect("formatted text should have a value"); + let commas = parse_result + .tokens + .iter() + .flatten() + .filter(|token| matches!(token.token, Token::Comma)) + .map(|token| token.range.start) + .collect::>(); + let result = match find_replacement(&levels, body, &commas, formatted_root, &formatted) { + Some(replacement) => format!( + "{}{}{}", + &text[..bom_len + replacement.original.start], + with_file_new_lines(&formatted[replacement.formatted], body), + &text[bom_len + replacement.original.end..] + ), + // the layout of the root changes around the range, so the whole file needs formatting + None => formatted, }; - - let result = format!( - "{}{}{}", - &text[..bom_len + original_range.start], - &formatted[formatted_range], - &text[bom_len + original_range.end..] - ); if result == text { Ok(None) } else { Ok(Some(result)) } } enum Target<'a> { /// The range reaches the root value's brackets or the root isn't an object or array. File, - /// The range only touches whitespace or comments between members. + /// The range is outside the root value or only touches whitespace or comments between members. Nothing, - Members(Selection<'a>), + /// Each object or array from the root to the innermost one containing the range. + Levels(Vec>), } -struct Selection<'a> { - /// Steps from the root to the object or array holding the members. - path: Vec>, +/// An object or array along with the members of it to format. +struct Level<'a> { container: &'a Value<'a>, - /// Steps to each touched member within the container, in order. - members: Vec>, - /// Range from the start of the first touched member to the end of the last. + members: Vec>, + /// Indexes of the members to format. For all but the innermost level, this is only the member + /// holding the next level. + indexes: RangeInclusive, +} + +struct Member<'a> { + step: Step<'a>, range: Range, + value: &'a Value<'a>, } /// Identifies a member by what it is rather than where it is, so it can be found again in the @@ -88,115 +108,125 @@ enum Step<'a> { Element(usize), } -struct Member<'a> { - step: Step<'a>, - range: Range, - value: &'a Value<'a>, +struct Replacement { + original: Range, + formatted: Range, } -fn find_target<'a>(root: &'a Value<'a>, range: &Range) -> Target<'a> { +fn find_levels<'a>(root: &'a Value<'a>, range: &Range) -> Target<'a> { + if range.end <= root.start() || range.start >= root.end() { + return Target::Nothing; + } if !is_within_brackets(root, range) { return Target::File; } - let mut path = Vec::new(); + let mut levels = Vec::new(); let mut container = root; loop { let Some(members) = members(container) else { return Target::File; }; - let touched = members - .into_iter() - .filter(|member| touches(&member.range, range)) - .collect::>(); - match touched.as_slice() { - [] => return Target::Nothing, - [member] if is_container(member.value) && is_within_brackets(member.value, range) => { - path.push(member.step); - container = member.value; - } - [first, .., last] | [first @ last] => { - return Target::Members(Selection { - range: first.range.start..last.range.end, - members: touched.iter().map(|member| member.step).collect(), - path, - container, - }); - } + let mut touched = members + .iter() + .enumerate() + .filter(|(_, member)| touches(&member.range, range)) + .map(|(index, _)| index); + let Some(first) = touched.next() else { + return Target::Nothing; + }; + let last = touched.next_back().unwrap_or(first); + let inner = Some(members[first].value).filter(|value| first == last && is_within_brackets(value, range)); + levels.push(Level { + container, + members, + indexes: first..=last, + }); + match inner { + Some(inner) => container = inner, + None => return Target::Levels(levels), } } } -/// Finds the range in the original text to replace and the formatted text to replace it with. +/// Finds the text to replace in the original and the formatted text to replace it with, starting +/// with the innermost level and moving out towards the root. fn find_replacement( - selection: &Selection, + levels: &[Level], text: &str, + commas: &[usize], formatted_root: &Value, formatted_text: &str, -) -> Option<(Range, Range)> { +) -> Option { + let mut formatted_levels = Vec::with_capacity(levels.len()); let mut formatted_container = formatted_root; - for step in &selection.path { - formatted_container = members(formatted_container)? - .into_iter() - .find(|member| member.step == *step)? - .value; + for level in levels { + let formatted_members = members(formatted_container)?; + let step = level.members[*level.indexes.start()].step; + let next_container = formatted_members.iter().find(|member| member.step == step)?.value; + formatted_levels.push((formatted_container, formatted_members)); + formatted_container = next_container; } - let formatted_members = members(formatted_container)?; - let indexes = selection - .members + levels .iter() - .map(|step| formatted_members.iter().position(|member| member.step == *step)) - .collect::>>()?; - let keeps_order = indexes.windows(2).all(|pair| pair[1] == pair[0] + 1); - let first_index = indexes[0]; - let last_index = *indexes.last()?; - // the text around the spliced members is kept, so the line breaks there need to already be - // what the formatter wants or the result would be half single and half multi-line - let original_members = members(selection.container)?; - let original_first_index = original_members - .iter() - .position(|member| member.step == selection.members[0])?; - let original_last_index = original_first_index + selection.members.len() - 1; - let keeps_lines = line_breaks_around( - selection.container, - &original_members, - original_first_index..original_last_index, - text, - ) == line_breaks_around( - formatted_container, - &formatted_members, - first_index..last_index, - formatted_text, - ); - if keeps_order && keeps_lines { - let start = formatted_members[first_index].range.start; - let end = formatted_members[last_index].range.end; - Some((selection.range.clone(), start..end)) - } else if selection.path.is_empty() { - // the container is the root, so the whole file needs formatting - None - } else { - Some((range_of(selection.container), range_of(formatted_container))) - } + .zip(&formatted_levels) + .rev() + .find_map(|(level, (formatted_container, formatted_members))| { + let formatted_indexes = level.members[level.indexes.clone()] + .iter() + .map(|member| { + formatted_members + .iter() + .position(|formatted| formatted.step == member.step) + }) + .collect::>>()?; + let keeps_order = formatted_indexes.windows(2).all(|pair| pair[1] == pair[0] + 1); + if !keeps_order { + return None; + } + let (first, last) = (*level.indexes.start(), *level.indexes.end()); + let (formatted_first, formatted_last) = (formatted_indexes[0], formatted_indexes[formatted_indexes.len() - 1]); + let original = Gaps::new(level.container, &level.members, first..=last); + let formatted = Gaps::new(formatted_container, formatted_members, formatted_first..=formatted_last); + // the text around the replaced text is kept, so the line breaks there need to already be + // what the formatter wants or the result would be half single and half multi-line + if original.line_breaks(text) != formatted.line_breaks(formatted_text) { + return None; + } + let edges = original.edges(text, commas); + // the replaced text holds the separator after the last member and, when it doesn't start at + // the first member's line, the one before the first, so those need to stay (ex. a member + // that's moved to the end loses its comma) + let keeps_next = (last == level.members.len() - 1) == (formatted_last == formatted_members.len() - 1); + let keeps_previous = edges.line_start || (first == 0) == (formatted_first == 0); + (keeps_next && keeps_previous).then(|| Replacement { + original: original.replaced_range(text, edges), + formatted: formatted.replaced_range(formatted_text, edges), + }) + }) } fn members<'a>(value: &'a Value<'a>) -> Option>> { match value { Value::Object(object) => { - let mut members: Vec> = Vec::with_capacity(object.properties.len()); - for prop in &object.properties { - let name = prop.name.as_str(); - let occurrence = members + let mut occurrences = HashMap::with_capacity(object.properties.len()); + Some( + object + .properties .iter() - .filter(|member| matches!(member.step, Step::Prop(other, _) if other == name)) - .count(); - members.push(Member { - step: Step::Prop(name, occurrence), - range: prop.range.start..prop.range.end, - value: &prop.value, - }); - } - Some(members) + .map(|prop| { + let name = prop.name.as_str(); + let occurrence = occurrences.entry(name).or_insert(0); + let step = Step::Prop(name, *occurrence); + *occurrence += 1; + Member { + step, + range: prop.range.start..prop.range.end, + value: &prop.value, + } + }) + .collect(), + ) } Value::Array(array) => Some( array @@ -205,7 +235,7 @@ fn members<'a>(value: &'a Value<'a>) -> Option>> { .enumerate() .map(|(index, element)| Member { step: Step::Element(index), - range: range_of(element), + range: element.start()..element.end(), value: element, }) .collect(), @@ -214,12 +244,8 @@ fn members<'a>(value: &'a Value<'a>) -> Option>> { } } -fn is_container(value: &Value) -> bool { - matches!(value, Value::Object(_) | Value::Array(_)) -} - fn is_within_brackets(value: &Value, range: &Range) -> bool { - is_container(value) && range.start > value.start() && range.end < value.end() + matches!(value, Value::Object(_) | Value::Array(_)) && range.start > value.start() && range.end < value.end() } fn touches(member: &Range, range: &Range) -> bool { @@ -230,33 +256,97 @@ fn touches(member: &Range, range: &Range) -> bool { } } -/// Gets whether there's a line break before the first member and after the last member of the -/// provided inclusive range of member indexes. -fn line_breaks_around(container: &Value, members: &[Member], indexes: Range, text: &str) -> (bool, bool) { - let first = &members[indexes.start]; - let last = &members[indexes.end]; - let before_start = match indexes.start { - 0 => container.start(), - index => members[index - 1].range.end, - }; - let after_end = members - .get(indexes.end + 1) - .map(|member| member.range.start) - .unwrap_or(container.end()); - ( - text[before_start..first.range.start].contains('\n'), - text[last.range.end..after_end].contains('\n'), - ) +/// The text between a run of members and what's on either side of them. +struct Gaps { + /// From the end of the previous member or the opening bracket to the first member. + before: Range, + /// From the last member to the start of the next member or the closing bracket. + after: Range, } -fn range_of(value: &Value) -> Range { - value.start()..value.end() +/// Where the replaced text starts and ends. +#[derive(Clone, Copy)] +struct Edges { + /// Starts at the beginning of the first member's line rather than after what's before it. + line_start: bool, + /// Ends at the end of the last member's line rather than at what's after it. + line_end: bool, +} + +impl Gaps { + fn new(container: &Value, members: &[Member], indexes: RangeInclusive) -> Self { + let before_start = match *indexes.start() { + 0 => container.start() + 1, // after the opening bracket + index => members[index - 1].range.end, + }; + let after_end = members + .get(indexes.end() + 1) + .map(|member| member.range.start) + .unwrap_or(container.end() - 1); // before the closing bracket + Gaps { + before: before_start..members[*indexes.start()].range.start, + after: members[*indexes.end()].range.end..after_end, + } + } + + /// Gets whether there's a line break before the first member and after the last member. + fn line_breaks(&self, text: &str) -> (bool, bool) { + ( + text[self.before.clone()].contains('\n'), + text[self.after.clone()].contains('\n'), + ) + } + + /// Goes to the start of the first member's line to fix its indentation and to the end of the + /// last member's line to include its separator and any trailing comment. A member sharing its + /// line with what's around it, or with the separator on the other side of the line break + /// (ex. comma-first style), goes to what's around it instead. + fn edges(&self, text: &str, commas: &[usize]) -> Edges { + let line_start = text[self.before.clone()] + .rfind('\n') + .is_some_and(|index| !has_comma(commas, self.before.start + index..self.before.end)); + let line_end = text[self.after.clone()] + .find('\n') + .is_some_and(|index| !has_comma(commas, self.after.start + index..self.after.end)); + Edges { line_start, line_end } + } + + fn replaced_range(&self, text: &str, edges: Edges) -> Range { + let start = match text[self.before.clone()].rfind('\n') { + Some(index) if edges.line_start => self.before.start + index + 1, + _ => self.before.start, + }; + let end = match text[self.after.clone()].find('\n') { + Some(index) if edges.line_end => { + let line = &text[self.after.start..self.after.start + index]; + self.after.start + line.trim_end_matches('\r').len() + } + _ => self.after.end, + }; + start..end + } +} + +fn has_comma(commas: &[usize], range: Range) -> bool { + let index = commas.partition_point(|comma| *comma < range.start); + commas.get(index).is_some_and(|comma| *comma < range.end) +} + +/// Keeps the line endings of the rest of the file since changing those is up to formatting the +/// whole file. +fn with_file_new_lines<'a>(formatted: &'a str, file_text: &str) -> Cow<'a, str> { + if !file_text.contains('\n') { + return Cow::Borrowed(formatted); + } + match resolve_new_line_kind(file_text, NewLineKind::Auto) { + "\r\n" if formatted.contains('\n') && !formatted.contains("\r\n") => Cow::Owned(formatted.replace('\n', "\r\n")), + "\n" if formatted.contains("\r\n") => Cow::Owned(formatted.replace("\r\n", "\n")), + _ => Cow::Borrowed(formatted), + } } #[cfg(test)] mod tests { - use std::path::Path; - use crate::configuration::ConfigurationBuilder; use super::*; @@ -272,4 +362,25 @@ mod tests { .unwrap(); assert_eq!(output, "\u{FEFF}{\n \"a\":1,\n \"b\": 2\n}\n"); } + + #[test] + fn keeps_file_line_endings() { + // the spec files can't express this since they normalize line endings + let config = ConfigurationBuilder::new().build(); + let output = format_b(&config, "{\r\n \"a\":1,\r\n \"b\":{\r\n\"x\":1}\r\n}\r\n"); + assert_eq!(output, "{\r\n \"a\":1,\r\n \"b\": {\r\n \"x\": 1\r\n }\r\n}\r\n"); + + let config = ConfigurationBuilder::new() + .new_line_kind(NewLineKind::CarriageReturnLineFeed) + .build(); + let output = format_b(&config, "{\n \"a\":1,\n \"b\":{\n\"x\":1}\n}\n"); + assert_eq!(output, "{\n \"a\":1,\n \"b\": {\n \"x\": 1\n }\n}\n"); + } + + fn format_b(config: &Configuration, text: &str) -> String { + let start = text.find("\"b\"").unwrap(); + format_text_range(Path::new("/file.json"), text, start..start + 3, config) + .unwrap() + .unwrap() + } } diff --git a/tests/specs/range/Range_Containers.txt b/tests/specs/range/Range_Containers.txt index 8b4e32f..c44291a 100644 --- a/tests/specs/range/Range_Containers.txt +++ b/tests/specs/range/Range_Containers.txt @@ -11,7 +11,7 @@ [|"b": { "c": 1, "d": 2 }|] } -== should format the whole object when the line breaks around the members change == +== should format the member holding the object when the line breaks around the members change == { "a":1, "b":{ @@ -21,7 +21,7 @@ [expect] { "a":1, - "b":{ + "b": { "c": 1, "d": 2 } diff --git a/tests/specs/range/Range_Lines.txt b/tests/specs/range/Range_Lines.txt new file mode 100644 index 0000000..4cf7e34 --- /dev/null +++ b/tests/specs/range/Range_Lines.txt @@ -0,0 +1,118 @@ +== should fix the indentation of the touched line == +{ + "a": 1, + [|"b":2|], + "c": 3 +} + +[expect] +{ + "a": 1, + [|"b": 2,|] + "c": 3 +} + +== should indent a multi-line member consistently == +{ + "a": 1, + [|"b": { +"x":1}|], + "c": 3 +} + +[expect] +{ + "a": 1, + [|"b": { + "x": 1 + },|] + "c": 3 +} + +== should format the separator and trailing comments of the last member == +{ + "a": 1, + [|"b":2|] , /*x*/ // y + "c": 3 +} + +[expect] +{ + "a": 1, + [|"b": 2, /*x*/ // y|] + "c": 3 +} + +== should add a missing comma == +{ + "a": 1, + [|"b":2|] + "c": 3 +} + +[expect] +{ + "a": 1, + [|"b": 2,|] + "c": 3 +} + +== should format the separator before a member on the same line == +{ + "a": [1,[|2|],3] +} + +[expect] +{ + "a": [1, [|2|], 3] +} + +== should format the member holding an array when the line breaks around its parent change == +{ + "a": 1, + "o": {"p": [ + 1, 2, + [|3|], 4 +]} +} + +[expect] +{ + "a": 1, + "o": { + "p": [ + 1, + 2, + 3, + 4 + ] + } +} + +== should not change anything for a cursor before the root value == +// comment[||] +{"a":1} + +[expect] +// comment +{"a":1} + +== should not change anything for a cursor after the root value == +{"a":1}[||] + +[expect] +{"a":1} + +== should keep separators with comma-first style == +[ + 1 + , [|2|] + , 3 +] + +[expect] +[ + 1, + 2, + 3 +] diff --git a/tests/specs/range/Range_PackageJson.txt b/tests/specs/range/Range_PackageJson.txt index 6aa6de4..86c2e47 100644 --- a/tests/specs/range/Range_PackageJson.txt +++ b/tests/specs/range/Range_PackageJson.txt @@ -9,23 +9,38 @@ [expect] { - [|"version": "1.0.0"|], + [|"version": "1.0.0",|] "dependencies": {"b":"1", "a":"1"}, "name":"a" } -== should format the whole object when the conventions reorder its members == +== should format the member holding an object the conventions reorder == { + "name":"a", "version":"1.0.0", "dependencies": {[|"b":"1", - "a"|]:"1"}, - "name":"a" + "a"|]:"1"} } [expect] +{ + "name":"a", + "version":"1.0.0", + "dependencies": { "a": "1", "b": "1" } +} + +== should format the file when a member moved to the end would lose its comma == { "version":"1.0.0", - "dependencies": { "a": "1", "b": "1" }, + "dependencies": {[|"b":"1", + "a"|]:"1"}, "name":"a" } + +[expect] +{ + "name": "a", + "version": "1.0.0", + "dependencies": { "a": "1", "b": "1" } +} diff --git a/tests/specs/range/Range_TrailingCommas_Always.txt b/tests/specs/range/Range_TrailingCommas_Always.txt new file mode 100644 index 0000000..6fedda5 --- /dev/null +++ b/tests/specs/range/Range_TrailingCommas_Always.txt @@ -0,0 +1,12 @@ +~~ trailingCommas: always ~~ +== should add the trailing comma to the last member == +{ + "a": 1, + [|"c":3|] +} + +[expect] +{ + "a": 1, + [|"c": 3,|] +} diff --git a/tests/specs/range/Range_TrailingCommas_Never.txt b/tests/specs/range/Range_TrailingCommas_Never.txt new file mode 100644 index 0000000..f37337d --- /dev/null +++ b/tests/specs/range/Range_TrailingCommas_Never.txt @@ -0,0 +1,18 @@ +~~ trailingCommas: never ~~ +== should remove the trailing comma from the last member == +{ + "a": 1, + [|"c":3|], +} + +[expect] +{ + "a": 1, + [|"c": 3|] +} + +== should remove the trailing comma in a single line array == +{"x": [1, [|2|],]} + +[expect] +{"x": [1, 2]} From 93150a120476c41283f6d99a60eb57d92abe9d2c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 27 Sep 2026 18:17:20 -0400 Subject: [PATCH 3/3] chore: upgrade dprint-development to 0.12.1 --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c84f29a..1d5bc9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -174,9 +174,9 @@ dependencies = [ [[package]] name = "dprint-development" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b380bab000ac6a5321d235375cfb1a727b5ff48a27891dd018dc9f741e7cba9a" +checksum = "b6696b3acf01bba505d35c400115ce277e6a5208ce41defa11799c53f6d02c5e" dependencies = [ "console", "file_test_runner", diff --git a/Cargo.toml b/Cargo.toml index e0c0bf0..0c28257 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,5 +35,5 @@ thiserror = "2" [dev-dependencies] debug-here = "0.2" -dprint-development = "0.12.0" +dprint-development = "0.12.1" serde_json = { version = "1.0" }