Fork of daniellemaywood.uk/gleam — Wasm codegen work
2

Configure Feed

Select the types of activity you want to include in your feed.

add remove redundant record update code action

author
Giacomo Cavalieri
committer
Louis Pilfold
date (May 12, 2026, 5:25 PM +0100) commit 056e1369 parent ffd16ab0 change-id zpwonlxv
+319 -18
+2 -2
compiler-core/src/parse.rs
··· 3523 3523 ) -> Result<Option<UntypedConstant>, ParseError> { 3524 3524 match self.maybe_one(&Token::LeftParen) { 3525 3525 Some((par_s, _)) => { 3526 - if self.maybe_one(&Token::DotDot).is_some() { 3526 + if let Some((dot_dot_start, _)) = self.maybe_one(&Token::DotDot) { 3527 3527 let record = match self.parse_const_value()? { 3528 3528 Some(value) => RecordBeingUpdated { 3529 - location: value.location(), 3529 + location: SrcSpan::new(dot_dot_start, value.location().end), 3530 3530 base: Box::new(value), 3531 3531 }, 3532 3532 None => {
+1 -1
compiler-core/src/parse/snapshots/gleam_core__parse__tests__byte_order_mark_module.snap
··· 62 62 type_: (), 63 63 }, 64 64 location: SrcSpan { 65 - start: 39, 65 + start: 37, 66 66 end: 49, 67 67 }, 68 68 },
+1 -1
compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_update_all_fields.snap
··· 278 278 type_: (), 279 279 }, 280 280 location: SrcSpan { 281 - start: 133, 281 + start: 131, 282 282 end: 137, 283 283 }, 284 284 },
+1 -1
compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_update_basic.snap
··· 229 229 type_: (), 230 230 }, 231 231 location: SrcSpan { 232 - start: 106, 232 + start: 104, 233 233 end: 111, 234 234 }, 235 235 },
+1 -1
compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_update_only.snap
··· 229 229 type_: (), 230 230 }, 231 231 location: SrcSpan { 232 - start: 106, 232 + start: 104, 233 233 end: 111, 234 234 }, 235 235 },
+1 -1
compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_record_update_with_module.snap
··· 62 62 type_: (), 63 63 }, 64 64 location: SrcSpan { 65 - start: 36, 65 + start: 34, 66 66 end: 46, 67 67 }, 68 68 },
+13
compiler-core/src/type_/error.rs
··· 886 886 }, 887 887 888 888 AllFieldsRecordUpdate { 889 + /// The location of the entire redundant record expression. 889 890 location: SrcSpan, 891 + /// The location covering the record spread, including any comma that 892 + /// might come after it: 893 + /// 894 + /// ```gleam 895 + /// Wibble(..wibble, a: 1) 896 + /// // ^^^^^^^^^ This! 897 + /// ``` 898 + /// 899 + /// You might wonder why include the comma location as well, that's 900 + /// what's most handy for the language server to suggest automatic 901 + /// fixes to remove the record! 902 + record_location: SrcSpan, 890 903 }, 891 904 892 905 UnusedType {
+31 -9
compiler-core/src/type_/expression.rs
··· 3050 3050 .clone(); 3051 3051 3052 3052 // infer the record being updated 3053 + let spread_location = record.location; 3053 3054 let record = self.infer(*record.base); 3054 3055 let record_location = record.location(); 3055 3056 let record_type = record.type_(); ··· 3085 3086 let variant = 3086 3087 self.infer_record_update_variant(&typed_constructor, &value_constructor, &record_var)?; 3087 3088 3088 - let arguments = 3089 - self.infer_record_update_arguments(&variant, &record_var, arguments, location)?; 3089 + let arguments = self.infer_record_update_arguments( 3090 + &variant, 3091 + &record_var, 3092 + arguments, 3093 + location, 3094 + spread_location, 3095 + )?; 3090 3096 3091 3097 Ok(TypedExpr::RecordUpdate { 3092 3098 location, ··· 3103 3109 record: &TypedExpr, 3104 3110 arguments: Vec<UntypedRecordUpdateArg>, 3105 3111 location: SrcSpan, 3112 + spread_location: SrcSpan, 3106 3113 ) -> Result<Vec<TypedCallArg>, Error> { 3107 3114 let record_location = record.location(); 3108 3115 let record_type = record.type_(); ··· 3318 3325 } 3319 3326 3320 3327 if implicit_arguments.is_empty() { 3321 - self.problems 3322 - .warning(Warning::AllFieldsRecordUpdate { location }); 3328 + self.problems.warning(Warning::AllFieldsRecordUpdate { 3329 + location, 3330 + record_location: SrcSpan::new( 3331 + spread_location.start, 3332 + arguments 3333 + .first() 3334 + .map_or(spread_location.end, |argument| argument.location.start), 3335 + ), 3336 + }); 3323 3337 } 3324 3338 3325 3339 let arguments = explicit_arguments ··· 3786 3800 .. 3787 3801 } => { 3788 3802 self.track_feature_usage(FeatureKind::ConstantRecordUpdate, location); 3803 + let first_argument_start = 3804 + arguments.first().map(|argument| argument.location.start); 3805 + 3789 3806 let constructor = match self.infer_value_constructor(&module, &name, &location) { 3790 3807 Ok(constructor) => constructor, 3791 3808 Err(error) => { ··· 3878 3895 given: typed_record_type, 3879 3896 situation: None, 3880 3897 }, 3881 - record.location, 3898 + record.base.location(), 3882 3899 )); 3883 3900 return self.new_invalid_constant(location); 3884 3901 }; ··· 3887 3904 // For multi-variant custom types, you can't spread Dog to create Cat 3888 3905 if tag != base_tag { 3889 3906 self.problems.error(Error::UnsafeRecordUpdate { 3890 - location: record.location, 3907 + location: record.base.location(), 3891 3908 reason: UnsafeRecordUpdateReason::WrongVariant { 3892 3909 constructed_variant: tag, 3893 3910 spread_variant: base_tag, ··· 3959 3976 3960 3977 // Emit warning if all fields are being overridden 3961 3978 if implicit_labelled_arguments.is_empty() { 3962 - self.problems 3963 - .warning(Warning::AllFieldsRecordUpdate { location }); 3979 + self.problems.warning(Warning::AllFieldsRecordUpdate { 3980 + location, 3981 + record_location: SrcSpan::new( 3982 + record.location.start, 3983 + first_argument_start.unwrap_or(record.location.end), 3984 + ), 3985 + }); 3964 3986 } 3965 3987 3966 3988 // Check that fields implicitly overridden (including unlabelled ones) have compatible types. ··· 3987 4009 } = unify_error 3988 4010 { 3989 4011 Error::UnsafeRecordUpdate { 3990 - location: record.location, 4012 + location: record.base.location(), 3991 4013 reason: UnsafeRecordUpdateReason::IncompatibleFieldTypes { 3992 4014 constructed_variant: expected_type.clone(), 3993 4015 record_variant: typed_record_type.clone(),
+1 -1
compiler-core/src/warning.rs
··· 541 541 }), 542 542 }, 543 543 544 - type_::Warning::AllFieldsRecordUpdate { location } => Diagnostic { 544 + type_::Warning::AllFieldsRecordUpdate { location, .. } => Diagnostic { 545 545 title: "Redundant record update".into(), 546 546 text: "".into(), 547 547 hint: Some("It is better style to use the record creation syntax.".into()),
+67
language-server/src/code_action.rs
··· 9723 9723 } 9724 9724 } 9725 9725 9726 + /// Code action to remove a record update when all of its fields have been 9727 + /// provided already: 9728 + /// 9729 + /// ```gleam 9730 + /// pub type Wibble { Wibble(one: Int, two: Int) } 9731 + /// 9732 + /// wibble(..wibble, one:, two:) 9733 + /// // ^^^^^^^^ This is not needed and raises a warning! 9734 + /// ``` 9735 + /// 9736 + pub struct RemoveRedundantRecordUpdate<'a> { 9737 + module: &'a Module, 9738 + params: &'a CodeActionParams, 9739 + edits: TextEdits<'a>, 9740 + } 9741 + 9742 + impl<'a> RemoveRedundantRecordUpdate<'a> { 9743 + pub fn new( 9744 + module: &'a Module, 9745 + line_numbers: &'a LineNumbers, 9746 + params: &'a CodeActionParams, 9747 + ) -> Self { 9748 + Self { 9749 + module, 9750 + params, 9751 + edits: TextEdits::new(line_numbers), 9752 + } 9753 + } 9754 + 9755 + pub fn code_actions(mut self) -> Vec<CodeAction> { 9756 + let spread_to_remove = self 9757 + .module 9758 + .ast 9759 + .type_info 9760 + .warnings 9761 + .iter() 9762 + .find_map(|warning| { 9763 + if let type_::Warning::AllFieldsRecordUpdate { 9764 + location, 9765 + record_location, 9766 + } = warning 9767 + && within( 9768 + self.params.range, 9769 + self.edits.src_span_to_lsp_range(*location), 9770 + ) 9771 + { 9772 + Some(*record_location) 9773 + } else { 9774 + None 9775 + } 9776 + }); 9777 + 9778 + let Some(spread_to_remove) = spread_to_remove else { 9779 + return vec![]; 9780 + }; 9781 + self.edits.delete(spread_to_remove); 9782 + 9783 + let mut action = Vec::with_capacity(1); 9784 + CodeActionBuilder::new("Remove redundant record update") 9785 + .kind(CodeActionKind::QUICKFIX) 9786 + .changes(self.params.text_document.uri.clone(), self.edits.edits) 9787 + .preferred(true) 9788 + .push_to(&mut action); 9789 + action 9790 + } 9791 + } 9792 + 9726 9793 /// Code action to add labels to a constructor/call where all the labels where 9727 9794 /// omitted. 9728 9795 ///
+3 -1
language-server/src/engine.rs
··· 33 33 use std::{collections::HashSet, sync::Arc}; 34 34 35 35 use crate::{ 36 - code_action::{ReplaceUnderscoreWithType, type_errors_for_module}, 36 + code_action::{RemoveRedundantRecordUpdate, ReplaceUnderscoreWithType, type_errors_for_module}, 37 37 rename::rename_module_alias, 38 38 }; 39 39 ··· 450 450 code_action_add_missing_patterns(module, &lines, &params, &this.error, &mut actions); 451 451 actions 452 452 .extend(RemoveUnreachableCaseClauses::new(module, &lines, &params).code_actions()); 453 + actions 454 + .extend(RemoveRedundantRecordUpdate::new(module, &lines, &params).code_actions()); 453 455 actions.extend(CollapseNestedCase::new(module, &lines, &params).code_actions()); 454 456 code_action_inexhaustive_let_to_case( 455 457 module,
+100
language-server/src/tests/action.rs
··· 210 210 const REPLACE_UNDERSCORE_WITH_TYPE: &str = "Replace `_` with type"; 211 211 const WRAP_IN_ANONYMOUS_FUNCTION: &str = "Wrap in anonymous function"; 212 212 const UNWRAP_ANONYMOUS_FUNCTION: &str = "Remove anonymous function wrapper"; 213 + const REMOVE_REDUNDANT_RECORD_UPDATE: &str = "Remove redundant record update"; 213 214 214 215 macro_rules! assert_code_action { 215 216 ($title:expr, $code:literal, $range_selector:expr $(,)?) => { ··· 14238 14239 find_position_of("main").to_selection() 14239 14240 ); 14240 14241 } 14242 + 14243 + #[test] 14244 + fn remove_redundant_record_update_triggered_on_the_record_spread() { 14245 + assert_code_action!( 14246 + REMOVE_REDUNDANT_RECORD_UPDATE, 14247 + " 14248 + pub fn go(record: Wibble) { 14249 + Wibble(..record, a: 1, b: 2) 14250 + } 14251 + 14252 + pub type Wibble { Wibble(a: Int, b: Int) } 14253 + ", 14254 + find_position_of("..").to_selection() 14255 + ); 14256 + } 14257 + 14258 + #[test] 14259 + fn remove_redundant_record_update_triggered_on_the_record() { 14260 + assert_code_action!( 14261 + REMOVE_REDUNDANT_RECORD_UPDATE, 14262 + " 14263 + pub fn go(record: Wibble) { 14264 + Wibble(..record, a: 1, b: 2) 14265 + } 14266 + 14267 + pub type Wibble { Wibble(a: Int, b: Int) } 14268 + ", 14269 + find_position_of("record").nth_occurrence(2).to_selection() 14270 + ); 14271 + } 14272 + 14273 + #[test] 14274 + fn remove_redundant_record_update_triggered_anywhere_on_the_expression() { 14275 + assert_code_action!( 14276 + REMOVE_REDUNDANT_RECORD_UPDATE, 14277 + " 14278 + pub fn go(record: Wibble) { 14279 + Wibble(..record, a: 1, b: 2) 14280 + } 14281 + 14282 + pub type Wibble { Wibble(a: Int, b: Int) } 14283 + ", 14284 + find_position_of("1").select_until(find_position_of("2")) 14285 + ); 14286 + } 14287 + 14288 + #[test] 14289 + fn remove_redundant_record_update_does_not_trigger_if_update_is_not_redundant() { 14290 + assert_no_code_actions!( 14291 + REMOVE_REDUNDANT_RECORD_UPDATE, 14292 + " 14293 + pub fn go(record: Wibble) { 14294 + Wibble(..record, a: 1) 14295 + } 14296 + 14297 + pub type Wibble { Wibble(a: Int, b: Int) } 14298 + ", 14299 + find_position_of("..record").to_selection() 14300 + ); 14301 + } 14302 + 14303 + #[test] 14304 + fn remove_redundant_constant_record_update_triggered_on_the_record_spread() { 14305 + assert_code_action!( 14306 + REMOVE_REDUNDANT_RECORD_UPDATE, 14307 + " 14308 + pub const updated = Wibble(..base, a: 1, b: 3) 14309 + pub const base = Wibble(a: 1, b: 2) 14310 + pub type Wibble { Wibble(a: Int, b: Int) } 14311 + ", 14312 + find_position_of("..").to_selection() 14313 + ); 14314 + } 14315 + 14316 + #[test] 14317 + fn remove_redundant_constant_record_update_triggered_on_the_record() { 14318 + assert_code_action!( 14319 + REMOVE_REDUNDANT_RECORD_UPDATE, 14320 + " 14321 + pub const updated = Wibble(..base, a: 1, b: 3) 14322 + pub const base = Wibble(a: 1, b: 2) 14323 + pub type Wibble { Wibble(a: Int, b: Int) } 14324 + ", 14325 + find_position_of("1").select_until(find_position_of("3")) 14326 + ); 14327 + } 14328 + 14329 + #[test] 14330 + fn remove_redundant_constant_record_update_does_not_trigger_if_update_i_not_redundant() { 14331 + assert_no_code_actions!( 14332 + REMOVE_REDUNDANT_RECORD_UPDATE, 14333 + " 14334 + pub const updated = Wibble(..base, a: 1) 14335 + pub const base = Wibble(a: 1, b: 2) 14336 + pub type Wibble { Wibble(a: Int, b: Int) } 14337 + ", 14338 + find_position_of("base").to_selection() 14339 + ); 14340 + }
+17
language-server/src/tests/snapshots/gleam_language_server__tests__action__remove_redundant_constant_record_update_triggered_on_the_record.snap
··· 1 + --- 2 + source: language-server/src/tests/action.rs 3 + expression: "\npub const updated = Wibble(..base, a: 1, b: 3)\npub const base = Wibble(a: 1, b: 2)\npub type Wibble { Wibble(a: Int, b: Int) }\n" 4 + --- 5 + ----- BEFORE ACTION 6 + 7 + pub const updated = Wibble(..base, a: 1, b: 3) 8 + ▔▔▔▔▔▔↑ 9 + pub const base = Wibble(a: 1, b: 2) 10 + pub type Wibble { Wibble(a: Int, b: Int) } 11 + 12 + 13 + ----- AFTER ACTION 14 + 15 + pub const updated = Wibble(a: 1, b: 3) 16 + pub const base = Wibble(a: 1, b: 2) 17 + pub type Wibble { Wibble(a: Int, b: Int) }
+17
language-server/src/tests/snapshots/gleam_language_server__tests__action__remove_redundant_constant_record_update_triggered_on_the_record_spread.snap
··· 1 + --- 2 + source: language-server/src/tests/action.rs 3 + expression: "\npub const updated = Wibble(..base, a: 1, b: 3)\npub const base = Wibble(a: 1, b: 2)\npub type Wibble { Wibble(a: Int, b: Int) }\n" 4 + --- 5 + ----- BEFORE ACTION 6 + 7 + pub const updated = Wibble(..base, a: 1, b: 3) 8 + 9 + pub const base = Wibble(a: 1, b: 2) 10 + pub type Wibble { Wibble(a: Int, b: Int) } 11 + 12 + 13 + ----- AFTER ACTION 14 + 15 + pub const updated = Wibble(a: 1, b: 3) 16 + pub const base = Wibble(a: 1, b: 2) 17 + pub type Wibble { Wibble(a: Int, b: Int) }
+21
language-server/src/tests/snapshots/gleam_language_server__tests__action__remove_redundant_record_update_triggered_anywhere_on_the_expression.snap
··· 1 + --- 2 + source: language-server/src/tests/action.rs 3 + expression: "\npub fn go(record: Wibble) {\n Wibble(..record, a: 1, b: 2)\n}\n\npub type Wibble { Wibble(a: Int, b: Int) }\n" 4 + --- 5 + ----- BEFORE ACTION 6 + 7 + pub fn go(record: Wibble) { 8 + Wibble(..record, a: 1, b: 2) 9 + ▔▔▔▔▔▔↑ 10 + } 11 + 12 + pub type Wibble { Wibble(a: Int, b: Int) } 13 + 14 + 15 + ----- AFTER ACTION 16 + 17 + pub fn go(record: Wibble) { 18 + Wibble(a: 1, b: 2) 19 + } 20 + 21 + pub type Wibble { Wibble(a: Int, b: Int) }
+21
language-server/src/tests/snapshots/gleam_language_server__tests__action__remove_redundant_record_update_triggered_on_the_record.snap
··· 1 + --- 2 + source: language-server/src/tests/action.rs 3 + expression: "\npub fn go(record: Wibble) {\n Wibble(..record, a: 1, b: 2)\n}\n\npub type Wibble { Wibble(a: Int, b: Int) }\n" 4 + --- 5 + ----- BEFORE ACTION 6 + 7 + pub fn go(record: Wibble) { 8 + Wibble(..record, a: 1, b: 2) 9 + 10 + } 11 + 12 + pub type Wibble { Wibble(a: Int, b: Int) } 13 + 14 + 15 + ----- AFTER ACTION 16 + 17 + pub fn go(record: Wibble) { 18 + Wibble(a: 1, b: 2) 19 + } 20 + 21 + pub type Wibble { Wibble(a: Int, b: Int) }
+21
language-server/src/tests/snapshots/gleam_language_server__tests__action__remove_redundant_record_update_triggered_on_the_record_spread.snap
··· 1 + --- 2 + source: language-server/src/tests/action.rs 3 + expression: "\npub fn go(record: Wibble) {\n Wibble(..record, a: 1, b: 2)\n}\n\npub type Wibble { Wibble(a: Int, b: Int) }\n" 4 + --- 5 + ----- BEFORE ACTION 6 + 7 + pub fn go(record: Wibble) { 8 + Wibble(..record, a: 1, b: 2) 9 + 10 + } 11 + 12 + pub type Wibble { Wibble(a: Int, b: Int) } 13 + 14 + 15 + ----- AFTER ACTION 16 + 17 + pub fn go(record: Wibble) { 18 + Wibble(a: 1, b: 2) 19 + } 20 + 21 + pub type Wibble { Wibble(a: Int, b: Int) }