From bb39d8f10a3820a1fbda6c50d8df5745ccc11de2 Mon Sep 17 00:00:00 2001 From: Orange <89233794+Ang149@users.noreply.github.com> Date: Mon, 7 Oct 2024 17:59:49 +0800 Subject: [PATCH] Improve hint when provided array for destructuring has fewer elements than expected (#5139) Co-authored-by: Laurenz --- crates/typst/src/eval/binding.rs | 64 +++++++++++++++---------- tests/suite/scripting/destructuring.typ | 16 +++---- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/crates/typst/src/eval/binding.rs b/crates/typst/src/eval/binding.rs index 4201faed4..89e536c09 100644 --- a/crates/typst/src/eval/binding.rs +++ b/crates/typst/src/eval/binding.rs @@ -1,6 +1,8 @@ use std::collections::HashSet; -use crate::diag::{bail, At, SourceResult}; +use ecow::eco_format; + +use crate::diag::{bail, error, At, SourceDiagnostic, SourceResult}; use crate::eval::{Access, Eval, Vm}; use crate::foundations::{Array, Dict, Value}; use crate::syntax::ast::{self, AstNode}; @@ -96,10 +98,7 @@ where match p { ast::DestructuringItem::Pattern(pattern) => { let Ok(v) = value.at(i as i64, None) else { - bail!( - pattern.span(), "not enough elements to destructure"; - hint: "the provided array has a length of {len}", - ); + bail!(wrong_number_of_elements(destruct, len)); }; destructure_impl(vm, pattern, v, f)?; i += 1; @@ -108,10 +107,7 @@ where let sink_size = (1 + len).checked_sub(destruct.items().count()); let sink = sink_size.and_then(|s| value.as_slice().get(i..i + s)); let (Some(sink_size), Some(sink)) = (sink_size, sink) else { - bail!( - spread.span(), "not enough elements to destructure"; - hint: "the provided array has a length of {len}", - ); + bail!(wrong_number_of_elements(destruct, len)); }; if let Some(expr) = spread.sink_expr() { f(vm, expr, Value::Array(sink.into()))?; @@ -125,22 +121,7 @@ where } if i < len { - if i == 0 { - bail!( - destruct.span(), "too many elements to destructure"; - hint: "the provided array has a length of {len}, but the pattern expects an empty array", - ) - } else if i == 1 { - bail!( - destruct.span(), "too many elements to destructure"; - hint: "the provided array has a length of {len}, but the pattern expects a single element", - ); - } else { - bail!( - destruct.span(), "too many elements to destructure"; - hint: "the provided array has a length of {len}, but the pattern expects {i} elements", - ); - } + bail!(wrong_number_of_elements(destruct, len)); } Ok(()) @@ -193,3 +174,36 @@ where Ok(()) } + +/// The error message when the number of elements of the destructuring and the +/// array is mismatched. +#[cold] +fn wrong_number_of_elements( + destruct: ast::Destructuring, + len: usize, +) -> SourceDiagnostic { + let mut count = 0; + let mut spread = false; + + for p in destruct.items() { + match p { + ast::DestructuringItem::Pattern(_) => count += 1, + ast::DestructuringItem::Spread(_) => spread = true, + ast::DestructuringItem::Named(_) => {} + } + } + + let quantifier = if len > count { "too many" } else { "not enough" }; + let expected = match (spread, count) { + (true, c) => eco_format!("at least {c} elements"), + (false, 0) => "an empty array".into(), + (false, 1) => "a single element".into(), + (false, c) => eco_format!("{c} elements",), + }; + + error!( + destruct.span(), "{quantifier} elements to destructure"; + hint: "the provided array has a length of {len}, \ + but the pattern expects {expected}", + ) +} diff --git a/tests/suite/scripting/destructuring.typ b/tests/suite/scripting/destructuring.typ index 9d28c195f..e70f676f5 100644 --- a/tests/suite/scripting/destructuring.typ +++ b/tests/suite/scripting/destructuring.typ @@ -128,13 +128,13 @@ #let () = (1, 2) --- destructuring-let-array-too-few-elements --- -// Error: 13-14 not enough elements to destructure -// Hint: 13-14 the provided array has a length of 2 +// Error: 6-15 not enough elements to destructure +// Hint: 6-15 the provided array has a length of 2, but the pattern expects 3 elements #let (a, b, c) = (1, 2) --- destructuring-let-array-too-few-elements-with-sink --- -// Error: 7-10 not enough elements to destructure -// Hint: 7-10 the provided array has a length of 2 +// Error: 6-20 not enough elements to destructure +// Hint: 6-20 the provided array has a length of 2, but the pattern expects at least 3 elements #let (..a, b, c, d) = (1, 2) --- destructuring-let-array-bool-invalid --- @@ -192,8 +192,8 @@ --- destructuring-let-array-trailing-placeholders --- // Trailing placeholders. -// Error: 10-11 not enough elements to destructure -// Hint: 10-11 the provided array has a length of 1 +// Error: 6-21 not enough elements to destructure +// Hint: 6-21 the provided array has a length of 1, but the pattern expects 5 elements #let (a, _, _, _, _) = (1,) #test(a, 1) @@ -360,8 +360,8 @@ #for (x, y) in (1, 2) {} --- issue-3275-destructuring-loop-over-2d-array-1 --- -// Error: 10-11 not enough elements to destructure -// Hint: 10-11 the provided array has a length of 1 +// Error: 6-12 not enough elements to destructure +// Hint: 6-12 the provided array has a length of 1, but the pattern expects 2 elements #for (x, y) in ((1,), (2,)) {} --- issue-3275-destructuring-loop-over-2d-array-2 ---