Skip to content

Commit

Permalink
Fix bug extracting module selects
Browse files Browse the repository at this point in the history
  • Loading branch information
giacomocavalieri committed Dec 22, 2024
1 parent 86952bf commit 03395ed
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
26 changes: 10 additions & 16 deletions compiler-core/src/language_server/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2711,8 +2711,16 @@ impl<'ast> ast::visit::Visit<'ast> for ExtractVariable<'ast> {
if self.position != Some(ExtractVariablePosition::TopLevelStatement)
&& within(self.params.range, expr_range)
{
self.selected_expression = Some(expr_location);
self.statement_before_selected_expression = self.latest_statement;
match expr {
// We don't extract variables, they're already good.
// And we don't extract module selects by themselves but always
// want to consider those as part of a function call.
TypedExpr::Var { .. } | TypedExpr::ModuleSelect { .. } => (),
_ => {
self.selected_expression = Some(expr_location);
self.statement_before_selected_expression = self.latest_statement;
}
}
}

let previous_position = self.position;
Expand Down Expand Up @@ -2744,20 +2752,6 @@ impl<'ast> ast::visit::Visit<'ast> for ExtractVariable<'ast> {
self.position = previous_position;
}

// We don't want to offer the action if the cursor is over a variable
// already!
fn visit_typed_expr_var(
&mut self,
location: &'ast SrcSpan,
_constructor: &'ast type_::ValueConstructor,
_name: &'ast EcoString,
) {
let var_range = self.edits.src_span_to_lsp_range(*location);
if within(self.params.range, var_range) {
self.selected_expression = None;
}
}

// We don't want to offer the action if the cursor is over some invalid
// piece of code.
fn visit_typed_expr_invalid(&mut self, location: &'ast SrcSpan, _type_: &'ast Arc<Type>) {
Expand Down
15 changes: 15 additions & 0 deletions compiler-core/src/language_server/tests/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3985,6 +3985,21 @@ fn do_not_extract_top_level_expression_in_let_statement() {
);
}

#[test]
fn do_not_extract_top_level_module_call() {
let src = r#"
import list
pub fn main() {
list.map([1, 2, 3], todo)
}"#;

assert_no_code_actions!(
EXTRACT_VARIABLE,
TestProject::for_source(src).add_module("list", "pub fn map(l, f) { todo }"),
find_position_of("map").to_selection()
);
}

#[test]
fn expand_function_capture() {
assert_code_action!(
Expand Down

0 comments on commit 03395ed

Please sign in to comment.