Skip to content

Commit 57778f2

Browse files
committed
refactor: use same error types for const and expression record updates
Refactor the record update logic so const updates use the same error types as regular record updates. This produces consistent messages for users and removes cases where similar problems surfaced in different forms. The change: - restores InvalidRecordConstructor instead of InvalidRecordUpdate with reasons - uses UnsafeRecordUpdate for variant mismatches to align with expression handling - adds a constructor_location field so the compiler can point at the constructor when reporting InvalidRecordConstructor - and introduces validation for incompatible field types in spreads
1 parent 111eea2 commit 57778f2

19 files changed

+179
-94
lines changed

compiler-core/src/ast/constant.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub enum Constant<T, RecordTag> {
5050

5151
RecordUpdate {
5252
location: SrcSpan,
53+
constructor_location: SrcSpan,
5354
module: Option<(EcoString, SrcSpan)>,
5455
name: EcoString,
5556
record: Box<Self>,

compiler-core/src/ast/visit.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ pub trait Visit<'ast> {
690690
fn visit_typed_constant_record_update(
691691
&mut self,
692692
location: &'ast SrcSpan,
693+
constructor_location: &'ast SrcSpan,
693694
module: &'ast Option<(EcoString, SrcSpan)>,
694695
name: &'ast EcoString,
695696
record: &'ast TypedConstant,
@@ -699,7 +700,7 @@ pub trait Visit<'ast> {
699700
field_map: &'ast Inferred<FieldMap>,
700701
) {
701702
visit_typed_constant_record_update(
702-
self, location, module, name, record, arguments, tag, type_, field_map,
703+
self, location, constructor_location, module, name, record, arguments, tag, type_, field_map,
703704
)
704705
}
705706

@@ -800,6 +801,7 @@ fn visit_typed_constant_record<'a, V: Visit<'a> + ?Sized>(
800801
fn visit_typed_constant_record_update<'a, V: Visit<'a> + ?Sized>(
801802
v: &mut V,
802803
_location: &'a SrcSpan,
804+
_constructor_location: &'a SrcSpan,
803805
_module: &'a Option<(EcoString, SrcSpan)>,
804806
_name: &'a EcoString,
805807
record: &'a TypedConstant,
@@ -1044,6 +1046,7 @@ pub fn visit_typed_constant<'a, V: Visit<'a> + ?Sized>(v: &mut V, constant: &'a
10441046
),
10451047
super::Constant::RecordUpdate {
10461048
location,
1049+
constructor_location,
10471050
module,
10481051
name,
10491052
record,
@@ -1052,7 +1055,7 @@ pub fn visit_typed_constant<'a, V: Visit<'a> + ?Sized>(v: &mut V, constant: &'a
10521055
type_,
10531056
field_map,
10541057
} => v.visit_typed_constant_record_update(
1055-
location, module, name, record, arguments, tag, type_, field_map,
1058+
location, constructor_location, module, name, record, arguments, tag, type_, field_map,
10561059
),
10571060
super::Constant::BitArray { location, segments } => {
10581061
v.visit_typed_constant_bit_array(location, segments)

compiler-core/src/ast_folder.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -985,14 +985,15 @@ pub trait UntypedConstantFolder {
985985

986986
Constant::RecordUpdate {
987987
location,
988+
constructor_location,
988989
module,
989990
name,
990991
record,
991992
arguments,
992993
tag: (),
993994
type_: (),
994995
field_map: _,
995-
} => self.fold_constant_record_update(location, module, name, record, arguments),
996+
} => self.fold_constant_record_update(location, constructor_location, module, name, record, arguments),
996997

997998
Constant::BitArray { location, segments } => {
998999
self.fold_constant_bit_array(location, segments)
@@ -1096,13 +1097,15 @@ pub trait UntypedConstantFolder {
10961097
fn fold_constant_record_update(
10971098
&mut self,
10981099
location: SrcSpan,
1100+
constructor_location: SrcSpan,
10991101
module: Option<(EcoString, SrcSpan)>,
11001102
name: EcoString,
11011103
record: Box<UntypedConstant>,
11021104
arguments: Vec<ConstantRecordUpdateArg<UntypedConstant>>,
11031105
) -> UntypedConstant {
11041106
Constant::RecordUpdate {
11051107
location,
1108+
constructor_location,
11061109
module,
11071110
name,
11081111
record,
@@ -1218,6 +1221,7 @@ pub trait UntypedConstantFolder {
12181221

12191222
Constant::RecordUpdate {
12201223
location,
1224+
constructor_location,
12211225
module,
12221226
name,
12231227
record,
@@ -1237,6 +1241,7 @@ pub trait UntypedConstantFolder {
12371241
.collect();
12381242
Constant::RecordUpdate {
12391243
location,
1244+
constructor_location,
12401245
module,
12411246
name,
12421247
record,

compiler-core/src/error.rs

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use crate::parse::error::ParseErrorDetails;
99
use crate::strings::{to_snake_case, to_upper_camel_case};
1010
use crate::type_::collapse_links;
1111
use crate::type_::error::{
12-
IncorrectArityContext, InvalidImportKind, InvalidRecordUpdateReason, MissingAnnotation,
13-
ModuleValueUsageContext, Named, UnknownField, UnknownTypeHint, UnsafeRecordUpdateReason,
12+
IncorrectArityContext, InvalidImportKind, MissingAnnotation, ModuleValueUsageContext, Named,
13+
UnknownField, UnknownTypeHint, UnsafeRecordUpdateReason,
1414
};
1515
use crate::type_::printer::{Names, Printer};
1616
use crate::type_::{FieldAccessUsage, error::PatternMatchKind};
@@ -3096,43 +3096,21 @@ UTF-codepoint pattern matching."
30963096
}),
30973097
}
30983098
}
3099-
TypeError::InvalidRecordUpdate { location, reason } => {
3100-
let (title, text, label_text) = match reason {
3101-
InvalidRecordUpdateReason::UnlabelledFields => (
3102-
"Invalid record update".into(),
3103-
"Only constructors with labelled fields can be used with the update syntax.".into(),
3104-
"This constructor has no labelled fields".into(),
3105-
),
3106-
InvalidRecordUpdateReason::WrongVariant { expected, given } => (
3107-
"Type mismatch".into(),
3108-
wrap(&format!(
3109-
"The record being spread is a `{}` but you are trying to construct a `{}`.",
3110-
given, expected
3111-
)),
3112-
format!("This is a `{}`, not a `{}`", given, expected),
3113-
),
3114-
InvalidRecordUpdateReason::NotARecordConstructor => (
3115-
"Invalid record update".into(),
3116-
"Only record constructors can be used with the update syntax.".into(),
3117-
"This is not a record constructor".into(),
3118-
),
3119-
};
3120-
Diagnostic {
3121-
title,
3122-
text,
3123-
hint: None,
3124-
level: Level::Error,
3125-
location: Some(Location {
3126-
label: Label {
3127-
text: Some(label_text),
3128-
span: *location,
3129-
},
3130-
path: path.clone(),
3131-
src: src.clone(),
3132-
extra_labels: vec![],
3133-
}),
3134-
}
3135-
}
3099+
TypeError::InvalidRecordConstructor { location } => Diagnostic {
3100+
title: "Invalid record constructor".into(),
3101+
text: "Only record constructors can be used with the update syntax.".into(),
3102+
hint: None,
3103+
level: Level::Error,
3104+
location: Some(Location {
3105+
label: Label {
3106+
text: Some("This is not a record constructor".into()),
3107+
span: *location,
3108+
},
3109+
path: path.clone(),
3110+
src: src.clone(),
3111+
extra_labels: vec![],
3112+
}),
3113+
},
31363114

31373115
TypeError::UnexpectedTypeHole { location } => Diagnostic {
31383116
title: "Unexpected type hole".into(),

compiler-core/src/parse.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3371,6 +3371,7 @@ where
33713371

33723372
Ok(Some(Constant::RecordUpdate {
33733373
location: SrcSpan { start, end: par_e },
3374+
constructor_location: SrcSpan { start, end },
33743375
module,
33753376
name,
33763377
record,

compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_spread_all_fields.snap

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: compiler-core/src/parse/tests.rs
33
expression: "\ntype Person {\n Person(name: String, age: Int, city: String)\n}\n\nconst base = Person(\"Alice\", 30, \"London\")\nconst updated = Person(..base, name: \"Bob\", age: 25, city: \"Paris\")\n"
4-
snapshot_kind: text
54
---
65
Parsed {
76
module: Module {
@@ -258,6 +257,10 @@ Parsed {
258257
start: 124,
259258
end: 175,
260259
},
260+
constructor_location: SrcSpan {
261+
start: 124,
262+
end: 130,
263+
},
261264
module: None,
262265
name: "Person",
263266
record: Var {

compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_spread_basic.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,10 @@ Parsed {
209209
start: 97,
210210
end: 125,
211211
},
212+
constructor_location: SrcSpan {
213+
start: 97,
214+
end: 103,
215+
},
212216
module: None,
213217
name: "Person",
214218
record: Var {

compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_spread_only.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,10 @@ Parsed {
209209
start: 97,
210210
end: 112,
211211
},
212+
constructor_location: SrcSpan {
213+
start: 97,
214+
end: 103,
215+
},
212216
module: None,
213217
name: "Person",
214218
record: Var {

compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_spread_with_module.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ Parsed {
2828
start: 21,
2929
end: 61,
3030
},
31+
constructor_location: SrcSpan {
32+
start: 21,
33+
end: 33,
34+
},
3135
module: Some(
3236
(
3337
"other",

compiler-core/src/type_/error.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,19 +139,6 @@ impl ModuleSuggestion {
139139
}
140140
}
141141

142-
#[derive(Debug, Eq, PartialEq, Clone)]
143-
pub enum InvalidRecordUpdateReason {
144-
/// Constructor has only unlabelled fields (tuple-like)
145-
UnlabelledFields,
146-
/// Not a record constructor at all (e.g., a function or variable)
147-
NotARecordConstructor,
148-
/// Spreading a different variant (e.g., Dog to create Cat)
149-
WrongVariant {
150-
expected: EcoString,
151-
given: EcoString,
152-
},
153-
}
154-
155142
#[derive(Debug, Eq, PartialEq, Clone)]
156143
pub enum Error {
157144
InvalidImport {
@@ -353,9 +340,8 @@ pub enum Error {
353340
location: SrcSpan,
354341
},
355342

356-
InvalidRecordUpdate {
343+
InvalidRecordConstructor {
357344
location: SrcSpan,
358-
reason: InvalidRecordUpdateReason,
359345
},
360346

361347
UnexpectedTypeHole {
@@ -1295,7 +1281,7 @@ impl Error {
12951281
| Error::NotATuple { location, .. }
12961282
| Error::NotATupleUnbound { location, .. }
12971283
| Error::RecordAccessUnknownType { location, .. }
1298-
| Error::InvalidRecordUpdate { location, .. }
1284+
| Error::InvalidRecordConstructor { location, .. }
12991285
| Error::UnexpectedTypeHole { location, .. }
13001286
| Error::NotExhaustivePatternMatch { location, .. }
13011287
| Error::ArgumentNameAlreadyUsed { location, .. }

0 commit comments

Comments
 (0)