diff --git a/src/formatting/blocks.rs b/src/formatting/blocks.rs index 3f49cb5..51c1351 100644 --- a/src/formatting/blocks.rs +++ b/src/formatting/blocks.rs @@ -39,11 +39,10 @@ impl<'a> Formatter<'a> { } if i < num_pipelines - 1 { - let separator_newlines = if self.indent_level == 0 && self.config.margin > 1 { - self.config.margin.saturating_add(1) - } else { - 1 - }; + let separator_newlines = self.separator_newlines_between_top_level_pipelines( + pipeline, + &block.pipelines[i + 1], + ); for _ in 0..separator_newlines { self.newline(); @@ -52,6 +51,81 @@ impl<'a> Formatter<'a> { } } + /// Decide how many newline characters to emit between adjacent pipelines. + /// + /// At top-level this respects `margin` while preserving author-intent groups + /// for `margin = 1`, and keeps adjacent `use` statements compact. + fn separator_newlines_between_top_level_pipelines( + &self, + current: &nu_protocol::ast::Pipeline, + next: &nu_protocol::ast::Pipeline, + ) -> usize { + if self.indent_level != 0 { + return 1; + } + + if self.is_use_pipeline(current) && self.is_use_pipeline(next) { + return 1; + } + + if self.config.margin == 1 { + let current_end = current + .elements + .last() + .map_or(0, |element| self.get_element_end_pos(element)); + let next_start = next + .elements + .first() + .map_or(current_end, |element| element.expr.span.start); + + if current_end < next_start { + let between = &self.source[current_end..next_start]; + if between.contains(&b'#') { + return 1; + } + + let mut previous_newline: Option = None; + let mut has_blank_line = false; + for (idx, byte) in between.iter().enumerate() { + if *byte == b'\n' { + if let Some(prev) = previous_newline { + if between[prev + 1..idx] + .iter() + .all(|b| b.is_ascii_whitespace()) + { + has_blank_line = true; + break; + } + } + previous_newline = Some(idx); + } + } + + if has_blank_line { + return 2; + } + } + + return 1; + } + + self.config.margin.saturating_add(1) + } + + /// Whether a pipeline is a top-level `use` command. + fn is_use_pipeline(&self, pipeline: &nu_protocol::ast::Pipeline) -> bool { + let Some(first) = pipeline.elements.first() else { + return false; + }; + + let Expr::Call(call) = &first.expr.expr else { + return false; + }; + + let decl = self.working_set.get_decl(call.decl_id); + matches!(decl.name(), "use" | "export use") + } + /// Get the end position of a pipeline element, including redirections. fn get_element_end_pos(&self, element: &PipelineElement) -> usize { element @@ -98,7 +172,7 @@ impl<'a> Formatter<'a> { } /// Format a single pipeline element (expression + optional redirection). - fn format_pipeline_element(&mut self, element: &PipelineElement) { + pub(super) fn format_pipeline_element(&mut self, element: &PipelineElement) { self.format_expression(&element.expr); if let Some(ref redirection) = element.redirection { self.format_redirection(redirection); @@ -173,6 +247,7 @@ impl<'a> Formatter<'a> { && block.pipelines[0].elements.len() == 1 && !self.block_has_nested_structures(block) && !source_has_newline; + let has_comments_in_block_span = self.has_comments_in_span(span.start, span.end); let preserve_compact_record_like = with_braces && is_simple && self.block_expression_looks_like_compact_record(span); @@ -185,7 +260,13 @@ impl<'a> Formatter<'a> { self.write(" "); } } else if block.pipelines.is_empty() { - if with_braces { + if with_braces && has_comments_in_block_span { + self.newline(); + self.indent_level += 1; + self.write_comments_before(span.end.saturating_sub(1)); + self.indent_level -= 1; + self.write_indent(); + } else if with_braces { self.write(" "); } } else { @@ -306,9 +387,11 @@ impl<'a> Formatter<'a> { self.write("|"); let block = self.working_set.get_block(block_id); + let has_comments = self.has_comments_in_span(span.start, span.end); let is_simple = block.pipelines.len() == 1 && block.pipelines[0].elements.len() == 1 - && !self.block_has_nested_structures(block); + && !self.block_has_nested_structures(block) + && !has_comments; if is_simple { self.space(); diff --git a/src/formatting/calls.rs b/src/formatting/calls.rs index 496fbb9..a95ffba 100644 --- a/src/formatting/calls.rs +++ b/src/formatting/calls.rs @@ -19,7 +19,7 @@ pub(super) const BLOCK_COMMANDS: &[&str] = &["for", "while", "loop", "module"]; pub(super) const CONDITIONAL_COMMANDS: &[&str] = &["if", "try"]; pub(super) const DEF_COMMANDS: &[&str] = &["def", "def-env", "export def"]; pub(super) const EXTERN_COMMANDS: &[&str] = &["extern", "export extern"]; -pub(super) const LET_COMMANDS: &[&str] = &["let", "let-env", "mut", "const"]; +pub(super) const LET_COMMANDS: &[&str] = &["let", "let-env", "mut", "const", "export const"]; impl<'a> Formatter<'a> { // ───────────────────────────────────────────────────────────────────────── @@ -32,6 +32,11 @@ impl<'a> Formatter<'a> { let decl_name = decl.name(); let cmd_type = Self::classify_command(decl_name); + if self.should_wrap_call_multiline(call, &cmd_type) { + self.format_wrapped_call(call); + return; + } + // Write command name if call.head.end != 0 { self.write_span(call.head); @@ -52,6 +57,86 @@ impl<'a> Formatter<'a> { } } + /// Decide if a call should be emitted as a parenthesized multiline call. + fn should_wrap_call_multiline( + &self, + call: &nu_protocol::ast::Call, + cmd_type: &CommandType, + ) -> bool { + if !matches!(cmd_type, CommandType::Regular) || call.arguments.len() < 3 { + return false; + } + + if !call.arguments.iter().all(|arg| { + matches!( + arg, + Argument::Positional(_) | Argument::Unknown(_) | Argument::Spread(_) + ) + }) { + return false; + } + + let end = call + .arguments + .iter() + .map(|arg| match arg { + Argument::Positional(expr) | Argument::Unknown(expr) | Argument::Spread(expr) => { + expr.span.end + } + Argument::Named(named) => named + .2 + .as_ref() + .map_or(named.0.span.end, |value| value.span.end), + }) + .max() + .unwrap_or(call.head.end); + + if call.head.start >= end || end > self.source.len() { + return false; + } + + let source_span = &self.source[call.head.start..end]; + if source_span.contains(&b'\n') { + return false; + } + + source_span.len() > self.config.line_length + } + + /// Format a long regular call as: + /// + /// `(cmd\n arg1\n arg2\n)` + fn format_wrapped_call(&mut self, call: &nu_protocol::ast::Call) { + self.write("("); + if call.head.end != 0 { + self.write_span(call.head); + } + self.newline(); + self.indent_level += 1; + + for arg in &call.arguments { + self.write_indent(); + match arg { + Argument::Positional(expr) | Argument::Unknown(expr) => { + self.format_expression(expr); + } + Argument::Spread(expr) => { + self.write("..."); + self.format_expression(expr); + } + Argument::Named(_) => { + // Guarded out by should_wrap_call_multiline. + self.format_call_argument(arg, &CommandType::Regular); + } + } + self.newline(); + } + + self.indent_level -= 1; + self.write_indent(); + self.write(")"); + } + /// Format `let`/`mut`/`const` calls while preserving explicit type annotations. pub(super) fn format_let_call(&mut self, call: &nu_protocol::ast::Call) { let positional: Vec<&Expression> = call @@ -290,7 +375,7 @@ impl<'a> Formatter<'a> { Expr::VarDecl(_) => self.format_expression(positional), Expr::Subexpression(block_id) => { self.write("= "); - self.format_subexpression(*block_id, positional.span); + self.format_assignment_subexpression(*block_id, positional.span); } Expr::Block(block_id) => { self.write("= "); @@ -304,6 +389,37 @@ impl<'a> Formatter<'a> { } } + /// Format let-assignment subexpressions, flattening redundant outer + /// parentheses around pipeline-leading subexpressions such as + /// `((pwd) | path join ...)`. + fn format_assignment_subexpression( + &mut self, + block_id: nu_protocol::BlockId, + span: nu_protocol::Span, + ) { + let block = self.working_set.get_block(block_id); + if block.pipelines.len() == 1 { + let pipeline = &block.pipelines[0]; + if pipeline.elements.len() > 1 + && matches!(pipeline.elements[0].expr.expr, Expr::Subexpression(_)) + { + if let Expr::Subexpression(inner_id) = &pipeline.elements[0].expr.expr { + let inner = self.working_set.get_block(*inner_id); + if inner.pipelines.len() == 1 && inner.pipelines[0].elements.len() == 1 { + self.format_pipeline_element(&inner.pipelines[0].elements[0]); + for element in pipeline.elements.iter().skip(1) { + self.write(" | "); + self.format_pipeline_element(element); + } + return; + } + } + } + } + + self.format_subexpression(block_id, span); + } + /// Format an external call (e.g. `^git status`). pub(super) fn format_external_call(&mut self, head: &Expression, args: &[ExternalArgument]) { // Preserve explicit `^` prefix @@ -335,7 +451,11 @@ impl<'a> Formatter<'a> { + sig.optional_positional.len() + sig.named.iter().filter(|f| f.long != "help").count() + usize::from(sig.rest_positional.is_some()); - let has_multiline = param_count > 3; + let has_multiline = if self.should_keep_simple_signature_inline(sig) { + false + } else { + param_count > 3 + }; if has_multiline { self.newline(); @@ -451,6 +571,37 @@ impl<'a> Formatter<'a> { } } + /// Keep simple required-positional signatures inline when they fit the + /// configured line length. + fn should_keep_simple_signature_inline(&self, sig: &Signature) -> bool { + if sig.required_positional.is_empty() + || !sig.optional_positional.is_empty() + || sig.rest_positional.is_some() + || !sig.input_output_types.is_empty() + || sig.named.iter().any(|flag| flag.long != "help") + { + return false; + } + + if sig + .required_positional + .iter() + .any(|param| param.shape != SyntaxShape::Any || param.completion.is_some()) + { + return false; + } + + let inline_len = 2 + + sig + .required_positional + .iter() + .map(|param| param.name.len()) + .sum::() + + sig.required_positional.len().saturating_sub(1) * 2; + + inline_len <= self.config.line_length + } + // ───────────────────────────────────────────────────────────────────────── // Custom completions and shapes // ───────────────────────────────────────────────────────────────────────── diff --git a/src/formatting/collections.rs b/src/formatting/collections.rs index 03a2a20..787987f 100644 --- a/src/formatting/collections.rs +++ b/src/formatting/collections.rs @@ -99,6 +99,9 @@ impl<'a> Formatter<'a> { return; } + let source_has_newline = + span.end > span.start && self.source[span.start..span.end].contains(&b'\n'); + let preserve_compact = self.record_preserve_compact_style(span); let all_simple = items.iter().all(|item| match item { @@ -122,7 +125,9 @@ impl<'a> Formatter<'a> { // Records with 2+ items and complex values should be multiline when nested let nested_multiline = self.indent_level > 0 && items.len() >= 2 && has_nested_complex; - if all_simple && items.len() <= 3 && !nested_multiline { + let preserve_multiline_top_level = source_has_newline && self.indent_level == 0; + + if all_simple && items.len() <= 3 && !nested_multiline && !preserve_multiline_top_level { // Inline format let record_start = self.output.len(); self.write("{"); @@ -278,6 +283,10 @@ impl<'a> Formatter<'a> { for (pattern, expr) in matches { self.write_indent(); self.format_match_pattern(pattern); + if let Some(guard) = &pattern.guard { + self.write(" if "); + self.format_expression(guard); + } self.write(" => "); self.format_block_or_expr(expr); self.newline(); diff --git a/src/formatting/expressions.rs b/src/formatting/expressions.rs index f5b9685..6728dc5 100644 --- a/src/formatting/expressions.rs +++ b/src/formatting/expressions.rs @@ -30,11 +30,16 @@ impl<'a> Formatter<'a> { | Expr::StringInterpolation(_) | Expr::GlobInterpolation(_, _) | Expr::ImportPattern(_) - | Expr::Overlay(_) - | Expr::Garbage => { + | Expr::Overlay(_) => { self.write_expr_span(expr); } + Expr::Garbage => { + if !self.try_write_redundant_pipeline_subexpr_without_outer_parens(expr) { + self.write_expr_span(expr); + } + } + // Glob patterns — normalise empty-brace globs like `{ }` to `{}` // (the parser treats `{ }` in unknown-command args as a glob). Expr::GlobPattern(_, _) => { @@ -233,6 +238,13 @@ impl<'a> Formatter<'a> { } if self.conditional_context_depth > 0 { + if self.preserve_subexpr_parens_depth > 0 { + self.write("("); + self.format_block(block); + self.write(")"); + return; + } + let can_drop_parens = block.pipelines.len() == 1 && block.pipelines[0].elements.len() == 1; if can_drop_parens { @@ -311,6 +323,49 @@ impl<'a> Formatter<'a> { // Helpers // ───────────────────────────────────────────────────────────────────────── + /// Best-effort normalisation for a narrow garbage case: + /// `((head) | tail)` -> `head | tail`. + fn try_write_redundant_pipeline_subexpr_without_outer_parens( + &mut self, + expr: &Expression, + ) -> bool { + let raw = &self.source[expr.span.start..expr.span.end]; + if raw.contains(&b'\n') || raw.len() < 5 { + return false; + } + + if !(raw.starts_with(b"((") && raw.ends_with(b")") && raw.contains(&b'|')) { + return false; + } + + let Some(inner) = raw.get(1..raw.len() - 1) else { + return false; + }; + let Some(pipe_idx) = inner.iter().position(|b| *b == b'|') else { + return false; + }; + + let left = &inner[..pipe_idx]; + let right = &inner[pipe_idx + 1..]; + let left_trimmed = left.trim_ascii(); + + if !(left_trimmed.starts_with(b"(") && left_trimmed.ends_with(b")")) { + return false; + } + + let Some(unwrapped_left) = left_trimmed.get(1..left_trimmed.len() - 1) else { + return false; + }; + if unwrapped_left.is_empty() { + return false; + } + + self.write_bytes(unwrapped_left); + self.write(" | "); + self.write_bytes(right.trim_ascii()); + true + } + /// Check if an expression is a simple primitive (used by collection /// formatting to decide inline vs. multiline layout). pub(super) fn is_simple_expr(&self, expr: &Expression) -> bool { @@ -327,11 +382,12 @@ impl<'a> Formatter<'a> { | Expr::GlobPattern(_, _) | Expr::DateTime(_) => true, Expr::FullCellPath(full_path) => { - full_path.tail.is_empty() - && matches!( - &full_path.head.expr, - Expr::Var(_) | Expr::Garbage | Expr::Int(_) | Expr::String(_) - ) + matches!( + &full_path.head.expr, + Expr::Var(_) | Expr::Garbage | Expr::Int(_) | Expr::String(_) + ) && full_path.tail.iter().all(|member| { + matches!(member, PathMember::String { .. } | PathMember::Int { .. }) + }) } _ => false, } diff --git a/src/formatting/mod.rs b/src/formatting/mod.rs index 0f80839..15397f5 100644 --- a/src/formatting/mod.rs +++ b/src/formatting/mod.rs @@ -31,8 +31,9 @@ use comments::extract_comments; use engine::get_engine_state; use garbage::block_contains_garbage; use repair::{ - detect_compact_if_else_spans, detect_missing_record_comma_spans, is_fatal_parse_error, - try_repair_parse_errors, ParseRepairOutcome, + detect_compact_if_else_spans, detect_missing_record_comma_spans, + detect_redundant_pipeline_subexpr_spans, is_fatal_parse_error, try_repair_parse_errors, + ParseRepairOutcome, }; // ───────────────────────────────────────────────────────────────────────────── @@ -220,6 +221,7 @@ fn format_inner_with_options(contents: &[u8], config: &Config) -> Result .collect(); malformed_spans.extend(detect_compact_if_else_spans(&source_text)); malformed_spans.extend(detect_missing_record_comma_spans(&source_text)); + malformed_spans.extend(detect_redundant_pipeline_subexpr_spans(&source_text)); let has_garbage = block_contains_garbage(&working_set, &parsed_block); let has_fatal_parse_error = working_set.parse_errors.iter().any(is_fatal_parse_error); diff --git a/src/formatting/repair.rs b/src/formatting/repair.rs index 1ea41bb..7759b97 100644 --- a/src/formatting/repair.rs +++ b/src/formatting/repair.rs @@ -260,6 +260,31 @@ pub(super) fn detect_missing_record_comma_spans(source: &str) -> Vec { .collect() } +/// Detect spans that likely contain redundant outer parentheses around +/// pipeline-leading subexpressions, such as `((pwd) | where true)`. +pub(super) fn detect_redundant_pipeline_subexpr_spans(source: &str) -> Vec { + let mut spans = Vec::new(); + + for (range_start, range_end) in non_string_ranges(source) { + let segment = &source[range_start..range_end]; + for (offset, _) in segment.match_indices("((") { + let idx = range_start + offset; + let line_end = source[idx..] + .find('\n') + .map_or(source.len(), |rel| idx + rel); + let line = &source[idx..line_end]; + if line.contains('|') && line.trim_end().ends_with(')') { + spans.push(Span { + start: idx.saturating_sub(32), + end: (line_end + 32).min(source.len()), + }); + } + } + } + + spans +} + /// Attempt to insert missing commas between record fields. fn try_repair_missing_record_commas(source: &str) -> (String, bool) { let insert_positions = detect_missing_record_comma_positions(source); @@ -283,6 +308,65 @@ fn try_repair_missing_record_commas(source: &str) -> (String, bool) { (repaired, true) } +/// Attempt to simplify redundant `((head) | tail)` wrappers line by line. +fn try_repair_redundant_pipeline_subexpr(source: &str) -> (String, bool) { + let mut output = String::with_capacity(source.len()); + let mut changed = false; + + for line in source.split_inclusive('\n') { + let (body, newline) = match line.strip_suffix('\n') { + Some(body) => (body, "\n"), + None => (line, ""), + }; + + let Some(start) = body.find("((") else { + output.push_str(line); + continue; + }; + + let candidate = &body[start..]; + let candidate_trimmed = candidate.trim_end(); + if !(candidate_trimmed.contains('|') && candidate_trimmed.ends_with(')')) { + output.push_str(line); + continue; + } + + let Some(inner) = candidate_trimmed.get(1..candidate_trimmed.len() - 1) else { + output.push_str(line); + continue; + }; + let Some(pipe_idx) = inner.find('|') else { + output.push_str(line); + continue; + }; + + let left = inner[..pipe_idx].trim(); + let right = inner[pipe_idx + 1..].trim(); + if !(left.starts_with('(') && left.ends_with(')')) { + output.push_str(line); + continue; + } + + let Some(unwrapped_left) = left.get(1..left.len() - 1) else { + output.push_str(line); + continue; + }; + if unwrapped_left.trim().is_empty() || right.is_empty() { + output.push_str(line); + continue; + } + + output.push_str(&body[..start]); + output.push_str(unwrapped_left.trim()); + output.push_str(" | "); + output.push_str(right); + output.push_str(newline); + changed = true; + } + + (output, changed) +} + // ───────────────────────────────────────────────────────────────────────────── // Brace padding // ───────────────────────────────────────────────────────────────────────────── @@ -385,6 +469,9 @@ fn align_to_char_boundary(source: &str, index: usize, forward: bool) -> usize { fn repair_region(source: &str) -> (String, bool) { let (repaired_if_else, if_else_changed) = try_repair_compact_if_else(source); let (mut repaired_record, record_changed) = try_repair_missing_record_commas(&repaired_if_else); + let (repaired_pipeline, pipeline_changed) = + try_repair_redundant_pipeline_subexpr(&repaired_record); + repaired_record = repaired_pipeline; let mut brace_spacing_changed = false; if record_changed { @@ -395,7 +482,7 @@ fn repair_region(source: &str) -> (String, bool) { ( repaired_record, - if_else_changed || record_changed || brace_spacing_changed, + if_else_changed || record_changed || pipeline_changed || brace_spacing_changed, ) } diff --git a/src/main.rs b/src/main.rs index 75d6fea..6599f80 100644 --- a/src/main.rs +++ b/src/main.rs @@ -84,7 +84,9 @@ fn exit_with_code(exit_code: ExitCode) -> ! { } fn main() { - env_logger::init(); + if std::env::var_os("RUST_LOG").is_some() { + let _ = env_logger::try_init(); + } let cli = Cli::parse(); trace!("received cli.files: {:?}", cli.files); diff --git a/tests/fixtures/expected/issue126.nu b/tests/fixtures/expected/issue126.nu new file mode 100644 index 0000000..8cc346a --- /dev/null +++ b/tests/fixtures/expected/issue126.nu @@ -0,0 +1,2 @@ +use a.nu +use b.nu diff --git a/tests/fixtures/expected/issue127.nu b/tests/fixtures/expected/issue127.nu new file mode 100644 index 0000000..5a7bbda --- /dev/null +++ b/tests/fixtures/expected/issue127.nu @@ -0,0 +1,5 @@ +use a.nu + +def foo[] {1} + +def boo[] {2} diff --git a/tests/fixtures/expected/issue136.nu b/tests/fixtures/expected/issue136.nu new file mode 100644 index 0000000..93952e7 --- /dev/null +++ b/tests/fixtures/expected/issue136.nu @@ -0,0 +1,3 @@ +use a.nu +def abc [] { } +def xyz [] { } diff --git a/tests/fixtures/expected/issue137.nu b/tests/fixtures/expected/issue137.nu new file mode 100644 index 0000000..82c9f03 --- /dev/null +++ b/tests/fixtures/expected/issue137.nu @@ -0,0 +1,3 @@ +export const ERROR_CODES: record = { + timeout: "timeout" +} diff --git a/tests/fixtures/expected/issue138.nu b/tests/fixtures/expected/issue138.nu new file mode 100644 index 0000000..c86eeb9 --- /dev/null +++ b/tests/fixtures/expected/issue138.nu @@ -0,0 +1 @@ +export def main [callbacks, msg, state, opts] { } diff --git a/tests/fixtures/expected/issue139.nu b/tests/fixtures/expected/issue139.nu new file mode 100644 index 0000000..6103a05 --- /dev/null +++ b/tests/fixtures/expected/issue139.nu @@ -0,0 +1,3 @@ +match $msg_type { + $type if $type == $MSG_CALL => { } +} diff --git a/tests/fixtures/expected/issue140.nu b/tests/fixtures/expected/issue140.nu new file mode 100644 index 0000000..c6f98e4 --- /dev/null +++ b/tests/fixtures/expected/issue140.nu @@ -0,0 +1,8 @@ +if true { + try { + do $thing + } catch {|err| + # TODO: proper logging + print --stderr $"thing callback crashed ($err)" + } +} diff --git a/tests/fixtures/expected/issue141.nu b/tests/fixtures/expected/issue141.nu new file mode 100644 index 0000000..4f523b7 --- /dev/null +++ b/tests/fixtures/expected/issue141.nu @@ -0,0 +1,3 @@ +def main [] { + $var.state +} diff --git a/tests/fixtures/expected/issue142.nu b/tests/fixtures/expected/issue142.nu new file mode 100644 index 0000000..fad7dab --- /dev/null +++ b/tests/fixtures/expected/issue142.nu @@ -0,0 +1,4 @@ +def main [] { + ["short", "string", ""] + [$var.abc] +} diff --git a/tests/fixtures/expected/issue143.nu b/tests/fixtures/expected/issue143.nu new file mode 100644 index 0000000..6f2a019 --- /dev/null +++ b/tests/fixtures/expected/issue143.nu @@ -0,0 +1,3 @@ +if (errors $err) == "nu::shell::" { + error make $err +} diff --git a/tests/fixtures/expected/issue144.nu b/tests/fixtures/expected/issue144.nu new file mode 100644 index 0000000..d93fccf --- /dev/null +++ b/tests/fixtures/expected/issue144.nu @@ -0,0 +1,8 @@ +if $outcome.skipped { + (echo + $acc.passed + ($acc.failed + 1) + $acc.skipped + ($acc.failures | append $outcome.failure) + ) +} diff --git a/tests/fixtures/expected/issue145.nu b/tests/fixtures/expected/issue145.nu new file mode 100644 index 0000000..ef08328 --- /dev/null +++ b/tests/fixtures/expected/issue145.nu @@ -0,0 +1 @@ +let search_path = pwd | where true diff --git a/tests/fixtures/expected/issue146.nu b/tests/fixtures/expected/issue146.nu new file mode 100644 index 0000000..482b4f0 --- /dev/null +++ b/tests/fixtures/expected/issue146.nu @@ -0,0 +1,5 @@ +if true { + # noop +} else { + do $thing +} diff --git a/tests/fixtures/input/issue126.nu b/tests/fixtures/input/issue126.nu new file mode 100644 index 0000000..8cc346a --- /dev/null +++ b/tests/fixtures/input/issue126.nu @@ -0,0 +1,2 @@ +use a.nu +use b.nu diff --git a/tests/fixtures/input/issue127.nu b/tests/fixtures/input/issue127.nu new file mode 100644 index 0000000..9cc0493 --- /dev/null +++ b/tests/fixtures/input/issue127.nu @@ -0,0 +1,6 @@ +use a.nu + +def foo[] {1} + + +def boo[] {2} diff --git a/tests/fixtures/input/issue136.nu b/tests/fixtures/input/issue136.nu new file mode 100644 index 0000000..93952e7 --- /dev/null +++ b/tests/fixtures/input/issue136.nu @@ -0,0 +1,3 @@ +use a.nu +def abc [] { } +def xyz [] { } diff --git a/tests/fixtures/input/issue137.nu b/tests/fixtures/input/issue137.nu new file mode 100644 index 0000000..4d5c271 --- /dev/null +++ b/tests/fixtures/input/issue137.nu @@ -0,0 +1,3 @@ +export const ERROR_CODES: record = { +timeout: "timeout" +} diff --git a/tests/fixtures/input/issue138.nu b/tests/fixtures/input/issue138.nu new file mode 100644 index 0000000..a0366fd --- /dev/null +++ b/tests/fixtures/input/issue138.nu @@ -0,0 +1,3 @@ +export def main [callbacks, msg, state, opts] { + +} diff --git a/tests/fixtures/input/issue139.nu b/tests/fixtures/input/issue139.nu new file mode 100644 index 0000000..998c3f8 --- /dev/null +++ b/tests/fixtures/input/issue139.nu @@ -0,0 +1,3 @@ +match $msg_type { +$type if $type == $MSG_CALL => { } +} diff --git a/tests/fixtures/input/issue140.nu b/tests/fixtures/input/issue140.nu new file mode 100644 index 0000000..e0c3222 --- /dev/null +++ b/tests/fixtures/input/issue140.nu @@ -0,0 +1,8 @@ +if true { +try { + do $thing +} catch {|err| + # TODO: proper logging + print --stderr $"thing callback crashed ($err)" +} +} diff --git a/tests/fixtures/input/issue141.nu b/tests/fixtures/input/issue141.nu new file mode 100644 index 0000000..fa457b8 --- /dev/null +++ b/tests/fixtures/input/issue141.nu @@ -0,0 +1,3 @@ +def main [] { +$var.state +} diff --git a/tests/fixtures/input/issue142.nu b/tests/fixtures/input/issue142.nu new file mode 100644 index 0000000..abb88e2 --- /dev/null +++ b/tests/fixtures/input/issue142.nu @@ -0,0 +1,4 @@ +def main [] { +["short", "string", ""] +[$var.abc] +} diff --git a/tests/fixtures/input/issue143.nu b/tests/fixtures/input/issue143.nu new file mode 100644 index 0000000..d3c06a9 --- /dev/null +++ b/tests/fixtures/input/issue143.nu @@ -0,0 +1,3 @@ +if (errors $err) == "nu::shell::" { +error make $err +} diff --git a/tests/fixtures/input/issue144.nu b/tests/fixtures/input/issue144.nu new file mode 100644 index 0000000..ba43265 --- /dev/null +++ b/tests/fixtures/input/issue144.nu @@ -0,0 +1,3 @@ +if $outcome.skipped { + echo $acc.passed ($acc.failed + 1) $acc.skipped ($acc.failures | append $outcome.failure) +} diff --git a/tests/fixtures/input/issue145.nu b/tests/fixtures/input/issue145.nu new file mode 100644 index 0000000..6e5a4f6 --- /dev/null +++ b/tests/fixtures/input/issue145.nu @@ -0,0 +1 @@ +let search_path = ((pwd) | where true) diff --git a/tests/fixtures/input/issue146.nu b/tests/fixtures/input/issue146.nu new file mode 100644 index 0000000..33729c0 --- /dev/null +++ b/tests/fixtures/input/issue146.nu @@ -0,0 +1,5 @@ +if true { +# noop +} else { +do $thing +} diff --git a/tests/ground_truth.rs b/tests/ground_truth.rs index f15970e..621b276 100644 --- a/tests/ground_truth.rs +++ b/tests/ground_truth.rs @@ -369,6 +369,8 @@ fixture_tests!( ("issue120", issue120_test, idempotency_issue120_test), ("issue121", issue121_test, idempotency_issue121_test), ("issue122", issue122_test, idempotency_issue122_test), + ("issue126", issue126_test, idempotency_issue126_test), + ("issue127", issue127_test, idempotency_issue127_test), ("issue128", issue128_test, idempotency_issue128_test), ("issue129", issue129_test, idempotency_issue129_test), ("issue130", issue130_test, idempotency_issue130_test), @@ -376,4 +378,15 @@ fixture_tests!( ("issue132", issue132_test, idempotency_issue132_test), ("issue133", issue133_test, idempotency_issue133_test), ("issue134", issue134_test, idempotency_issue134_test), + ("issue136", issue136_test, idempotency_issue136_test), + ("issue137", issue137_test, idempotency_issue137_test), + ("issue138", issue138_test, idempotency_issue138_test), + ("issue139", issue139_test, idempotency_issue139_test), + ("issue140", issue140_test, idempotency_issue140_test), + ("issue141", issue141_test, idempotency_issue141_test), + ("issue142", issue142_test, idempotency_issue142_test), + ("issue143", issue143_test, idempotency_issue143_test), + ("issue144", issue144_test, idempotency_issue144_test), + ("issue145", issue145_test, idempotency_issue145_test), + ("issue146", issue146_test, idempotency_issue146_test), ); diff --git a/tests/main.rs b/tests/main.rs index f493bdf..5b0f55b 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -1,6 +1,6 @@ mod ground_truth; use ground_truth::get_test_binary; -use std::{fs, path::PathBuf, process::Command}; +use std::{fs, io::Write, path::PathBuf, process::Command}; use tempfile::tempdir; const INVALID: &str = "# beginning of script comment @@ -11,6 +11,25 @@ const VALID: &str = "# beginning of script comment let one = 1 "; +fn run_stdin(input: &str) -> std::process::Output { + let mut child = Command::new(get_test_binary()) + .arg("--stdin") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("Failed to spawn nufmt"); + + child + .stdin + .as_mut() + .expect("stdin should be piped") + .write_all(input.as_bytes()) + .expect("Failed to write stdin"); + + child.wait_with_output().expect("Failed to wait for nufmt") +} + #[test] fn failure_with_invalid_config() { let dir = tempdir().unwrap(); @@ -293,3 +312,94 @@ fn format_fixtures_basic() { assert!(output.status.code() == Some(0) || output.status.code() == Some(1)); } } + +#[test] +fn issue136_mixed_use_and_def_does_not_emit_parser_errors() { + let output = run_stdin("use a.nu\ndef abc [] { }\ndef xyz [] { }\n"); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert_eq!(output.status.code(), Some(0)); + assert!( + !stderr.contains("compile_block_with_id called with parse errors"), + "unexpected parser error noise on stderr: {stderr}" + ); +} + +#[test] +fn issue141_cell_path_in_def_block_does_not_emit_parser_errors() { + let output = run_stdin("def main [] {\n$var.state\n}\n"); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert_eq!(output.status.code(), Some(0)); + assert!( + !stderr.contains("compile_block_with_id called with parse errors"), + "unexpected parser error noise on stderr: {stderr}" + ); +} + +#[test] +fn issue126_margin_two_keeps_adjacent_use_statements_tight() { + let dir = tempdir().unwrap(); + let config_file = dir.path().join("nufmt.nuon"); + let file = dir.path().join("issue126.nu"); + + fs::write( + &config_file, + "{\n indent: 2\n line_length: 80\n margin: 2\n}\n", + ) + .unwrap(); + fs::write(&file, "use a.nu\nuse b.nu\n").unwrap(); + + let output = Command::new(get_test_binary()) + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg(file.to_str().unwrap()) + .output() + .unwrap(); + + assert_eq!(output.status.code(), Some(0)); + let content = fs::read_to_string(&file).unwrap(); + assert_eq!(content, "use a.nu\nuse b.nu\n"); +} + +#[test] +fn issue127_margin_one_preserves_vertical_spacing_groups() { + let dir = tempdir().unwrap(); + let config_file = dir.path().join("nufmt.nuon"); + let file = dir.path().join("issue127.nu"); + + fs::write( + &config_file, + "{\n indent: 2\n line_length: 80\n margin: 1\n}\n", + ) + .unwrap(); + fs::write(&file, "use a.nu\n\ndef foo[] {1}\n\n\ndef boo[] {2}\n").unwrap(); + + let output = Command::new(get_test_binary()) + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg(file.to_str().unwrap()) + .output() + .unwrap(); + + assert_eq!(output.status.code(), Some(0)); + let content = fs::read_to_string(&file).unwrap(); + assert_eq!(content, "use a.nu\n\ndef foo[] {1}\n\ndef boo[] {2}\n"); +} + +#[test] +fn issue145_mixed_line_string_literal_and_pipeline_repair_are_safe() { + let output = + run_stdin("let x = \"((pwd) | where true)\"; let search_path = ((pwd) | where true)\n"); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0)); + assert!( + stdout.contains("let x = \"((pwd) | where true)\""), + "string literal content should be preserved: {stdout}" + ); + assert!( + stdout.contains("let search_path = pwd | where true"), + "pipeline repair should still apply to executable code: {stdout}" + ); +}