From a6abecd25fe7fc2ad5882cb6e3d6abc2a7119788 Mon Sep 17 00:00:00 2001 From: fruno Date: Tue, 11 Nov 2025 19:45:39 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20Recognize=20missing=20patter?= =?UTF-8?q?n=20in=20alternative=20as=20syntax=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- compiler-core/src/parse.rs | 15 +++++++--- ...s__case_alternative_clause_no_subject.snap | 20 +++++++++++++ ..._parse__tests__case_clause_no_subject.snap | 24 ++++++++++++++++ compiler-core/src/parse/tests.rs | 28 +++++++++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_alternative_clause_no_subject.snap create mode 100644 compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_clause_no_subject.snap diff --git a/compiler-core/src/parse.rs b/compiler-core/src/parse.rs index 016b62b8a24..f751c4aeb11 100644 --- a/compiler-core/src/parse.rs +++ b/compiler-core/src/parse.rs @@ -1599,11 +1599,18 @@ where match &patterns.first() { Some(lead) => { let mut alternative_patterns = vec![]; - loop { - if self.maybe_one(&Token::Vbar).is_none() { - break; + while let Some((vbar_start, vbar_end)) = self.maybe_one(&Token::Vbar) { + let patterns = self.parse_patterns(PatternPosition::CaseClause)?; + if patterns.is_empty() { + return parse_error( + ParseErrorType::ExpectedPattern, + SrcSpan { + start: vbar_start, + end: vbar_end, + }, + ); } - alternative_patterns.push(self.parse_patterns(PatternPosition::CaseClause)?); + alternative_patterns.push(patterns); } let guard = self.parse_case_clause_guard()?; let (arr_s, arr_e) = self diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_alternative_clause_no_subject.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_alternative_clause_no_subject.snap new file mode 100644 index 00000000000..aa829aceb40 --- /dev/null +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_alternative_clause_no_subject.snap @@ -0,0 +1,20 @@ +--- +source: compiler-core/src/parse/tests.rs +expression: "\nfn main() {\n case 1 {\n 1 | -> 1\n _ -> 1\n }\n}\n" +--- +----- SOURCE CODE + +fn main() { + case 1 { + 1 | -> 1 + _ -> 1 + } +} + + +----- ERROR +error: Syntax error + ┌─ /src/parse/error.gleam:4:9 + │ +4 │ 1 | -> 1 + │ ^ I was expecting a pattern after this diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_clause_no_subject.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_clause_no_subject.snap new file mode 100644 index 00000000000..9786ebe9e68 --- /dev/null +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__case_clause_no_subject.snap @@ -0,0 +1,24 @@ +--- +source: compiler-core/src/parse/tests.rs +expression: "\nfn main() {\n case 1 {\n -> 1\n _ -> 2\n }\n}\n" +--- +----- SOURCE CODE + +fn main() { + case 1 { + -> 1 + _ -> 2 + } +} + + +----- ERROR +error: Syntax error + ┌─ /src/parse/error.gleam:4:7 + │ +4 │ -> 1 + │ ^^ I was not expecting this + +Found `->`, expected one of: +- `}` +- a case clause diff --git a/compiler-core/src/parse/tests.rs b/compiler-core/src/parse/tests.rs index 2e87f13b16d..c59151feb1a 100644 --- a/compiler-core/src/parse/tests.rs +++ b/compiler-core/src/parse/tests.rs @@ -1144,6 +1144,34 @@ fn main() { ); } +#[test] +fn case_clause_no_subject() { + assert_module_error!( + " +fn main() { + case 1 { + -> 1 + _ -> 2 + } +} +" + ); +} + +#[test] +fn case_alternative_clause_no_subject() { + assert_module_error!( + " +fn main() { + case 1 { + 1 | -> 1 + _ -> 1 + } +} +" + ); +} + #[test] fn use_invalid_assignments() { assert_module_error!( From d69f5e157a38feaab664c3a8c813d97304f333d4 Mon Sep 17 00:00:00 2001 From: fruno Date: Tue, 11 Nov 2025 17:44:29 +0100 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9C=A8=20Improve=20error=20for=20wrong?= =?UTF-8?q?=20number=20of=20subjects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- compiler-core/src/type_/expression.rs | 2 +- compiler-core/src/type_/pattern.rs | 12 ++++++--- compiler-core/src/type_/tests/errors.rs | 5 ++++ ...sts__errors__wrong_number_of_subjects.snap | 2 +- ...mber_of_subjects_alternative_patterns.snap | 25 +++++++++++++++++++ 5 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects_alternative_patterns.snap diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index 1398ac9051f..444182380b7 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -2339,7 +2339,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { PatternPosition::CaseClause, ); - let typed_pattern = pattern_typer.infer_multi_pattern(pattern, subjects, location); + let typed_pattern = pattern_typer.infer_multi_pattern(pattern, subjects); // Each case clause has one or more patterns that may match the // subject in order for the clause to be selected, so we must type diff --git a/compiler-core/src/type_/pattern.rs b/compiler-core/src/type_/pattern.rs index 061c51150db..76c5ff85dbc 100644 --- a/compiler-core/src/type_/pattern.rs +++ b/compiler-core/src/type_/pattern.rs @@ -266,7 +266,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> { location: &SrcSpan, ) -> Vec { self.mode = PatternMode::Alternative(vec![]); - let typed_multi = self.infer_multi_pattern(multi_pattern, subjects, location); + let typed_multi = self.infer_multi_pattern(multi_pattern, subjects); if self.error_encountered { return typed_multi; @@ -300,12 +300,18 @@ impl<'a, 'b> PatternTyper<'a, 'b> { &mut self, multi_pattern: UntypedMultiPattern, subjects: &[TypedExpr], - location: &SrcSpan, ) -> Vec { // If there are N subjects the multi-pattern is expected to be N patterns if subjects.len() != multi_pattern.len() { + let first = multi_pattern + .first() + .expect("multi-pattern to contain at least one pattern"); + let last = multi_pattern + .last() + .expect("multi-pattern to contain at least one pattern"); + self.error(Error::IncorrectNumClausePatterns { - location: *location, + location: first.location().merge(&last.location()), expected: subjects.len(), given: multi_pattern.len(), }); diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs index 14d5e515708..7001ae76456 100644 --- a/compiler-core/src/type_/tests/errors.rs +++ b/compiler-core/src/type_/tests/errors.rs @@ -385,6 +385,11 @@ fn wrong_number_of_subjects() { assert_error!("case 1 { _, _ -> 1 }"); } +#[test] +fn wrong_number_of_subjects_alternative_patterns() { + assert_error!("case 1 { _, _ | _ | _, _, _ -> 1 }"); +} + #[test] fn recursive_var() { assert_error!("let id = fn(x) { x(x) } 1"); diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects.snap index e931739e16e..cac73f9c8b8 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects.snap @@ -10,7 +10,7 @@ error: Incorrect number of patterns ┌─ /src/one/two.gleam:1:10 │ 1 │ case 1 { _, _ -> 1 } - │ ^^^^^^^^^ Expected 1 patterns, got 2 + │ ^^^^ Expected 1 patterns, got 2 This case expression has 1 subjects, but this pattern matches 2. Each clause must have a pattern for every subject value. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects_alternative_patterns.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects_alternative_patterns.snap new file mode 100644 index 00000000000..70c92e30f02 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__wrong_number_of_subjects_alternative_patterns.snap @@ -0,0 +1,25 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "case 1 { _, _ | _ | _, _, _ -> 1 }" +--- +----- SOURCE CODE +case 1 { _, _ | _ | _, _, _ -> 1 } + +----- ERROR +error: Incorrect number of patterns + ┌─ /src/one/two.gleam:1:10 + │ +1 │ case 1 { _, _ | _ | _, _, _ -> 1 } + │ ^^^^ Expected 1 patterns, got 2 + +This case expression has 1 subjects, but this pattern matches 2. +Each clause must have a pattern for every subject value. + +error: Incorrect number of patterns + ┌─ /src/one/two.gleam:1:21 + │ +1 │ case 1 { _, _ | _ | _, _, _ -> 1 } + │ ^^^^^^^ Expected 1 patterns, got 3 + +This case expression has 1 subjects, but this pattern matches 3. +Each clause must have a pattern for every subject value. From ba810f16456137e225e5809fefa04c7a6f42a334 Mon Sep 17 00:00:00 2001 From: fruno Date: Tue, 11 Nov 2025 22:19:01 +0100 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=93=9D=20Update=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f441ed5d1de..c8b3f941b15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,18 @@ ([Surya Rose](https://github.com/GearsDatapacks)) +- When matching the wrong number of subjects, the compiler now pinpoints the + error location instead of marking the entire branch. + ``` + case wibble { + 0, _ -> 1 + ^^^^ Expected 1 patterns, got 2 + 0 | -> 1 + ^ I was expecting a pattern after this + } + ``` + ([fruno](https://github.com/fruno-bulax/)) + - The performance of `==` and `!=` has been improved for single-variant custom types when compiling to JavaScript. This was done by generating comparison code specific to the custom type rather than using the generic equality check