fix: improve formatting array with only comments

Author: dsherret

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"] }

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<SyntaxKind>, 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::<Vec<_>>();
+      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::<Vec<_>>(),
+          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<Item =
   })
 }
 
-fn get_child_with_kind(node: SyntaxNode, kind: SyntaxKind) -> Result<SyntaxNode, ()> {
+fn get_child_with_kind(node: &SyntaxNode, kind: SyntaxKind) -> Result<SyntaxNode, ()> {
   match node.children().find(|c| c.kind() == kind) {
     Some(node) => Ok(node),
     None => Err(()),
   }
 }
 
-fn get_token_with_kind(node: SyntaxNode, kind: SyntaxKind) -> Result<SyntaxToken, ()> {
+fn get_token_with_kind(node: &SyntaxNode, kind: SyntaxKind) -> Result<SyntaxToken, ()> {
   match node.children_with_tokens().find(|c| c.kind() == kind) {
     Some(NodeOrToken::Token(token)) => Ok(token),
     _ => Err(()),

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")
+    }),
+  )
+}

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
+]

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();