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

Configure Feed

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

Fix renaming of alternative variables

author
GearsDatapacks committer
Louis Pilfold date (Jul 6, 2025, 2:01 PM +0100) commit 357ab020 parent bbfaaedf
+268 -17
+4
CHANGELOG.md
··· 419 419 internal modules from other packages. 420 420 ([Surya Rose](https://github.com/GearsDatapacks)) 421 421 422 + - Fixed a bug where the language server would not rename variables defined in 423 + alternative patterns. 424 + ([Surya Rose](https://github.com/GearsDatapacks)) 425 + 422 426 ## v1.11.1 - 2025-06-05 423 427 424 428 ### Compiler
+14
compiler-core/src/ast.rs
··· 1598 1598 .find_map(|p| p.find_node(byte_index)) 1599 1599 .or_else(|| self.then.find_node(byte_index)) 1600 1600 } 1601 + 1602 + pub fn pattern_location(&self) -> SrcSpan { 1603 + let start = self.pattern.first().map(|pattern| pattern.location().start); 1604 + 1605 + let end = if let Some(last_alternative) = self.alternative_patterns.last() 1606 + && let Some(last_pattern) = last_alternative.last() 1607 + { 1608 + Some(last_pattern.location().end) 1609 + } else { 1610 + self.pattern.last().map(|pattern| pattern.location().end) 1611 + }; 1612 + 1613 + SrcSpan::new(start.unwrap_or_default(), end.unwrap_or_default()) 1614 + } 1601 1615 } 1602 1616 1603 1617 pub type UntypedClauseGuard = ClauseGuard<(), ()>;
+7 -6
compiler-core/src/language_server/code_action.rs
··· 5920 5920 self.actions 5921 5921 } 5922 5922 5923 - fn maybe_inline(&mut self, location: SrcSpan) { 5924 - let reference = match find_variable_references(&self.module.ast, location).as_slice() { 5923 + fn maybe_inline(&mut self, location: SrcSpan, name: EcoString) { 5924 + let reference = match find_variable_references(&self.module.ast, location, name).as_slice() 5925 + { 5925 5926 [only_reference] => *only_reference, 5926 5927 _ => return, 5927 5928 }; ··· 5992 5993 &mut self, 5993 5994 location: &'ast SrcSpan, 5994 5995 constructor: &'ast ValueConstructor, 5995 - _name: &'ast EcoString, 5996 + name: &'ast EcoString, 5996 5997 ) { 5997 5998 let range = self.edits.src_span_to_lsp_range(*location); 5998 5999 ··· 6016 6017 | VariableDeclaration::Generated => return, 6017 6018 } 6018 6019 6019 - self.maybe_inline(*location); 6020 + self.maybe_inline(*location, name.clone()); 6020 6021 } 6021 6022 6022 6023 fn visit_typed_pattern_variable( 6023 6024 &mut self, 6024 6025 location: &'ast SrcSpan, 6025 - _name: &'ast EcoString, 6026 + name: &'ast EcoString, 6026 6027 _type: &'ast Arc<Type>, 6027 6028 origin: &'ast VariableOrigin, 6028 6029 ) { ··· 6042 6043 return; 6043 6044 } 6044 6045 6045 - self.maybe_inline(*location); 6046 + self.maybe_inline(*location, name.clone()); 6046 6047 } 6047 6048 } 6048 6049
+11 -2
compiler-core/src/language_server/engine.rs
··· 693 693 Some(Referenced::LocalVariable { 694 694 origin, 695 695 definition_location, 696 + name, 696 697 .. 697 698 }) => { 698 699 let rename_kind = match origin.map(|origin| origin.syntax) { ··· 705 706 ) 706 707 | None => VariableReferenceKind::Variable, 707 708 }; 708 - rename_local_variable(module, &lines, &params, definition_location, rename_kind) 709 + rename_local_variable( 710 + module, 711 + &lines, 712 + &params, 713 + definition_location, 714 + name, 715 + rename_kind, 716 + ) 709 717 } 710 718 Some(Referenced::ModuleValue { 711 719 module: module_name, ··· 774 782 origin, 775 783 definition_location, 776 784 location, 785 + name, 777 786 }) if location.contains(byte_index) => match origin.map(|origin| origin.syntax) { 778 787 Some(VariableSyntax::Generated) => None, 779 788 Some( ··· 783 792 ) 784 793 | None => { 785 794 let variable_references = 786 - find_variable_references(&module.ast, definition_location); 795 + find_variable_references(&module.ast, definition_location, name); 787 796 788 797 let mut reference_locations = 789 798 Vec::with_capacity(variable_references.len() + 1);
+85 -8
compiler-core/src/language_server/reference.rs
··· 25 25 definition_location: SrcSpan, 26 26 location: SrcSpan, 27 27 origin: Option<VariableOrigin>, 28 + name: EcoString, 28 29 }, 29 30 ModuleValue { 30 31 module: EcoString, ··· 59 60 .. 60 61 }, 61 62 location, 62 - .. 63 + name, 63 64 }, 64 65 .. 65 66 } => Some(Referenced::LocalVariable { 66 67 definition_location: *definition_location, 67 68 location: *location, 68 69 origin: Some(origin.clone()), 70 + name: name.clone(), 69 71 }), 70 72 Located::Pattern(Pattern::Variable { 71 - location, origin, .. 73 + location, 74 + origin, 75 + name, 76 + .. 72 77 }) => Some(Referenced::LocalVariable { 73 78 definition_location: *location, 74 79 location: *location, 75 80 origin: Some(origin.clone()), 81 + name: name.clone(), 76 82 }), 77 83 Located::Pattern(Pattern::VarUsage { 78 84 constructor, 79 85 location, 86 + name, 80 87 .. 81 88 }) => constructor 82 89 .as_ref() ··· 88 95 definition_location: *definition_location, 89 96 location: *location, 90 97 origin: Some(origin.clone()), 98 + name: name.clone(), 91 99 }), 92 100 _ => None, 93 101 }), 94 - Located::Pattern(Pattern::Assign { location, .. }) => Some(Referenced::LocalVariable { 95 - definition_location: *location, 96 - location: *location, 97 - origin: None, 98 - }), 102 + Located::Pattern(Pattern::Assign { location, name, .. }) => { 103 + Some(Referenced::LocalVariable { 104 + definition_location: *location, 105 + location: *location, 106 + origin: None, 107 + name: name.clone(), 108 + }) 109 + } 99 110 Located::Arg(arg) => match &arg.names { 100 - ArgNames::Named { location, .. } 111 + ArgNames::Named { location, name } 101 112 | ArgNames::NamedLabelled { 102 113 name_location: location, 114 + name, 103 115 .. 104 116 } => Some(Referenced::LocalVariable { 105 117 definition_location: *location, 106 118 location: *location, 107 119 origin: None, 120 + name: name.clone(), 108 121 }), 109 122 ArgNames::Discard { .. } | ArgNames::LabelledDiscard { .. } => None, 110 123 }, ··· 344 357 pub fn find_variable_references( 345 358 module: &TypedModule, 346 359 definition_location: SrcSpan, 360 + name: EcoString, 347 361 ) -> Vec<VariableReference> { 348 362 let mut finder = FindVariableReferences { 349 363 references: Vec::new(), 350 364 definition_location, 365 + alternative_variable: AlternativeVariable::Ignore, 366 + name, 351 367 }; 352 368 finder.visit_typed_module(module); 353 369 finder.references 354 370 } 355 371 372 + /// How to treat variables defined in alternative patterns 373 + enum AlternativeVariable { 374 + Track, 375 + Ignore, 376 + } 377 + 356 378 struct FindVariableReferences { 357 379 references: Vec<VariableReference>, 358 380 definition_location: SrcSpan, 381 + alternative_variable: AlternativeVariable, 382 + name: EcoString, 359 383 } 360 384 361 385 impl<'ast> Visit<'ast> for FindVariableReferences { ··· 397 421 location: *location, 398 422 kind: VariableReferenceKind::Variable, 399 423 }) 424 + } 425 + } 426 + 427 + fn visit_typed_clause(&mut self, clause: &'ast ast::TypedClause) { 428 + // If this alternative pattern contains the variable we are finding 429 + // references for, we track that so we can find alternative definitions 430 + // of the target variable. 431 + if clause 432 + .pattern_location() 433 + .contains(self.definition_location.start) 434 + { 435 + self.alternative_variable = AlternativeVariable::Track; 436 + } 437 + 438 + for pattern in clause.pattern.iter() { 439 + self.visit_typed_pattern(pattern); 440 + } 441 + for patterns in clause.alternative_patterns.iter() { 442 + for pattern in patterns { 443 + self.visit_typed_pattern(pattern); 444 + } 445 + } 446 + 447 + self.alternative_variable = AlternativeVariable::Ignore; 448 + 449 + if let Some(guard) = &clause.guard { 450 + self.visit_typed_clause_guard(guard); 451 + } 452 + self.visit_typed_expr(&clause.then); 453 + } 454 + 455 + fn visit_typed_pattern_variable( 456 + &mut self, 457 + location: &'ast SrcSpan, 458 + name: &'ast EcoString, 459 + _type_: &'ast std::sync::Arc<Type>, 460 + _origin: &'ast VariableOrigin, 461 + ) { 462 + match self.alternative_variable { 463 + // If we are inside the same alternative pattern as the target 464 + // variable and the name is the same, this is an alternative definition 465 + // of the same variable. We don't register the reference if this is 466 + // the exact variable though, as that would result in a duplicated 467 + // reference. 468 + AlternativeVariable::Track 469 + if *name == self.name && *location != self.definition_location => 470 + { 471 + self.references.push(VariableReference { 472 + location: *location, 473 + kind: VariableReferenceKind::Variable, 474 + }); 475 + } 476 + AlternativeVariable::Track | AlternativeVariable::Ignore => {} 400 477 } 401 478 } 402 479
+2 -1
compiler-core/src/language_server/rename.rs
··· 35 35 line_numbers: &LineNumbers, 36 36 params: &RenameParams, 37 37 definition_location: SrcSpan, 38 + name: EcoString, 38 39 kind: VariableReferenceKind, 39 40 ) -> Option<WorkspaceEdit> { 40 41 if name::check_name_case( ··· 50 51 let uri = params.text_document_position.text_document.uri.clone(); 51 52 let mut edits = TextEdits::new(line_numbers); 52 53 53 - let references = find_variable_references(&module.ast, definition_location); 54 + let references = find_variable_references(&module.ast, definition_location, name); 54 55 55 56 match kind { 56 57 VariableReferenceKind::Variable => {
+56
compiler-core/src/language_server/tests/rename.rs
··· 1350 1350 find_position_of("second =") 1351 1351 ); 1352 1352 } 1353 + 1354 + // https://github.com/gleam-lang/gleam/issues/4748 1355 + #[test] 1356 + fn rename_alternative_pattern() { 1357 + assert_rename!( 1358 + " 1359 + pub fn main(x) { 1360 + case x { 1361 + #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) -> 1362 + wibble + wobble 1363 + _ -> 0 1364 + } 1365 + } 1366 + ", 1367 + "new_name", 1368 + find_position_of("wibble") 1369 + ); 1370 + } 1371 + 1372 + #[test] 1373 + fn rename_alternative_pattern_from_usage() { 1374 + assert_rename!( 1375 + " 1376 + pub fn main(x) { 1377 + case x { 1378 + #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) -> 1379 + wibble + wobble 1380 + _ -> 0 1381 + } 1382 + } 1383 + ", 1384 + "new_name", 1385 + find_position_of("wibble +") 1386 + ); 1387 + } 1388 + 1389 + #[test] 1390 + fn rename_variable_with_alternative_pattern_with_same_name() { 1391 + assert_rename!( 1392 + " 1393 + pub fn main(x) { 1394 + let some_var = 10 1395 + 1396 + case x { 1397 + #(some_var, []) | #(_, [some_var]) -> 1398 + some_var 1399 + _ -> 0 1400 + } 1401 + 1402 + some_var 1403 + } 1404 + ", 1405 + "new_name", 1406 + find_position_of("some_var") 1407 + ); 1408 + }
+27
compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__rename__rename_alternative_pattern.snap
··· 1 + --- 2 + source: compiler-core/src/language_server/tests/rename.rs 3 + expression: "\npub fn main(x) {\n case x {\n #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) ->\n wibble + wobble\n _ -> 0\n }\n}\n" 4 + --- 5 + ----- BEFORE RENAME 6 + -- app.gleam 7 + 8 + pub fn main(x) { 9 + case x { 10 + #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) -> 11 + ↑▔▔▔▔▔ 12 + wibble + wobble 13 + _ -> 0 14 + } 15 + } 16 + 17 + 18 + ----- AFTER RENAME 19 + -- app.gleam 20 + 21 + pub fn main(x) { 22 + case x { 23 + #(new_name, [wobble]) | #(wobble, [new_name, _]) | #(_, [new_name, wobble, ..]) -> 24 + new_name + wobble 25 + _ -> 0 26 + } 27 + }
+27
compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__rename__rename_alternative_pattern_from_usage.snap
··· 1 + --- 2 + source: compiler-core/src/language_server/tests/rename.rs 3 + expression: "\npub fn main(x) {\n case x {\n #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) ->\n wibble + wobble\n _ -> 0\n }\n}\n" 4 + --- 5 + ----- BEFORE RENAME 6 + -- app.gleam 7 + 8 + pub fn main(x) { 9 + case x { 10 + #(wibble, [wobble]) | #(wobble, [wibble, _]) | #(_, [wibble, wobble, ..]) -> 11 + wibble + wobble 12 + ↑▔▔▔▔▔ 13 + _ -> 0 14 + } 15 + } 16 + 17 + 18 + ----- AFTER RENAME 19 + -- app.gleam 20 + 21 + pub fn main(x) { 22 + case x { 23 + #(new_name, [wobble]) | #(wobble, [new_name, _]) | #(_, [new_name, wobble, ..]) -> 24 + new_name + wobble 25 + _ -> 0 26 + } 27 + }
+35
compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__rename__rename_variable_with_alternative_pattern_with_same_name.snap
··· 1 + --- 2 + source: compiler-core/src/language_server/tests/rename.rs 3 + expression: "\npub fn main(x) {\n let some_var = 10\n\n case x {\n #(some_var, []) | #(_, [some_var]) ->\n some_var\n _ -> 0\n }\n\n some_var\n}\n" 4 + --- 5 + ----- BEFORE RENAME 6 + -- app.gleam 7 + 8 + pub fn main(x) { 9 + let some_var = 10 10 + ↑▔▔▔▔▔▔▔ 11 + 12 + case x { 13 + #(some_var, []) | #(_, [some_var]) -> 14 + some_var 15 + _ -> 0 16 + } 17 + 18 + some_var 19 + } 20 + 21 + 22 + ----- AFTER RENAME 23 + -- app.gleam 24 + 25 + pub fn main(x) { 26 + let new_name = 10 27 + 28 + case x { 29 + #(some_var, []) | #(_, [some_var]) -> 30 + some_var 31 + _ -> 0 32 + } 33 + 34 + new_name 35 + }