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 inlining of variables from non-`let` sources

author
GearsDatapacks committer
Louis Pilfold date (Jun 10, 2025, 2:07 PM +0100) commit 22b916b5 parent 3f291eb5
+177 -45
+5 -2
compiler-core/src/ast/tests.rs
··· 6 6 use crate::build::{ExpressionPosition, Origin, Target}; 7 7 use crate::config::PackageConfig; 8 8 use crate::line_numbers::LineNumbers; 9 - use crate::type_::error::VariableOrigin; 9 + use crate::type_::error::{VariableDeclarationKind, VariableOrigin}; 10 10 use crate::type_::expression::{FunctionDefinition, Purity}; 11 11 use crate::type_::{Deprecation, PRELUDE_MODULE_NAME, Problems}; 12 12 use crate::warning::WarningEmitter; ··· 327 327 publicity: Publicity::Private, 328 328 variant: ValueConstructorVariant::LocalVariable { 329 329 location: SrcSpan { start: 5, end: 11 }, 330 - origin: VariableOrigin::Variable("wibble".into()), 330 + origin: VariableOrigin::Variable { 331 + name: "wibble".into(), 332 + kind: VariableDeclarationKind::LetPattern, 333 + }, 331 334 }, 332 335 type_: type_::int(), 333 336 },
+16 -6
compiler-core/src/language_server/code_action.rs
··· 19 19 strings::to_snake_case, 20 20 type_::{ 21 21 self, FieldMap, ModuleValueConstructor, Type, TypeVar, TypedCallArg, ValueConstructor, 22 - error::{ModuleSuggestion, VariableOrigin}, 22 + error::{ModuleSuggestion, VariableDeclarationKind, VariableOrigin}, 23 23 printer::{Names, Printer}, 24 24 }, 25 25 }; ··· 5969 5969 }; 5970 5970 5971 5971 // We can only inline it if it comes from a regular variable pattern, 5972 - // not a complex pattern or generated assignment. 5972 + // not a complex pattern or generated assignment. We can also only inline 5973 + // variables assigned by `let` statements, as it doesn't make sense to do 5974 + // so with any other kind of variable. 5973 5975 match origin { 5974 - VariableOrigin::Variable(_) => {} 5975 - VariableOrigin::LabelShorthand(_) 5976 + VariableOrigin::Variable { 5977 + kind: VariableDeclarationKind::LetPattern, 5978 + .. 5979 + } => {} 5980 + VariableOrigin::Variable { .. } 5981 + | VariableOrigin::LabelShorthand(_) 5976 5982 | VariableOrigin::AssignmentPattern 5977 5983 | VariableOrigin::Generated => return, 5978 5984 } ··· 5989 5995 ) { 5990 5996 // We can only inline it if it is a regular variable pattern 5991 5997 match origin { 5992 - VariableOrigin::Variable(_) => {} 5993 - VariableOrigin::LabelShorthand(_) 5998 + VariableOrigin::Variable { 5999 + kind: VariableDeclarationKind::LetPattern, 6000 + .. 6001 + } => {} 6002 + VariableOrigin::Variable { .. } 6003 + | VariableOrigin::LabelShorthand(_) 5994 6004 | VariableOrigin::AssignmentPattern 5995 6005 | VariableOrigin::Generated => return, 5996 6006 }
+5 -3
compiler-core/src/language_server/engine.rs
··· 640 640 }) if location.contains(byte_index) => match origin { 641 641 Some(VariableOrigin::Generated) => None, 642 642 Some( 643 - VariableOrigin::Variable(_) 643 + VariableOrigin::Variable { .. } 644 644 | VariableOrigin::AssignmentPattern 645 645 | VariableOrigin::LabelShorthand(_), 646 646 ) ··· 700 700 Some(VariableOrigin::LabelShorthand(_)) => { 701 701 VariableReferenceKind::LabelShorthand 702 702 } 703 - Some(VariableOrigin::AssignmentPattern | VariableOrigin::Variable(_)) 703 + Some( 704 + VariableOrigin::AssignmentPattern | VariableOrigin::Variable { .. }, 705 + ) 704 706 | None => VariableReferenceKind::Variable, 705 707 }; 706 708 rename_local_variable(module, &lines, &params, definition_location, rename_kind) ··· 777 779 Some( 778 780 VariableOrigin::LabelShorthand(_) 779 781 | VariableOrigin::AssignmentPattern 780 - | VariableOrigin::Variable(_), 782 + | VariableOrigin::Variable { .. }, 781 783 ) 782 784 | None => { 783 785 let variable_references =
+57
compiler-core/src/language_server/tests/action.rs
··· 8681 8681 find_position_of("sum = ").to_selection() 8682 8682 ); 8683 8683 } 8684 + 8685 + // https://github.com/gleam-lang/gleam/issues/4660 8686 + #[test] 8687 + fn no_inline_variable_action_for_parameter() { 8688 + assert_no_code_actions!( 8689 + INLINE_VARIABLE, 8690 + " 8691 + pub fn main() { 8692 + let x = fn(something) { 8693 + something 8694 + } 8695 + 8696 + x 8697 + } 8698 + ", 8699 + find_position_of("something") 8700 + .nth_occurrence(2) 8701 + .to_selection() 8702 + ); 8703 + } 8704 + 8705 + #[test] 8706 + fn no_inline_variable_action_for_use_pattern() { 8707 + assert_no_code_actions!( 8708 + INLINE_VARIABLE, 8709 + " 8710 + pub fn main() { 8711 + let x = { 8712 + use something <- todo 8713 + something 8714 + } 8715 + 8716 + x 8717 + } 8718 + ", 8719 + find_position_of("something").to_selection() 8720 + ); 8721 + } 8722 + 8723 + #[test] 8724 + fn no_inline_variable_action_for_case_pattern() { 8725 + assert_no_code_actions!( 8726 + INLINE_VARIABLE, 8727 + " 8728 + pub fn main() { 8729 + let x = case todo { 8730 + something -> something 8731 + } 8732 + 8733 + x 8734 + } 8735 + ", 8736 + find_position_of("something") 8737 + .nth_occurrence(2) 8738 + .to_selection() 8739 + ); 8740 + }
+68 -29
compiler-core/src/parse.rs
··· 72 72 use crate::exhaustiveness::CompiledCase; 73 73 use crate::parse::extra::ModuleExtra; 74 74 use crate::type_::Deprecation; 75 - use crate::type_::error::VariableOrigin; 75 + use crate::type_::error::{VariableDeclarationKind, VariableOrigin}; 76 76 use crate::type_::expression::{Implementations, Purity}; 77 77 use crate::warning::{DeprecatedSyntaxWarning, WarningEmitter}; 78 78 use camino::Utf8PathBuf; ··· 986 986 fn parse_use_assignment(&mut self) -> Result<Option<UntypedUseAssignment>, ParseError> { 987 987 let start = self.tok0.as_ref().map(|t| t.0).unwrap_or(0); 988 988 989 - let pattern = self.parse_pattern()?.ok_or_else(|| ParseError { 990 - error: ParseErrorType::ExpectedPattern, 991 - location: SrcSpan { start, end: start }, 992 - })?; 989 + let pattern = self 990 + .parse_pattern(PatternPosition::UsePattern)? 991 + .ok_or_else(|| ParseError { 992 + error: ParseErrorType::ExpectedPattern, 993 + location: SrcSpan { start, end: start }, 994 + })?; 993 995 994 996 let annotation = self.parse_type_annotation(&Token::Colon)?; 995 997 let end = match annotation { ··· 1017 1019 } 1018 1020 _ => AssignmentKind::Let, 1019 1021 }; 1020 - let pattern = match self.parse_pattern()? { 1022 + let pattern = match self.parse_pattern(PatternPosition::LetAssignment)? { 1021 1023 Some(p) => p, 1022 1024 _ => { 1023 1025 // DUPE: 62884 ··· 1174 1176 } 1175 1177 1176 1178 // The left side of an "=" or a "->" 1177 - fn parse_pattern(&mut self) -> Result<Option<UntypedPattern>, ParseError> { 1179 + fn parse_pattern( 1180 + &mut self, 1181 + position: PatternPosition, 1182 + ) -> Result<Option<UntypedPattern>, ParseError> { 1178 1183 let pattern = match self.tok0.take() { 1179 1184 // Pattern::Var or Pattern::Constructor start 1180 1185 Some((start, Token::Name { name }, end)) => { ··· 1189 1194 // We're doing this to get a better error message instead of a generic 1190 1195 // `I was expecting a type`, you can have a look at this issue to get 1191 1196 // a better idea: https://github.com/gleam-lang/gleam/issues/2841. 1192 - match self.expect_constructor_pattern(Some((start, name, end))) { 1197 + match self.expect_constructor_pattern(Some((start, name, end)), position) { 1193 1198 Ok(result) => result, 1194 1199 Err(ParseError { 1195 1200 location: SrcSpan { end, .. }, ··· 1209 1214 SrcSpan { start, end }, 1210 1215 ); 1211 1216 } 1212 - _ => Pattern::Variable { 1213 - origin: VariableOrigin::Variable(name.clone()), 1214 - location: SrcSpan { start, end }, 1215 - name, 1216 - type_: (), 1217 - }, 1217 + _ => { 1218 + let kind = match position { 1219 + PatternPosition::LetAssignment => { 1220 + VariableDeclarationKind::LetPattern 1221 + } 1222 + PatternPosition::CaseClause => { 1223 + VariableDeclarationKind::ClausePattern 1224 + } 1225 + PatternPosition::UsePattern => VariableDeclarationKind::UsePattern, 1226 + }; 1227 + Pattern::Variable { 1228 + origin: VariableOrigin::Variable { 1229 + name: name.clone(), 1230 + kind, 1231 + }, 1232 + location: SrcSpan { start, end }, 1233 + name, 1234 + type_: (), 1235 + } 1236 + } 1218 1237 } 1219 1238 } 1220 1239 } 1221 1240 // Constructor 1222 1241 Some((start, tok @ Token::UpName { .. }, end)) => { 1223 1242 self.tok0 = Some((start, tok, end)); 1224 - self.expect_constructor_pattern(None)? 1243 + self.expect_constructor_pattern(None, position)? 1225 1244 } 1226 1245 1227 1246 Some((start, Token::DiscardName { name }, end)) => { ··· 1333 1352 Some((start, Token::Hash, _)) => { 1334 1353 self.advance(); 1335 1354 let _ = self.expect_one(&Token::LeftParen)?; 1336 - let elements = 1337 - Parser::series_of(self, &Parser::parse_pattern, Some(&Token::Comma))?; 1355 + let elements = Parser::series_of( 1356 + self, 1357 + &|this| this.parse_pattern(position), 1358 + Some(&Token::Comma), 1359 + )?; 1338 1360 let (_, end) = self.expect_one_following_series(&Token::RightParen, "a pattern")?; 1339 1361 Pattern::Tuple { 1340 1362 location: SrcSpan { start, end }, ··· 1349 1371 &|s| { 1350 1372 Parser::parse_bit_array_segment( 1351 1373 s, 1352 - &|s| match Parser::parse_pattern(s) { 1374 + &|s| match s.parse_pattern(position) { 1353 1375 Ok(Some(Pattern::BitArray { location, .. })) => { 1354 1376 parse_error(ParseErrorType::NestedBitArrayPattern, location) 1355 1377 } ··· 1372 1394 Some((start, Token::LeftSquare, _)) => { 1373 1395 self.advance(); 1374 1396 let (elements, elements_end_with_comma) = self.series_of_has_trailing_separator( 1375 - &Parser::parse_pattern, 1397 + &|this| this.parse_pattern(position), 1376 1398 Some(&Token::Comma), 1377 1399 )?; 1378 1400 ··· 1392 1414 } 1393 1415 1394 1416 self.advance(); 1395 - let pat = self.parse_pattern()?; 1417 + let pat = self.parse_pattern(position)?; 1396 1418 if self.maybe_one(&Token::Comma).is_some() { 1397 1419 // See if there's a list of items after the tail, 1398 1420 // like `[..wibble, wobble, wabble]` 1399 1421 let elements = Parser::series_of( 1400 1422 self, 1401 - &Parser::parse_pattern, 1423 + &|this| this.parse_pattern(position), 1402 1424 Some(&Token::Comma), 1403 1425 ); 1404 1426 match elements { ··· 1501 1523 // pattern, pattern if -> expr 1502 1524 // pattern, pattern | pattern, pattern if -> expr 1503 1525 fn parse_case_clause(&mut self) -> Result<Option<UntypedClause>, ParseError> { 1504 - let patterns = self.parse_patterns()?; 1526 + let patterns = self.parse_patterns(PatternPosition::CaseClause)?; 1505 1527 match &patterns.first() { 1506 1528 Some(lead) => { 1507 1529 let mut alternative_patterns = vec![]; ··· 1509 1531 if self.maybe_one(&Token::Vbar).is_none() { 1510 1532 break; 1511 1533 } 1512 - alternative_patterns.push(self.parse_patterns()?); 1534 + alternative_patterns.push(self.parse_patterns(PatternPosition::CaseClause)?); 1513 1535 } 1514 1536 let guard = self.parse_case_clause_guard(false)?; 1515 1537 let (arr_s, arr_e) = self ··· 1544 1566 _ => Ok(None), 1545 1567 } 1546 1568 } 1547 - fn parse_patterns(&mut self) -> Result<Vec<UntypedPattern>, ParseError> { 1548 - Parser::series_of(self, &Parser::parse_pattern, Some(&Token::Comma)) 1569 + fn parse_patterns( 1570 + &mut self, 1571 + position: PatternPosition, 1572 + ) -> Result<Vec<UntypedPattern>, ParseError> { 1573 + Parser::series_of( 1574 + self, 1575 + &|this| this.parse_pattern(position), 1576 + Some(&Token::Comma), 1577 + ) 1549 1578 } 1550 1579 1551 1580 // examples: ··· 1800 1829 fn expect_constructor_pattern( 1801 1830 &mut self, 1802 1831 module: Option<(u32, EcoString, u32)>, 1832 + position: PatternPosition, 1803 1833 ) -> Result<UntypedPattern, ParseError> { 1804 1834 let (name_start, name, name_end) = self.expect_upname()?; 1805 1835 let mut start = name_start; 1806 - let (args, spread, end) = self.parse_constructor_pattern_args(name_end)?; 1836 + let (args, spread, end) = self.parse_constructor_pattern_args(name_end, position)?; 1807 1837 if let Some((s, _, _)) = module { 1808 1838 start = s; 1809 1839 } ··· 1825 1855 fn parse_constructor_pattern_args( 1826 1856 &mut self, 1827 1857 upname_end: u32, 1858 + position: PatternPosition, 1828 1859 ) -> Result<(Vec<CallArg<UntypedPattern>>, Option<SrcSpan>, u32), ParseError> { 1829 1860 if self.maybe_one(&Token::LeftParen).is_some() { 1830 1861 let (args, args_end_with_comma) = self.series_of_has_trailing_separator( 1831 - &Parser::parse_constructor_pattern_arg, 1862 + &|this| this.parse_constructor_pattern_arg(position), 1832 1863 Some(&Token::Comma), 1833 1864 )?; 1834 1865 ··· 1858 1889 // <pattern> 1859 1890 fn parse_constructor_pattern_arg( 1860 1891 &mut self, 1892 + position: PatternPosition, 1861 1893 ) -> Result<Option<CallArg<UntypedPattern>>, ParseError> { 1862 1894 match (self.tok0.take(), self.tok1.take()) { 1863 1895 // named arg 1864 1896 (Some((start, Token::Name { name }, _)), Some((_, Token::Colon, end))) => { 1865 1897 self.advance(); 1866 1898 self.advance(); 1867 - match self.parse_pattern()? { 1899 + match self.parse_pattern(position)? { 1868 1900 Some(value) => Ok(Some(CallArg { 1869 1901 implicit: None, 1870 1902 location: SrcSpan { ··· 1894 1926 (t0, t1) => { 1895 1927 self.tok0 = t0; 1896 1928 self.tok1 = t1; 1897 - match self.parse_pattern()? { 1929 + match self.parse_pattern(position)? { 1898 1930 Some(value) => Ok(Some(CallArg { 1899 1931 implicit: None, 1900 1932 location: value.location(), ··· 4383 4415 FollowingPipe, 4384 4416 Other, 4385 4417 } 4418 + 4419 + #[derive(Debug, Clone, Copy)] 4420 + enum PatternPosition { 4421 + LetAssignment, 4422 + CaseClause, 4423 + UsePattern, 4424 + }
+14 -2
compiler-core/src/type_/error.rs
··· 712 712 /// The origin of a variable. Used to determine how it can be ignored when unused. 713 713 pub enum VariableOrigin { 714 714 /// A variable that can be ignored by prefixing with an underscore, `_name` 715 - Variable(EcoString), 715 + Variable { 716 + name: EcoString, 717 + kind: VariableDeclarationKind, 718 + }, 716 719 /// A variable from label shorthand syntax, which can be ignored with an underscore: `label: _` 717 720 LabelShorthand(EcoString), 718 721 /// A variable from an assignment pattern, which can be ignored by removing `as name`, ··· 721 724 Generated, 722 725 } 723 726 727 + #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] 728 + pub enum VariableDeclarationKind { 729 + LetPattern, 730 + UsePattern, 731 + ClausePattern, 732 + FunctionParameter, 733 + Implicit, 734 + } 735 + 724 736 impl VariableOrigin { 725 737 pub fn how_to_ignore(&self) -> Option<String> { 726 738 match self { 727 - VariableOrigin::Variable(name) => { 739 + VariableOrigin::Variable { name, .. } => { 728 740 Some(format!("You can ignore it with an underscore: `_{name}`.")) 729 741 } 730 742 VariableOrigin::LabelShorthand(label) => Some(format!(
+4 -1
compiler-core/src/type_/expression.rs
··· 4269 4269 let origin = if name == CAPTURE_VARIABLE { 4270 4270 VariableOrigin::Generated 4271 4271 } else { 4272 - VariableOrigin::Variable(name.clone()) 4272 + VariableOrigin::Variable { 4273 + name: name.clone(), 4274 + kind: VariableDeclarationKind::FunctionParameter, 4275 + } 4273 4276 }; 4274 4277 4275 4278 // Insert a variable for the argument into the environment
+8 -2
compiler-core/src/type_/pattern.rs
··· 213 213 self.environment.insert_local_variable( 214 214 name.clone(), 215 215 variable.definition_location().span, 216 - VariableOrigin::Variable(name), 216 + VariableOrigin::Variable { 217 + name, 218 + kind: VariableDeclarationKind::Implicit, 219 + }, 217 220 type_, 218 221 ); 219 222 } ··· 671 674 right, 672 675 string(), 673 676 right_location, 674 - VariableOrigin::Variable(right.clone()), 677 + VariableOrigin::Variable { 678 + name: right.clone(), 679 + kind: VariableDeclarationKind::ClausePattern, 680 + }, 675 681 ); 676 682 } 677 683 AssignName::Discard(_) => {