From 94b34f57b3e9c3800b7ca5f1744510c7c42cceb7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 30 Sep 2024 08:39:43 -0400 Subject: [PATCH] fix: improve formatting array with only comments --- Cargo.toml | 5 ++ src/generation/generate.rs | 30 ++++++---- tests/spec_test.rs | 44 ++++++++++++++ tests/specs/Array/ArrayWithComments.txt | 78 +++++++++++++++++++++++++ tests/test.rs | 48 --------------- 5 files changed, 145 insertions(+), 60 deletions(-) create mode 100644 tests/spec_test.rs create mode 100644 tests/specs/Array/ArrayWithComments.txt diff --git a/Cargo.toml b/Cargo.toml index ad5d278..05fa4a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,11 @@ panic = "abort" wasm = ["serde_json", "dprint-core/wasm"] tracing = ["dprint-core/tracing"] +[[test]] +name = "specs" +path = "tests/spec_test.rs" +harness = false + [dependencies] anyhow = "1.0.65" dprint-core = { version = "0.66.2", features = ["formatting"] } diff --git a/src/generation/generate.rs b/src/generation/generate.rs index 4cae94b..84abfa2 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -35,7 +35,7 @@ fn gen_node<'a>(node: SyntaxElement, context: &mut Context<'a>) -> PrintItems { fn gen_node_with_inner<'a>(node: SyntaxElement, context: &mut Context<'a>, inner_parse: impl FnOnce(PrintItems, &mut Context<'a>) -> PrintItems) -> PrintItems { let mut items = PrintItems::new(); - // println!("{:?}", node); + // eprintln!("{:?}", node); if node.kind() != SyntaxKind::COMMENT { for comment in node.get_comments_on_previous_lines() { @@ -146,17 +146,24 @@ fn allow_blank_line(previous_kind: Option, current_kind: SyntaxKind) fn gen_array<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintItemsResult { let values = node.children(); - let open_token = get_token_with_kind(node.clone(), SyntaxKind::BRACKET_START)?; - let close_token = get_token_with_kind(node.clone(), SyntaxKind::BRACKET_END)?; + let open_token = get_token_with_kind(&node, SyntaxKind::BRACKET_START)?; + let close_token = get_token_with_kind(&node, SyntaxKind::BRACKET_END)?; let is_in_inline_table = node.ancestors().any(|a| a.kind() == SyntaxKind::INLINE_TABLE); - let force_use_new_lines = !is_in_inline_table && has_following_newline(open_token.clone()); + let force_use_new_lines = !is_in_inline_table && has_following_newline(open_token.clone()) && node.children_with_tokens().skip(3).next().is_some(); ensure_all_kind(values.clone(), SyntaxKind::VALUE)?; Ok(gen_surrounded_by_tokens( |context| { + let nodes = values.into_iter().map(|v| v.into()).collect::>(); + if nodes.is_empty() { + if force_use_new_lines { + return Signal::NewLine.into(); + } + return PrintItems::new(); + } gen_comma_separated_values( ParseCommaSeparatedValuesOptions { - nodes: values.into_iter().map(|v| v.into()).collect::>(), + nodes, prefer_hanging: false, force_use_new_lines, allow_blank_lines: true, @@ -205,8 +212,8 @@ fn gen_inline_table<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintIte } fn gen_entry<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintItemsResult { - let key = get_child_with_kind(node.clone(), SyntaxKind::KEY)?; - let value = get_child_with_kind(node.clone(), SyntaxKind::VALUE)?; + let key = get_child_with_kind(&node, SyntaxKind::KEY)?; + let value = get_child_with_kind(&node, SyntaxKind::VALUE)?; let mut items = PrintItems::new(); items.extend(gen_node(key.into(), context)); @@ -236,7 +243,7 @@ fn gen_children_inline<'a>(node: SyntaxNode, context: &mut Context<'a>) -> Print fn gen_table_header<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintItemsResult { // Spec: Naming rules for tables are the same as for keys - let key = get_child_with_kind(node.clone(), SyntaxKind::KEY)?; + let key = get_child_with_kind(&node, SyntaxKind::KEY)?; let mut items = PrintItems::new(); items.push_sc(sc!("[")); items.extend(gen_node(key.into(), context)); @@ -246,7 +253,7 @@ fn gen_table_header<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintIte fn gen_table_array_header<'a>(node: SyntaxNode, context: &mut Context<'a>) -> PrintItemsResult { // Spec: Naming rules for tables are the same as for keys - let key = get_child_with_kind(node.clone(), SyntaxKind::KEY)?; + let key = get_child_with_kind(&node, SyntaxKind::KEY)?; let mut items = PrintItems::new(); items.push_sc(sc!("[[")); items.extend(gen_node(key.into(), context)); @@ -264,7 +271,6 @@ fn gen_surrounded_by_tokens<'a, 'b>( opts: ParseSurroundedByTokensParams, context: &mut Context<'a>, ) -> PrintItems { - // parse let mut items = PrintItems::new(); items.extend(gen_node(opts.open_token.clone().into(), context)); @@ -533,14 +539,14 @@ fn get_children_with_non_trivia_tokens(node: SyntaxNode) -> impl Iterator Result { +fn get_child_with_kind(node: &SyntaxNode, kind: SyntaxKind) -> Result { match node.children().find(|c| c.kind() == kind) { Some(node) => Ok(node), None => Err(()), } } -fn get_token_with_kind(node: SyntaxNode, kind: SyntaxKind) -> Result { +fn get_token_with_kind(node: &SyntaxNode, kind: SyntaxKind) -> Result { match node.children_with_tokens().find(|c| c.kind() == kind) { Some(NodeOrToken::Token(token)) => Ok(token), _ => Err(()), diff --git a/tests/spec_test.rs b/tests/spec_test.rs new file mode 100644 index 0000000..4498a90 --- /dev/null +++ b/tests/spec_test.rs @@ -0,0 +1,44 @@ +use std::path::PathBuf; +use std::sync::Arc; + +use dprint_core::configuration::*; +use dprint_development::*; +use dprint_plugin_toml::configuration::resolve_config; +use dprint_plugin_toml::*; + +fn main() { + let global_config = GlobalConfiguration::default(); + + run_specs( + &PathBuf::from("./tests/specs"), + &ParseSpecOptions { + default_file_name: "file.toml", + }, + &RunSpecsOptions { + fix_failures: false, + format_twice: true, + }, + { + let global_config = global_config.clone(); + Arc::new(move |file_path, file_text, 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(file_path, &file_text, &config_result.config) + }) + }, + Arc::new(move |_file_path, _file_text, _spec_config| { + #[cfg(feature = "tracing")] + { + 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); + return serde_json::to_string(&trace_file(_file_path, _file_text, &config_result.config)).unwrap(); + } + + #[cfg(not(feature = "tracing"))] + panic!("\n====\nPlease run with `cargo test --features tracing` to get trace output\n====\n") + }), + ) +} diff --git a/tests/specs/Array/ArrayWithComments.txt b/tests/specs/Array/ArrayWithComments.txt new file mode 100644 index 0000000..02698d4 --- /dev/null +++ b/tests/specs/Array/ArrayWithComments.txt @@ -0,0 +1,78 @@ +== should not add blank line at start of array with only comments == +disallowed-types = [ + # some comment + # { path = "std::sync::Arc", reason = "use crate::sync::MaybeArc instead" }, + + # next comment +] + +[expect] +disallowed-types = [ + # some comment + # { path = "std::sync::Arc", reason = "use crate::sync::MaybeArc instead" }, + + # next comment +] + +== should format with nodes == +disallowed-types = [ + # some comment + # { path = "std::sync::Arc", reason = "use crate::sync::MaybeArc instead" }, + + # next comment + "test" +] + +[expect] +disallowed-types = [ + # some comment + # { path = "std::sync::Arc", reason = "use crate::sync::MaybeArc instead" }, + + # next comment + "test", +] + +== comment first line == +disallowed-types = [ # comment + "test", +] + +[expect] +disallowed-types = [ # comment + "test", +] + +== comment first line no elements == +disallowed-types = [ # comment +] + +[expect] +disallowed-types = [ # comment +] + +== empty == +value = [ +] + +[expect] +value = [] + +== one element == +value = [ + "value" +] + +[expect] +value = [ + "value", +] + +== one comment == +value = [ + # comment +] + +[expect] +value = [ + # comment +] diff --git a/tests/test.rs b/tests/test.rs index 554b271..6e00d9a 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,56 +1,8 @@ -extern crate dprint_development; -extern crate dprint_plugin_toml; - -//#[macro_use] extern crate debug_here; - use std::path::PathBuf; -use std::sync::Arc; -// use std::time::Instant; -use dprint_core::configuration::*; -use dprint_development::*; -use dprint_plugin_toml::configuration::resolve_config; use dprint_plugin_toml::configuration::ConfigurationBuilder; use dprint_plugin_toml::*; -#[test] -fn test_specs() { - //debug_here!(); - let global_config = GlobalConfiguration::default(); - - run_specs( - &PathBuf::from("./tests/specs"), - &ParseSpecOptions { - default_file_name: "file.toml", - }, - &RunSpecsOptions { - fix_failures: false, - format_twice: true, - }, - { - let global_config = global_config.clone(); - Arc::new(move |file_path, file_text, 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(file_path, &file_text, &config_result.config) - }) - }, - Arc::new(move |_file_path, _file_text, _spec_config| { - #[cfg(feature = "tracing")] - { - let config_result = resolve_config(parse_config_key_map(_spec_config), &global_config); - ensure_no_diagnostics(&config_result.diagnostics); - return serde_json::to_string(&trace_file(_file_name, _file_text, &config_result.config)).unwrap(); - } - - #[cfg(not(feature = "tracing"))] - panic!("\n====\nPlease run with `cargo test --features tracing` to get trace output\n====\n") - }), - ) -} - #[test] fn should_handle_windows_newlines() { let config = ConfigurationBuilder::new().build();