diff --git a/CHANGELOG.md b/CHANGELOG.md index 70ba01f9fd1..6aabe86512d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ for record fields in custom type definitions. ([cysabi](https://github.com/cysabi)) +- Fixed a bug where the "Inline variable" code action would be offered for + function parameters and other invalid cases. + ([Surya Rose](https://github.com/GearsDatapacks)) + +- Fixed a bug where the "Inline variable" code action would not be applied + correctly to variables using label shorthand syntax. + ([Surya Rose](https://github.com/GearsDatapacks)) + ## v1.11.1 - 2025-06-05 ### Compiler diff --git a/compiler-core/src/ast/tests.rs b/compiler-core/src/ast/tests.rs index eca094164cf..065044ed76f 100644 --- a/compiler-core/src/ast/tests.rs +++ b/compiler-core/src/ast/tests.rs @@ -6,7 +6,7 @@ use crate::analyse::TargetSupport; use crate::build::{ExpressionPosition, Origin, Target}; use crate::config::PackageConfig; use crate::line_numbers::LineNumbers; -use crate::type_::error::VariableOrigin; +use crate::type_::error::{VariableDeclaration, VariableOrigin, VariableSyntax}; use crate::type_::expression::{FunctionDefinition, Purity}; use crate::type_::{Deprecation, PRELUDE_MODULE_NAME, Problems}; use crate::warning::WarningEmitter; @@ -327,7 +327,10 @@ wibble}"#, publicity: Publicity::Private, variant: ValueConstructorVariant::LocalVariable { location: SrcSpan { start: 5, end: 11 }, - origin: VariableOrigin::Variable("wibble".into()), + origin: VariableOrigin { + syntax: VariableSyntax::Variable("wibble".into()), + declaration: VariableDeclaration::LetPattern, + }, }, type_: type_::int(), }, diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs index b843af8b51d..07550c45295 100644 --- a/compiler-core/src/language_server/code_action.rs +++ b/compiler-core/src/language_server/code_action.rs @@ -19,7 +19,7 @@ use crate::{ strings::to_snake_case, type_::{ self, FieldMap, ModuleValueConstructor, Type, TypeVar, TypedCallArg, ValueConstructor, - error::{ModuleSuggestion, VariableOrigin}, + error::{ModuleSuggestion, VariableDeclaration, VariableOrigin}, printer::{Names, Printer}, }, }; @@ -35,7 +35,7 @@ use super::{ edits::{add_newlines_after_import, get_import_edit, position_of_first_definition_if_import}, engine::{overlaps, within}, files::FileSystemProxy, - reference::find_variable_references, + reference::{VariableReferenceKind, find_variable_references}, src_span_to_lsp_range, url_from_path, }; @@ -5883,11 +5883,10 @@ impl<'a> InlineVariable<'a> { } fn maybe_inline(&mut self, location: SrcSpan) { - let variable_location = - match find_variable_references(&self.module.ast, location).as_slice() { - [only_reference] => only_reference.location, - _ => return, - }; + let reference = match find_variable_references(&self.module.ast, location).as_slice() { + [only_reference] => *only_reference, + _ => return, + }; let Some(ast::Statement::Assignment(assignment)) = self.module.ast.find_statement(location.start) @@ -5919,7 +5918,15 @@ impl<'a> InlineVariable<'a> { .get(value_location.start as usize..value_location.end as usize) .expect("Span is valid"); - self.edits.replace(variable_location, value.into()); + match reference.kind { + VariableReferenceKind::Variable => { + self.edits.replace(reference.location, value.into()); + } + VariableReferenceKind::LabelShorthand => { + self.edits + .insert(reference.location.end, format!(" {value}")); + } + } let mut location = assignment.location; @@ -5961,13 +5968,14 @@ impl<'ast> ast::visit::Visit<'ast> for InlineVariable<'ast> { return; }; - // We can only inline it if it comes from a regular variable pattern, - // not a complex pattern or generated assignment. - match origin { - VariableOrigin::Variable(_) => {} - VariableOrigin::LabelShorthand(_) - | VariableOrigin::AssignmentPattern - | VariableOrigin::Generated => return, + // We can only inline variables assigned by `let` statements, as it + //doesn't make sense to do so with any other kind of variable. + match origin.declaration { + VariableDeclaration::LetPattern => {} + VariableDeclaration::UsePattern + | VariableDeclaration::ClausePattern + | VariableDeclaration::FunctionParameter + | VariableDeclaration::Generated => return, } self.maybe_inline(*location); @@ -5980,12 +5988,14 @@ impl<'ast> ast::visit::Visit<'ast> for InlineVariable<'ast> { _type: &'ast Arc, origin: &'ast VariableOrigin, ) { - // We can only inline it if it is a regular variable pattern - match origin { - VariableOrigin::Variable(_) => {} - VariableOrigin::LabelShorthand(_) - | VariableOrigin::AssignmentPattern - | VariableOrigin::Generated => return, + // We can only inline variables assigned by `let` statements, as it + //doesn't make sense to do so with any other kind of variable. + match origin.declaration { + VariableDeclaration::LetPattern => {} + VariableDeclaration::UsePattern + | VariableDeclaration::ClausePattern + | VariableDeclaration::FunctionParameter + | VariableDeclaration::Generated => return, } let range = self.edits.src_span_to_lsp_range(*location); diff --git a/compiler-core/src/language_server/engine.rs b/compiler-core/src/language_server/engine.rs index 2cfd48a1ace..00d83bd7b7b 100644 --- a/compiler-core/src/language_server/engine.rs +++ b/compiler-core/src/language_server/engine.rs @@ -19,7 +19,7 @@ use crate::{ type_::{ self, Deprecation, ModuleInterface, Type, TypeConstructor, ValueConstructor, ValueConstructorVariant, - error::{Named, VariableOrigin}, + error::{Named, VariableSyntax}, printer::Printer, }, }; @@ -637,12 +637,12 @@ where Ok(match reference_for_ast_node(found, ¤t_module.name) { Some(Referenced::LocalVariable { location, origin, .. - }) if location.contains(byte_index) => match origin { - Some(VariableOrigin::Generated) => None, + }) if location.contains(byte_index) => match origin.map(|origin| origin.syntax) { + Some(VariableSyntax::Generated) => None, Some( - VariableOrigin::Variable(_) - | VariableOrigin::AssignmentPattern - | VariableOrigin::LabelShorthand(_), + VariableSyntax::Variable { .. } + | VariableSyntax::AssignmentPattern + | VariableSyntax::LabelShorthand(_), ) | None => success_response(location), }, @@ -695,12 +695,14 @@ where definition_location, .. }) => { - let rename_kind = match origin { - Some(VariableOrigin::Generated) => return Ok(None), - Some(VariableOrigin::LabelShorthand(_)) => { + let rename_kind = match origin.map(|origin| origin.syntax) { + Some(VariableSyntax::Generated) => return Ok(None), + Some(VariableSyntax::LabelShorthand(_)) => { VariableReferenceKind::LabelShorthand } - Some(VariableOrigin::AssignmentPattern | VariableOrigin::Variable(_)) + Some( + VariableSyntax::AssignmentPattern | VariableSyntax::Variable { .. }, + ) | None => VariableReferenceKind::Variable, }; rename_local_variable(module, &lines, ¶ms, definition_location, rename_kind) @@ -772,12 +774,12 @@ where origin, definition_location, location, - }) if location.contains(byte_index) => match origin { - Some(VariableOrigin::Generated) => None, + }) if location.contains(byte_index) => match origin.map(|origin| origin.syntax) { + Some(VariableSyntax::Generated) => None, Some( - VariableOrigin::LabelShorthand(_) - | VariableOrigin::AssignmentPattern - | VariableOrigin::Variable(_), + VariableSyntax::LabelShorthand(_) + | VariableSyntax::AssignmentPattern + | VariableSyntax::Variable { .. }, ) | None => { let variable_references = diff --git a/compiler-core/src/language_server/reference.rs b/compiler-core/src/language_server/reference.rs index 328a263f140..77ead3b21b1 100644 --- a/compiler-core/src/language_server/reference.rs +++ b/compiler-core/src/language_server/reference.rs @@ -329,11 +329,13 @@ fn find_references_in_module( } } +#[derive(Debug, Clone, Copy)] pub struct VariableReference { pub location: SrcSpan, pub kind: VariableReferenceKind, } +#[derive(Debug, Clone, Copy)] pub enum VariableReferenceKind { Variable, LabelShorthand, diff --git a/compiler-core/src/language_server/tests/action.rs b/compiler-core/src/language_server/tests/action.rs index 72633ce5f41..4e03f82ff13 100644 --- a/compiler-core/src/language_server/tests/action.rs +++ b/compiler-core/src/language_server/tests/action.rs @@ -8661,3 +8661,80 @@ pub fn main() { find_position_of("add").to_selection() ); } + +// https://github.com/gleam-lang/gleam/issues/4660#issuecomment-2932371619 +#[test] +fn inline_variable_label_shorthand() { + assert_code_action!( + INLINE_VARIABLE, + " +pub type Example { + Example(sum: Int, nil: Nil) +} + +pub fn main() { + let sum = 1 + 1 + + Example(Nil, sum:) +} +", + find_position_of("sum = ").to_selection() + ); +} + +// https://github.com/gleam-lang/gleam/issues/4660 +#[test] +fn no_inline_variable_action_for_parameter() { + assert_no_code_actions!( + INLINE_VARIABLE, + " +pub fn main() { + let x = fn(something) { + something + } + + x +} +", + find_position_of("something") + .nth_occurrence(2) + .to_selection() + ); +} + +#[test] +fn no_inline_variable_action_for_use_pattern() { + assert_no_code_actions!( + INLINE_VARIABLE, + " +pub fn main() { + let x = { + use something <- todo + something + } + + x +} +", + find_position_of("something").to_selection() + ); +} + +#[test] +fn no_inline_variable_action_for_case_pattern() { + assert_no_code_actions!( + INLINE_VARIABLE, + " +pub fn main() { + let x = case todo { + something -> something + } + + x +} +", + find_position_of("something") + .nth_occurrence(2) + .to_selection() + ); +} diff --git a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__inline_variable_label_shorthand.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__inline_variable_label_shorthand.snap new file mode 100644 index 00000000000..3d4c9c06790 --- /dev/null +++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__inline_variable_label_shorthand.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/language_server/tests/action.rs +expression: "\npub type Example {\n Example(sum: Int, nil: Nil)\n}\n\npub fn main() {\n let sum = 1 + 1\n\n Example(Nil, sum:)\n}\n" +--- +----- BEFORE ACTION + +pub type Example { + Example(sum: Int, nil: Nil) +} + +pub fn main() { + let sum = 1 + 1 + ↑ + + Example(Nil, sum:) +} + + +----- AFTER ACTION + +pub type Example { + Example(sum: Int, nil: Nil) +} + +pub fn main() { + Example(Nil, sum: 1 + 1) +} diff --git a/compiler-core/src/parse.rs b/compiler-core/src/parse.rs index 113d30b27f9..71941f1f37d 100644 --- a/compiler-core/src/parse.rs +++ b/compiler-core/src/parse.rs @@ -72,7 +72,7 @@ use crate::error::wrap; use crate::exhaustiveness::CompiledCase; use crate::parse::extra::ModuleExtra; use crate::type_::Deprecation; -use crate::type_::error::VariableOrigin; +use crate::type_::error::{VariableDeclaration, VariableOrigin, VariableSyntax}; use crate::type_::expression::{Implementations, Purity}; use crate::warning::{DeprecatedSyntaxWarning, WarningEmitter}; use camino::Utf8PathBuf; @@ -986,10 +986,12 @@ where fn parse_use_assignment(&mut self) -> Result, ParseError> { let start = self.tok0.as_ref().map(|t| t.0).unwrap_or(0); - let pattern = self.parse_pattern()?.ok_or_else(|| ParseError { - error: ParseErrorType::ExpectedPattern, - location: SrcSpan { start, end: start }, - })?; + let pattern = self + .parse_pattern(PatternPosition::UsePattern)? + .ok_or_else(|| ParseError { + error: ParseErrorType::ExpectedPattern, + location: SrcSpan { start, end: start }, + })?; let annotation = self.parse_type_annotation(&Token::Colon)?; let end = match annotation { @@ -1017,7 +1019,7 @@ where } _ => AssignmentKind::Let, }; - let pattern = match self.parse_pattern()? { + let pattern = match self.parse_pattern(PatternPosition::LetAssignment)? { Some(p) => p, _ => { // DUPE: 62884 @@ -1174,7 +1176,10 @@ where } // The left side of an "=" or a "->" - fn parse_pattern(&mut self) -> Result, ParseError> { + fn parse_pattern( + &mut self, + position: PatternPosition, + ) -> Result, ParseError> { let pattern = match self.tok0.take() { // Pattern::Var or Pattern::Constructor start Some((start, Token::Name { name }, end)) => { @@ -1189,7 +1194,7 @@ where // We're doing this to get a better error message instead of a generic // `I was expecting a type`, you can have a look at this issue to get // a better idea: https://github.com/gleam-lang/gleam/issues/2841. - match self.expect_constructor_pattern(Some((start, name, end))) { + match self.expect_constructor_pattern(Some((start, name, end)), position) { Ok(result) => result, Err(ParseError { location: SrcSpan { end, .. }, @@ -1210,7 +1215,10 @@ where ); } _ => Pattern::Variable { - origin: VariableOrigin::Variable(name.clone()), + origin: VariableOrigin { + syntax: VariableSyntax::Variable(name.clone()), + declaration: position.to_declaration(), + }, location: SrcSpan { start, end }, name, type_: (), @@ -1221,7 +1229,7 @@ where // Constructor Some((start, tok @ Token::UpName { .. }, end)) => { self.tok0 = Some((start, tok, end)); - self.expect_constructor_pattern(None)? + self.expect_constructor_pattern(None, position)? } Some((start, Token::DiscardName { name }, end)) => { @@ -1333,8 +1341,11 @@ where Some((start, Token::Hash, _)) => { self.advance(); let _ = self.expect_one(&Token::LeftParen)?; - let elements = - Parser::series_of(self, &Parser::parse_pattern, Some(&Token::Comma))?; + let elements = Parser::series_of( + self, + &|this| this.parse_pattern(position), + Some(&Token::Comma), + )?; let (_, end) = self.expect_one_following_series(&Token::RightParen, "a pattern")?; Pattern::Tuple { location: SrcSpan { start, end }, @@ -1349,7 +1360,7 @@ where &|s| { Parser::parse_bit_array_segment( s, - &|s| match Parser::parse_pattern(s) { + &|s| match s.parse_pattern(position) { Ok(Some(Pattern::BitArray { location, .. })) => { parse_error(ParseErrorType::NestedBitArrayPattern, location) } @@ -1372,7 +1383,7 @@ where Some((start, Token::LeftSquare, _)) => { self.advance(); let (elements, elements_end_with_comma) = self.series_of_has_trailing_separator( - &Parser::parse_pattern, + &|this| this.parse_pattern(position), Some(&Token::Comma), )?; @@ -1392,13 +1403,13 @@ where } self.advance(); - let pat = self.parse_pattern()?; + let pat = self.parse_pattern(position)?; if self.maybe_one(&Token::Comma).is_some() { // See if there's a list of items after the tail, // like `[..wibble, wobble, wabble]` let elements = Parser::series_of( self, - &Parser::parse_pattern, + &|this| this.parse_pattern(position), Some(&Token::Comma), ); match elements { @@ -1501,7 +1512,7 @@ where // pattern, pattern if -> expr // pattern, pattern | pattern, pattern if -> expr fn parse_case_clause(&mut self) -> Result, ParseError> { - let patterns = self.parse_patterns()?; + let patterns = self.parse_patterns(PatternPosition::CaseClause)?; match &patterns.first() { Some(lead) => { let mut alternative_patterns = vec![]; @@ -1509,7 +1520,7 @@ where if self.maybe_one(&Token::Vbar).is_none() { break; } - alternative_patterns.push(self.parse_patterns()?); + alternative_patterns.push(self.parse_patterns(PatternPosition::CaseClause)?); } let guard = self.parse_case_clause_guard(false)?; let (arr_s, arr_e) = self @@ -1544,8 +1555,15 @@ where _ => Ok(None), } } - fn parse_patterns(&mut self) -> Result, ParseError> { - Parser::series_of(self, &Parser::parse_pattern, Some(&Token::Comma)) + fn parse_patterns( + &mut self, + position: PatternPosition, + ) -> Result, ParseError> { + Parser::series_of( + self, + &|this| this.parse_pattern(position), + Some(&Token::Comma), + ) } // examples: @@ -1800,10 +1818,11 @@ where fn expect_constructor_pattern( &mut self, module: Option<(u32, EcoString, u32)>, + position: PatternPosition, ) -> Result { let (name_start, name, name_end) = self.expect_upname()?; let mut start = name_start; - let (args, spread, end) = self.parse_constructor_pattern_args(name_end)?; + let (args, spread, end) = self.parse_constructor_pattern_args(name_end, position)?; if let Some((s, _, _)) = module { start = s; } @@ -1825,10 +1844,11 @@ where fn parse_constructor_pattern_args( &mut self, upname_end: u32, + position: PatternPosition, ) -> Result<(Vec>, Option, u32), ParseError> { if self.maybe_one(&Token::LeftParen).is_some() { let (args, args_end_with_comma) = self.series_of_has_trailing_separator( - &Parser::parse_constructor_pattern_arg, + &|this| this.parse_constructor_pattern_arg(position), Some(&Token::Comma), )?; @@ -1858,13 +1878,14 @@ where // fn parse_constructor_pattern_arg( &mut self, + position: PatternPosition, ) -> Result>, ParseError> { match (self.tok0.take(), self.tok1.take()) { // named arg (Some((start, Token::Name { name }, _)), Some((_, Token::Colon, end))) => { self.advance(); self.advance(); - match self.parse_pattern()? { + match self.parse_pattern(position)? { Some(value) => Ok(Some(CallArg { implicit: None, location: SrcSpan { @@ -1881,7 +1902,10 @@ where location: SrcSpan { start, end }, label: Some(name.clone()), value: UntypedPattern::Variable { - origin: VariableOrigin::LabelShorthand(name.clone()), + origin: VariableOrigin { + syntax: VariableSyntax::LabelShorthand(name.clone()), + declaration: position.to_declaration(), + }, name, location: SrcSpan { start, end }, type_: (), @@ -1894,7 +1918,7 @@ where (t0, t1) => { self.tok0 = t0; self.tok1 = t1; - match self.parse_pattern()? { + match self.parse_pattern(position)? { Some(value) => Ok(Some(CallArg { implicit: None, location: value.location(), @@ -4383,3 +4407,20 @@ enum ExpressionUnitContext { FollowingPipe, Other, } + +#[derive(Debug, Clone, Copy)] +pub enum PatternPosition { + LetAssignment, + CaseClause, + UsePattern, +} + +impl PatternPosition { + pub fn to_declaration(&self) -> VariableDeclaration { + match self { + PatternPosition::LetAssignment => VariableDeclaration::LetPattern, + PatternPosition::CaseClause => VariableDeclaration::ClausePattern, + PatternPosition::UsePattern => VariableDeclaration::UsePattern, + } + } +} diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__arithmetic_in_guards.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__arithmetic_in_guards.snap index 6c96ef2b640..3435ecdcfcd 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__arithmetic_in_guards.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__arithmetic_in_guards.snap @@ -42,9 +42,12 @@ expression: "\ncase 2, 3 {\n x, y if x + y == 1 -> True\n}" }, name: "x", type_: (), - origin: Variable( - "x", - ), + origin: VariableOrigin { + syntax: Variable( + "x", + ), + declaration: ClausePattern, + }, }, Variable { location: SrcSpan { @@ -53,9 +56,12 @@ expression: "\ncase 2, 3 {\n x, y if x + y == 1 -> True\n}" }, name: "y", type_: (), - origin: Variable( - "y", - ), + origin: VariableOrigin { + syntax: Variable( + "y", + ), + declaration: ClausePattern, + }, }, ], alternative_patterns: [], diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples.snap index 689734ac96c..240c99a1106 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples.snap @@ -56,9 +56,12 @@ expression: "\nlet tup = #(#(#(#(4))))\n{{{tup.0}.0}.0}.0\n" }, name: "tup", type_: (), - origin: Variable( - "tup", - ), + origin: VariableOrigin { + syntax: Variable( + "tup", + ), + declaration: LetPattern, + }, }, kind: Let, compiled_case: CompiledCase { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples_no_block.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples_no_block.snap index 33e91f5f65d..a436cd95728 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples_no_block.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__deeply_nested_tuples_no_block.snap @@ -56,9 +56,12 @@ expression: "\nlet tup = #(#(#(#(4))))\ntup.0.0.0.0\n" }, name: "tup", type_: (), - origin: Variable( - "tup", - ), + origin: VariableOrigin { + syntax: Variable( + "tup", + ), + declaration: LetPattern, + }, }, kind: Let, compiled_case: CompiledCase { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__inner_single_quote_parses.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__inner_single_quote_parses.snap index a2a8bb7e604..0991a3a962d 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__inner_single_quote_parses.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__inner_single_quote_parses.snap @@ -23,9 +23,12 @@ expression: "\nlet a = \"inner 'quotes'\"\n" }, name: "a", type_: (), - origin: Variable( - "a", - ), + origin: VariableOrigin { + syntax: Variable( + "a", + ), + declaration: LetPattern, + }, }, kind: Let, compiled_case: CompiledCase { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples.snap index e3dc96275bc..4b827e6b569 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples.snap @@ -48,9 +48,12 @@ expression: "\nlet tup = #(#(5, 6))\n{tup.0}.1\n" }, name: "tup", type_: (), - origin: Variable( - "tup", - ), + origin: VariableOrigin { + syntax: Variable( + "tup", + ), + declaration: LetPattern, + }, }, kind: Let, compiled_case: CompiledCase { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples_no_block.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples_no_block.snap index 3d8590f40d4..3aad4165d5a 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples_no_block.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_tuples_no_block.snap @@ -48,9 +48,12 @@ expression: "\nlet tup = #(#(5, 6))\ntup.0.1\n" }, name: "tup", type_: (), - origin: Variable( - "tup", - ), + origin: VariableOrigin { + syntax: Variable( + "tup", + ), + declaration: LetPattern, + }, }, kind: Let, compiled_case: CompiledCase { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3.snap index 292d3a5a969..ae8127ddde5 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3.snap @@ -1,8 +1,6 @@ --- source: compiler-core/src/parse/tests.rs -assertion_line: 440 expression: "let assert [x] = [2]" -snapshot_kind: text --- [ Assignment( @@ -41,9 +39,12 @@ snapshot_kind: text }, name: "x", type_: (), - origin: Variable( - "x", - ), + origin: VariableOrigin { + syntax: Variable( + "x", + ), + declaration: LetPattern, + }, }, ], tail: None, diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3_and_annotation.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3_and_annotation.snap index a8e6c16d825..fdb1c071fd6 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3_and_annotation.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__with_let_binding3_and_annotation.snap @@ -1,8 +1,6 @@ --- source: compiler-core/src/parse/tests.rs -assertion_line: 445 expression: "let assert [x]: List(Int) = [2]" -snapshot_kind: text --- [ Assignment( @@ -41,9 +39,12 @@ snapshot_kind: text }, name: "x", type_: (), - origin: Variable( - "x", - ), + origin: VariableOrigin { + syntax: Variable( + "x", + ), + declaration: LetPattern, + }, }, ], tail: None, diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs index 31fe97ab2e3..b0b9512bcb4 100644 --- a/compiler-core/src/type_.rs +++ b/compiler-core/src/type_.rs @@ -789,11 +789,9 @@ impl ValueConstructorVariant { #[must_use] pub fn is_generated_variable(&self) -> bool { match self { - ValueConstructorVariant::LocalVariable { - origin: VariableOrigin::Generated, - .. - } => true, - ValueConstructorVariant::LocalVariable { .. } => false, + ValueConstructorVariant::LocalVariable { origin, .. } => { + matches!(origin.syntax, VariableSyntax::Generated) + } ValueConstructorVariant::ModuleConstant { .. } | ValueConstructorVariant::LocalConstant { .. } | ValueConstructorVariant::ModuleFn { .. } diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index f40b2ac93a4..9fb5ff22552 100644 --- a/compiler-core/src/type_/error.rs +++ b/compiler-core/src/type_/error.rs @@ -709,8 +709,15 @@ impl Named { } #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] -/// The origin of a variable. Used to determine how it can be ignored when unused. -pub enum VariableOrigin { +pub struct VariableOrigin { + pub syntax: VariableSyntax, + pub declaration: VariableDeclaration, +} + +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +/// The syntax used to define a variable. Used to determine how it can be ignored +/// when unused. +pub enum VariableSyntax { /// A variable that can be ignored by prefixing with an underscore, `_name` Variable(EcoString), /// A variable from label shorthand syntax, which can be ignored with an underscore: `label: _` @@ -721,17 +728,34 @@ pub enum VariableOrigin { Generated, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +/// The source of a variable, such as a `let` assignment, or function parameter. +pub enum VariableDeclaration { + LetPattern, + UsePattern, + ClausePattern, + FunctionParameter, + Generated, +} + impl VariableOrigin { pub fn how_to_ignore(&self) -> Option { - match self { - VariableOrigin::Variable(name) => { + match &self.syntax { + VariableSyntax::Variable(name) => { Some(format!("You can ignore it with an underscore: `_{name}`.")) } - VariableOrigin::LabelShorthand(label) => Some(format!( + VariableSyntax::LabelShorthand(label) => Some(format!( "You can ignore it with an underscore: `{label}: _`." )), - VariableOrigin::AssignmentPattern => Some("You can safely remove it.".to_string()), - VariableOrigin::Generated => None, + VariableSyntax::AssignmentPattern => Some("You can safely remove it.".to_string()), + VariableSyntax::Generated => None, + } + } + + pub fn generated() -> Self { + Self { + syntax: VariableSyntax::Generated, + declaration: VariableDeclaration::Generated, } } } diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index d7cd9c1e681..e1116ffbeef 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -15,6 +15,7 @@ use crate::{ }, build::Target, exhaustiveness::{self, CompileCaseResult, CompiledCase, Reachability}, + parse::PatternPosition, reference::ReferenceKind, }; use hexpm::version::Version; @@ -1760,6 +1761,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { &self.current_function_definition, &self.hydrator, self.problems, + PatternPosition::LetAssignment, ); let pattern = pattern_typer.infer_single_pattern(pattern, &value); @@ -2111,6 +2113,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { &self.current_function_definition, &self.hydrator, self.problems, + PatternPosition::CaseClause, ); let typed_pattern = pattern_typer.infer_multi_pattern(pattern, subjects, location); @@ -2977,7 +2980,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { location: record_location, name: RECORD_UPDATE_VARIABLE.into(), type_: record_type.clone(), - origin: VariableOrigin::Generated, + origin: VariableOrigin::generated(), }, annotation: None, compiled_case: CompiledCase::failure(), @@ -2993,7 +2996,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { type_: record_type, variant: ValueConstructorVariant::LocalVariable { location: record_location, - origin: VariableOrigin::Generated, + origin: VariableOrigin::generated(), }, }, name: RECORD_UPDATE_VARIABLE.into(), @@ -4266,10 +4269,15 @@ impl<'a, 'b> ExprTyper<'a, 'b> { }); } - let origin = if name == CAPTURE_VARIABLE { - VariableOrigin::Generated + let syntax = if name == CAPTURE_VARIABLE { + VariableSyntax::Generated } else { - VariableOrigin::Variable(name.clone()) + VariableSyntax::Variable(name.clone()) + }; + + let origin = VariableOrigin { + syntax, + declaration: VariableDeclaration::FunctionParameter, }; // Insert a variable for the argument into the environment diff --git a/compiler-core/src/type_/pattern.rs b/compiler-core/src/type_/pattern.rs index 9f3101e8f2c..eaeb2683086 100644 --- a/compiler-core/src/type_/pattern.rs +++ b/compiler-core/src/type_/pattern.rs @@ -12,6 +12,7 @@ use crate::{ ast::{ AssignName, BitArrayOption, ImplicitCallArgOrigin, Layer, UntypedPatternBitArraySegment, }, + parse::PatternPosition, reference::ReferenceKind, type_::expression::FunctionDefinition, }; @@ -45,6 +46,9 @@ pub struct PatternTyper<'a, 'b> { /// keeps track of whether it is in scope, so that we can correctly detect /// valid/invalid uses. variables: HashMap, + + /// What kind of pattern we are typing + position: PatternPosition, } #[derive(Debug)] @@ -96,6 +100,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> { current_function: &'a FunctionDefinition, hydrator: &'a Hydrator, problems: &'a mut Problems, + position: PatternPosition, ) -> Self { Self { environment, @@ -109,6 +114,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> { problems, error_encountered: false, variables: HashMap::new(), + position, } } @@ -209,11 +215,20 @@ impl<'a, 'b> PatternTyper<'a, 'b> { let _ = self .inferred_variant_variables .insert(name.clone(), variant_index); + + let origin = match &variable.variant { + ValueConstructorVariant::LocalVariable { origin, .. } => origin.clone(), + ValueConstructorVariant::ModuleConstant { .. } + | ValueConstructorVariant::LocalConstant { .. } + | ValueConstructorVariant::ModuleFn { .. } + | ValueConstructorVariant::Record { .. } => VariableOrigin::generated(), + }; + // This variable is only inferred in this branch of the case expression self.environment.insert_local_variable( name.clone(), variable.definition_location().span, - VariableOrigin::Variable(name), + origin, type_, ); } @@ -660,7 +675,10 @@ impl<'a, 'b> PatternTyper<'a, 'b> { left, string(), *left_location, - VariableOrigin::AssignmentPattern, + VariableOrigin { + syntax: VariableSyntax::AssignmentPattern, + declaration: self.position.to_declaration(), + }, ); } @@ -671,7 +689,10 @@ impl<'a, 'b> PatternTyper<'a, 'b> { right, string(), right_location, - VariableOrigin::Variable(right.clone()), + VariableOrigin { + syntax: VariableSyntax::Variable(right.clone()), + declaration: self.position.to_declaration(), + }, ); } AssignName::Discard(_) => { @@ -701,7 +722,10 @@ impl<'a, 'b> PatternTyper<'a, 'b> { &name, pattern.type_().clone(), location, - VariableOrigin::AssignmentPattern, + VariableOrigin { + syntax: VariableSyntax::AssignmentPattern, + declaration: self.position.to_declaration(), + }, ); Pattern::Assign { name, diff --git a/compiler-core/src/type_/pipe.rs b/compiler-core/src/type_/pipe.rs index 07e8329e2f9..c288bae6d85 100644 --- a/compiler-core/src/type_/pipe.rs +++ b/compiler-core/src/type_/pipe.rs @@ -252,7 +252,7 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> { name: PIPE_VARIABLE.into(), constructor: ValueConstructor::local_variable( self.argument_location, - VariableOrigin::Generated, + VariableOrigin::generated(), self.argument_type.clone(), ), } @@ -443,7 +443,7 @@ fn new_pipeline_assignment( expr_typer.environment.insert_local_variable( PIPE_VARIABLE.into(), location, - VariableOrigin::Generated, + VariableOrigin::generated(), expression.type_(), ); TypedPipelineAssignment {