From 5cf51e1433a919d08cb991f7c667005869588c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Fri, 8 Aug 2025 23:29:53 +0200 Subject: [PATCH 01/17] assists/inline: add negative test for inherit --- crates/ide/src/ide/assists/inline.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index ba91dcf..9650a92 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -144,4 +144,22 @@ mod tests { expect!["rec { foo = 1; bar = 1; baz = 1; }"], ); } + + #[test] + fn no_inherit() { + check_no( + r#" +{ + outputs = { + self, + nixpkgs, + flake-utils, + }: { + inherit $0flake-utils; + }; +} + "#, + ); + check_no("let $0foo = 1; in { inherit foo; }"); + } } From 4bef45187d849caa3c72c5c250a22ca2379b3386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 00:36:33 +0200 Subject: [PATCH 02/17] assists/inline: add negative test for patfield --- crates/ide/src/ide/assists/inline.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 9650a92..b858b8f 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -162,4 +162,21 @@ mod tests { ); check_no("let $0foo = 1; in { inherit foo; }"); } + + #[test] + fn no_patfield() { + check_no( + r#" +{ + outputs = { + self, + nixpkgs, + $0flake-utils, + }: { + inherit flake-utils; + }; +} + "#, + ); + } } From 5a5e594193986909229c6c48b20801a8debe8856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 00:11:21 +0200 Subject: [PATCH 03/17] assists/inline: don't assist for inherit and patfield --- crates/ide/src/ide/assists/inline.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index b858b8f..661c81b 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -7,7 +7,6 @@ use syntax::{ast, SyntaxNode}; pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let file_id = ctx.frange.file_id; - let parse = ctx.db.parse(file_id); let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); @@ -24,7 +23,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { // Only provide assist when there is only one node // i.e. `let a.b = 1; a.c = 2; in a` is not supported if let [ptr] = nodes.as_slice() { - ptr.to_node(&parse.syntax_node()) + ptr.to_node(ctx.ast.syntax()) .ancestors() .flat_map(ast::AttrpathValue::cast) .find_map(|path_value| path_value.value())? @@ -38,6 +37,16 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { insert: maybe_parenthesize(&definition, usage.syntax()), }); } else if let Some(definition) = ctx.covering_node::() { + let parent = definition.syntax().parent(); + if + // `foo` in { inherit `foo`; } is considered as an attr. + parent.clone().and_then(ast::Inherit::cast).is_some() + // `foo` in `{ foo }: …` is considered as an attr. PatField is a bind site. + || parent.and_then(ast::PatField::cast).is_some() + { + return None; + } + let ptr = AstPtr::new(definition.syntax()); let name_id = source_map.name_for_node(ptr)?; let path_value = definition @@ -60,7 +69,12 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { .map(|ptr| ptr.to_node(ctx.ast.syntax())), _ => None, }) + // Don't consider inherit usages for now + .filter(|usage| usage.parent().and_then(ast::Inherit::cast).is_none()) .collect::>(); + if usages.is_empty() { + return None; + } let is_letin = ast::LetIn::cast(path_value.syntax().parent()?).is_some(); if is_letin { From cf98b02fc12149cb8b165ad166ca484b303c7a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 09:27:36 +0200 Subject: [PATCH 04/17] assists/inline: refactor test cases --- crates/ide/src/ide/assists/inline.rs | 33 +++++----------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 661c81b..b6b92d2 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -161,36 +161,15 @@ mod tests { #[test] fn no_inherit() { - check_no( - r#" -{ - outputs = { - self, - nixpkgs, - flake-utils, - }: { - inherit $0flake-utils; - }; -} - "#, - ); - check_no("let $0foo = 1; in { inherit foo; }"); + check_no("{ outputs = { nixpkgs, ... }: { inherit $0nixpkgs; }; }"); + check("let $0foo = 1; in { inherit foo; }", expect!["let in { foo = 1; }"]); + check_no("with lib; let inherit (lib) $0foo; in foo"); + check_no("with lib; let inherit (lib) foo; in $0foo"); } #[test] fn no_patfield() { - check_no( - r#" -{ - outputs = { - self, - nixpkgs, - $0flake-utils, - }: { - inherit flake-utils; - }; -} - "#, - ); + check_no("{ outputs = { $0nixpkgs, ... }: { inherit nixpkgs; }; }"); + check_no("{ outputs = { $0nixpkgs, ... }: nixpkgs; }"); } } From 453921df042e55dbf7b9aa8b3542a42e15bbeaa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 09:28:06 +0200 Subject: [PATCH 05/17] assists/inline: handle inherits when inlining from definition --- crates/ide/src/ide/assists/inline.rs | 54 +++++++++++++++++----------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index b6b92d2..bc4db76 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -3,7 +3,7 @@ use crate::def::{AstPtr, ResolveResult}; use crate::TextEdit; use smol_str::{SmolStr, ToSmolStr}; use syntax::ast::AstNode; -use syntax::{ast, SyntaxNode}; +use syntax::{ast, SyntaxNode, TextRange}; pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let file_id = ctx.frange.file_id; @@ -61,20 +61,12 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let definition = path_value.value()?; - let usages = name_res - .iter() - .filter_map(|(id, res)| match res { - &ResolveResult::Definition(def) if def == name_id => source_map - .node_for_expr(id) - .map(|ptr| ptr.to_node(ctx.ast.syntax())), - _ => None, - }) - // Don't consider inherit usages for now - .filter(|usage| usage.parent().and_then(ast::Inherit::cast).is_none()) - .collect::>(); - if usages.is_empty() { - return None; - } + let usages = name_res.iter().filter_map(|(id, res)| match res { + &ResolveResult::Definition(def) if def == name_id => source_map + .node_for_expr(id) + .map(|ptr| ptr.to_node(ctx.ast.syntax())), + _ => None, + }); let is_letin = ast::LetIn::cast(path_value.syntax().parent()?).is_some(); if is_letin { @@ -85,10 +77,29 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { }; for usage in usages { - rewrites.push(TextEdit { - delete: usage.text_range(), - insert: maybe_parenthesize(&definition, &usage), - }); + let parent = usage.parent(); + if let Some(inherit) = parent.and_then(ast::Inherit::cast) { + // Delete one inherit field + rewrites.push(TextEdit { + delete: usage.text_range(), + insert: "".into(), + }); + + // Insert new binding right after + rewrites.push(TextEdit { + delete: TextRange::new( + inherit.syntax().text_range().end(), + inherit.syntax().text_range().end(), + ), + // Unambiguous and never need parens + insert: format!(" {} = {};", usage.text(), definition.syntax().text()).into(), + }); + } else { + rewrites.push(TextEdit { + delete: usage.text_range(), + insert: maybe_parenthesize(&definition, &usage), + }); + } } } else { return None; @@ -162,7 +173,10 @@ mod tests { #[test] fn no_inherit() { check_no("{ outputs = { nixpkgs, ... }: { inherit $0nixpkgs; }; }"); - check("let $0foo = 1; in { inherit foo; }", expect!["let in { foo = 1; }"]); + check( + "let $0foo = 1; in { inherit foo; }", + expect!["let in { foo = 1; }"], + ); check_no("with lib; let inherit (lib) $0foo; in foo"); check_no("with lib; let inherit (lib) foo; in $0foo"); } From 1287e193fb7761541b5a851298b13c8de22275c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 10:22:07 +0200 Subject: [PATCH 06/17] assists/inline: test multi-attr inherit inline --- crates/ide/src/ide/assists/inline.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index bc4db76..76283dd 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -171,18 +171,40 @@ mod tests { } #[test] - fn no_inherit() { - check_no("{ outputs = { nixpkgs, ... }: { inherit $0nixpkgs; }; }"); + fn allow_inherit_usage_def() { + // inherit in usage from definition check( "let $0foo = 1; in { inherit foo; }", - expect!["let in { foo = 1; }"], + expect!["let in { inherit ; foo = 1; }"], + ); + check( + "let $0foo = 1; bar = 2; in { inherit foo bar; }", + expect!["let bar = 2; in { inherit bar; foo = 1; }"], + ); + } + + #[test] + fn allow_inherit_usage_ref() { + // inherit in usage from definition + check( + "let foo = 1; in { inherit $0foo; }", + expect!["let foo = 1; in { inherit ; foo = 1; }"], + ); + check( + "let foo = 1; bar = 2; in { inherit $0foo bar; }", + expect!["let foo = 1; bar = 2; in { inherit bar; foo = 1; }"], ); + } + + #[test] + fn no_inherit_definition() { check_no("with lib; let inherit (lib) $0foo; in foo"); check_no("with lib; let inherit (lib) foo; in $0foo"); } #[test] fn no_patfield() { + check_no("{ outputs = { nixpkgs, ... }: { inherit $0nixpkgs; }; }"); check_no("{ outputs = { $0nixpkgs, ... }: { inherit nixpkgs; }; }"); check_no("{ outputs = { $0nixpkgs, ... }: nixpkgs; }"); } From 6ef168641039bfecf44b0e05a838f98ed3f1b453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 10:57:59 +0200 Subject: [PATCH 07/17] assists/inline: handle inherits when inlining from reference --- crates/ide/src/ide/assists/inline.rs | 170 +++++++++++++++------------ 1 file changed, 92 insertions(+), 78 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 76283dd..9722b3c 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -1,8 +1,9 @@ use super::{AssistKind, AssistsCtx}; -use crate::def::{AstPtr, ResolveResult}; +use crate::def::{AstPtr, Name, ResolveResult}; use crate::TextEdit; +use la_arena::Idx; use smol_str::{SmolStr, ToSmolStr}; -use syntax::ast::AstNode; +use syntax::ast::{AstNode, Expr}; use syntax::{ast, SyntaxNode, TextRange}; pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { @@ -10,95 +11,81 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); - let mut rewrites: Vec = vec![]; + let definition_of = |name: Idx| -> Option { + let nodes = source_map.nodes_for_name(name).collect::>(); + // Only provide assist when there is only one node + // i.e. `let a.b = 1; a.c = 2; in a` is not supported + if let [ptr] = nodes.as_slice() { + ptr.to_node(ctx.ast.syntax()) + .parent()? + .parent() + .and_then(ast::AttrpathValue::cast) + .and_then(|path_value| path_value.value()) + } else { + return None; + } + }; + let mut rewrites: Vec = vec![]; if let Some(usage) = ctx.covering_node::() { let ptr = AstPtr::new(usage.syntax()); - let expr_id = source_map.expr_for_node(ptr)?; - let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { + let expr = source_map.expr_for_node(ptr)?; + let &ResolveResult::Definition(name) = name_res.get(expr)? else { return None; }; - let definition = { - let nodes = source_map.nodes_for_name(name).collect::>(); - // Only provide assist when there is only one node - // i.e. `let a.b = 1; a.c = 2; in a` is not supported - if let [ptr] = nodes.as_slice() { - ptr.to_node(ctx.ast.syntax()) - .ancestors() - .flat_map(ast::AttrpathValue::cast) - .find_map(|path_value| path_value.value())? - } else { - return None; - } - }; - - rewrites.push(TextEdit { - delete: usage.syntax().text_range(), - insert: maybe_parenthesize(&definition, usage.syntax()), - }); - } else if let Some(definition) = ctx.covering_node::() { - let parent = definition.syntax().parent(); - if - // `foo` in { inherit `foo`; } is considered as an attr. - parent.clone().and_then(ast::Inherit::cast).is_some() - // `foo` in `{ foo }: …` is considered as an attr. PatField is a bind site. - || parent.and_then(ast::PatField::cast).is_some() - { + let definition = definition_of(name)?; + replace_usage(&mut rewrites, &definition, usage.syntax()); + } else if let Some(attr) = ctx.covering_node::() { + let attr_syntax = attr.syntax(); + + // `foo` in `{ foo }: …` is considered as an attr. + // PatField is a bind site and doesn't make sense here. + if attr_syntax.parent().and_then(ast::PatField::cast).is_some() { return None; } - let ptr = AstPtr::new(definition.syntax()); - let name_id = source_map.name_for_node(ptr)?; - let path_value = definition - .syntax() - .ancestors() - .find_map(ast::AttrpathValue::cast)?; - - // Don't provide assist when there are more than one attrname - if path_value.attrpath()?.attrs().count() > 1 { - return None; - }; - - let definition = path_value.value()?; - - let usages = name_res.iter().filter_map(|(id, res)| match res { - &ResolveResult::Definition(def) if def == name_id => source_map - .node_for_expr(id) - .map(|ptr| ptr.to_node(ctx.ast.syntax())), - _ => None, - }); - - let is_letin = ast::LetIn::cast(path_value.syntax().parent()?).is_some(); - if is_letin { - rewrites.push(TextEdit { - delete: path_value.syntax().text_range(), - insert: Default::default(), + // `foo` in { inherit `foo`; } is considered as an attr. + if attr_syntax.parent().and_then(ast::Inherit::cast).is_some() { + // Attr is an usage here + let ptr = AstPtr::new(attr.syntax()); + let expr_id = source_map.expr_for_node(ptr)?; + let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { + return None; + }; + let definition = definition_of(name)?; + replace_usage(&mut rewrites, &definition, attr.syntax()); + } else { + // attr is a definition here + let ptr = AstPtr::new(attr.syntax()); + let name = source_map.name_for_node(ptr)?; + let path_value = attr + .syntax() + .ancestors() + .find_map(ast::AttrpathValue::cast)?; + + // Don't provide assist when there are more than one attrname + if path_value.attrpath()?.attrs().count() > 1 { + return None; + }; + + let definition = path_value.value()?; + let usages = name_res.iter().filter_map(|(id, res)| match res { + &ResolveResult::Definition(def) if def == name => source_map + .node_for_expr(id) + .map(|ptr| ptr.to_node(ctx.ast.syntax())), + _ => None, }); - }; - for usage in usages { - let parent = usage.parent(); - if let Some(inherit) = parent.and_then(ast::Inherit::cast) { - // Delete one inherit field + let is_letin = ast::LetIn::cast(path_value.syntax().parent()?).is_some(); + if is_letin { rewrites.push(TextEdit { - delete: usage.text_range(), - insert: "".into(), + delete: path_value.syntax().text_range(), + insert: Default::default(), }); + }; - // Insert new binding right after - rewrites.push(TextEdit { - delete: TextRange::new( - inherit.syntax().text_range().end(), - inherit.syntax().text_range().end(), - ), - // Unambiguous and never need parens - insert: format!(" {} = {};", usage.text(), definition.syntax().text()).into(), - }); - } else { - rewrites.push(TextEdit { - delete: usage.text_range(), - insert: maybe_parenthesize(&definition, &usage), - }); + for usage in usages { + replace_usage(&mut rewrites, &definition, &usage); } } } else { @@ -126,6 +113,33 @@ fn maybe_parenthesize(replacement: &ast::Expr, original: &SyntaxNode) -> SmolStr } } +// Replace an usage of a binding +fn replace_usage(rewrites: &mut Vec, replacement: &ast::Expr, original: &SyntaxNode) { + let parent = original.parent(); + if let Some(inherit) = parent.and_then(ast::Inherit::cast) { + // Delete one inherit field + rewrites.push(TextEdit { + delete: original.text_range(), + insert: Default::default(), + }); + + // Insert new binding right after + rewrites.push(TextEdit { + delete: TextRange::new( + inherit.syntax().text_range().end(), + inherit.syntax().text_range().end(), + ), + // Unambiguous and never need parens + insert: format!(" {} = {};", original.text(), replacement.syntax().text()).into(), + }); + } else { + rewrites.push(TextEdit { + delete: original.text_range(), + insert: maybe_parenthesize(&replacement, &original), + }); + } +} + #[cfg(test)] mod tests { use expect_test::expect; From ea63c21d27da3a7d349551400525eb4f093d3e70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 11:18:44 +0200 Subject: [PATCH 08/17] assists/inline: refactor tests --- crates/ide/src/ide/assists/inline.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 9722b3c..c7a4cd7 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -147,10 +147,7 @@ mod tests { #[test] fn let_in_ref() { - check( - r#"let a = "foo"; in $0a"#, - expect![r#"let a = "foo"; in "foo""#], - ); + check("let a = 1; in $0a", expect!["let a = 1; in 1"]); check( "let a = x: x; in $0a 1", expect!["let a = x: x; in (x: x) 1"], @@ -164,12 +161,13 @@ mod tests { #[test] fn no_let_in_multi() { - check_no(r#"let a.b = "foo"; a.c = "bar"; in $0a"#); - check_no(r#"let a.b$0 = "foo"; a.c = "bar"; in a"#); + check_no("let a.b = 1; a.c = 2; in $0a"); + check_no("let a.b$0 = 1; a.c = 2; in a"); + check_no("let $0a.b = 1; a.c = 2; in a"); } #[test] - fn attr_ref() { + fn rec_attr_ref() { check( "rec { foo = 1; bar = $0foo; }", expect!["rec { foo = 1; bar = 1; }"], @@ -177,7 +175,7 @@ mod tests { } #[test] - fn attr_def() { + fn rec_attr_def() { check( "rec { $0foo = 1; bar = foo; baz = foo; }", expect!["rec { foo = 1; bar = 1; baz = 1; }"], @@ -186,7 +184,6 @@ mod tests { #[test] fn allow_inherit_usage_def() { - // inherit in usage from definition check( "let $0foo = 1; in { inherit foo; }", expect!["let in { inherit ; foo = 1; }"], @@ -199,7 +196,6 @@ mod tests { #[test] fn allow_inherit_usage_ref() { - // inherit in usage from definition check( "let foo = 1; in { inherit $0foo; }", expect!["let foo = 1; in { inherit ; foo = 1; }"], @@ -211,9 +207,9 @@ mod tests { } #[test] - fn no_inherit_definition() { - check_no("with lib; let inherit (lib) $0foo; in foo"); - check_no("with lib; let inherit (lib) foo; in $0foo"); + fn no_allow_inherit_as_def() { + check_no("let inherit (lib) $0foo; in foo"); + check_no("let inherit (lib) foo; in $0foo"); } #[test] From 6a244b62d4f616ef184a7c8e50aa2e34130cb258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 11:18:53 +0200 Subject: [PATCH 09/17] assists/inline: constistent checking of definition --- crates/ide/src/ide/assists/inline.rs | 35 +++++++++++++++------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index c7a4cd7..6da5a28 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -11,7 +11,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); - let definition_of = |name: Idx| -> Option { + let definition_of = |name: Idx| -> Option<(Expr, ast::AttrpathValue)> { let nodes = source_map.nodes_for_name(name).collect::>(); // Only provide assist when there is only one node // i.e. `let a.b = 1; a.c = 2; in a` is not supported @@ -19,10 +19,12 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { ptr.to_node(ctx.ast.syntax()) .parent()? .parent() - .and_then(ast::AttrpathValue::cast) - .and_then(|path_value| path_value.value()) + .and_then(|node| { + let path_value = ast::AttrpathValue::cast(node)?; + Some((path_value.value()?, path_value)) + }) } else { - return None; + None } }; @@ -33,42 +35,44 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr)? else { return None; }; - let definition = definition_of(name)?; + let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { let attr_syntax = attr.syntax(); // `foo` in `{ foo }: …` is considered as an attr. - // PatField is a bind site and doesn't make sense here. + // PatField is a bind site without definition so doesn't make sense here. if attr_syntax.parent().and_then(ast::PatField::cast).is_some() { return None; } // `foo` in { inherit `foo`; } is considered as an attr. - if attr_syntax.parent().and_then(ast::Inherit::cast).is_some() { + if attr_syntax.parent().and_then(ast::Inherit::cast).is_some() + && attr_syntax + .parent()? + .parent() + .and_then(ast::LetIn::cast) + .is_none() + { // Attr is an usage here let ptr = AstPtr::new(attr.syntax()); let expr_id = source_map.expr_for_node(ptr)?; let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { return None; }; - let definition = definition_of(name)?; + let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, attr.syntax()); } else { // attr is a definition here let ptr = AstPtr::new(attr.syntax()); let name = source_map.name_for_node(ptr)?; - let path_value = attr - .syntax() - .ancestors() - .find_map(ast::AttrpathValue::cast)?; + let (definition, path_value) = definition_of(name)?; // Don't provide assist when there are more than one attrname if path_value.attrpath()?.attrs().count() > 1 { return None; }; - let definition = path_value.value()?; let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map .node_for_expr(id) @@ -115,8 +119,7 @@ fn maybe_parenthesize(replacement: &ast::Expr, original: &SyntaxNode) -> SmolStr // Replace an usage of a binding fn replace_usage(rewrites: &mut Vec, replacement: &ast::Expr, original: &SyntaxNode) { - let parent = original.parent(); - if let Some(inherit) = parent.and_then(ast::Inherit::cast) { + if let Some(inherit) = original.parent().and_then(ast::Inherit::cast) { // Delete one inherit field rewrites.push(TextEdit { delete: original.text_range(), @@ -135,7 +138,7 @@ fn replace_usage(rewrites: &mut Vec, replacement: &ast::Expr, original } else { rewrites.push(TextEdit { delete: original.text_range(), - insert: maybe_parenthesize(&replacement, &original), + insert: maybe_parenthesize(replacement, original), }); } } From d86634aaec6e3149d9211134264993b628308d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 14:40:12 +0200 Subject: [PATCH 10/17] assists/inline: refactor --- crates/ide/src/ide/assists/inline.rs | 37 ++++++++++------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 6da5a28..e5ab50e 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -12,20 +12,17 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let source_map = ctx.db.source_map(file_id); let definition_of = |name: Idx| -> Option<(Expr, ast::AttrpathValue)> { - let nodes = source_map.nodes_for_name(name).collect::>(); + let defs = source_map.nodes_for_name(name).collect::>(); + let ptr = defs.first()?; + let grandparent = ptr.to_node(ctx.ast.syntax()).parent()?.parent()?; + let path_value = ast::AttrpathValue::cast(grandparent)?; + // Only provide assist when there is only one node // i.e. `let a.b = 1; a.c = 2; in a` is not supported - if let [ptr] = nodes.as_slice() { - ptr.to_node(ctx.ast.syntax()) - .parent()? - .parent() - .and_then(|node| { - let path_value = ast::AttrpathValue::cast(node)?; - Some((path_value.value()?, path_value)) - }) - } else { - None - } + if path_value.attrpath()?.attrs().count() > 1 { + return None; + }; + Some((path_value.value()?, path_value)) }; let mut rewrites: Vec = vec![]; @@ -38,21 +35,17 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { - let attr_syntax = attr.syntax(); + let parent = attr.syntax().parent()?; // `foo` in `{ foo }: …` is considered as an attr. // PatField is a bind site without definition so doesn't make sense here. - if attr_syntax.parent().and_then(ast::PatField::cast).is_some() { + if ast::PatField::cast(parent.clone()).is_some() { return None; } // `foo` in { inherit `foo`; } is considered as an attr. - if attr_syntax.parent().and_then(ast::Inherit::cast).is_some() - && attr_syntax - .parent()? - .parent() - .and_then(ast::LetIn::cast) - .is_none() + if ast::Inherit::cast(parent.clone()).is_some() + && parent.parent().and_then(ast::LetIn::cast).is_none() { // Attr is an usage here let ptr = AstPtr::new(attr.syntax()); @@ -68,10 +61,6 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name = source_map.name_for_node(ptr)?; let (definition, path_value) = definition_of(name)?; - // Don't provide assist when there are more than one attrname - if path_value.attrpath()?.attrs().count() > 1 { - return None; - }; let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map From 763b72cf95286e95041a4617f71e69e53cd128c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 9 Aug 2025 23:53:41 +0200 Subject: [PATCH 11/17] assists/inline: remove let-in check early return --- crates/ide/src/ide/assists/inline.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index e5ab50e..c47aeca 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -69,7 +69,11 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { _ => None, }); - let is_letin = ast::LetIn::cast(path_value.syntax().parent()?).is_some(); + let is_letin = path_value + .syntax() + .parent() + .and_then(ast::LetIn::cast) + .is_some(); if is_letin { rewrites.push(TextEdit { delete: path_value.syntax().text_range(), From dee98e4f3b796ea47ee1829b35f40070b4c46c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Mon, 11 Aug 2025 20:20:04 +0200 Subject: [PATCH 12/17] assists/inline: don't publish assist for non rec attrset --- crates/ide/src/ide/assists/inline.rs | 29 ++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index c47aeca..2a8fca5 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -35,7 +35,11 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { - let parent = attr.syntax().parent()?; + let (parent, parent2, parent3) = { + let mut it = attr.syntax().ancestors(); + it.next(); // drop self + (it.next()?, it.next(), it.next()) + }; // `foo` in `{ foo }: …` is considered as an attr. // PatField is a bind site without definition so doesn't make sense here. @@ -45,7 +49,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { // `foo` in { inherit `foo`; } is considered as an attr. if ast::Inherit::cast(parent.clone()).is_some() - && parent.parent().and_then(ast::LetIn::cast).is_none() + && parent2.clone().and_then(ast::AttrSet::cast).is_some() { // Attr is an usage here let ptr = AstPtr::new(attr.syntax()); @@ -55,12 +59,22 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { }; let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, attr.syntax()); - } else { + } else if let Some(path_value) = parent2.clone().and_then(ast::AttrpathValue::cast) { + // Only handle let in or recursive attribute set, a plain attribute set doesn't create a binding. + if parent3.clone().and_then(ast::LetIn::cast).is_none() + && parent3 + .clone() + .and_then(|x| ast::AttrSet::cast(x)?.rec_token()) + .is_none() + { + return None; + } + // attr is a definition here let ptr = AstPtr::new(attr.syntax()); let name = source_map.name_for_node(ptr)?; - let (definition, path_value) = definition_of(name)?; + let (definition, _) = definition_of(name)?; let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map @@ -84,6 +98,8 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { for usage in usages { replace_usage(&mut rewrites, &definition, &usage); } + } else { + return None; } } else { return None; @@ -214,4 +230,9 @@ mod tests { check_no("{ outputs = { $0nixpkgs, ... }: { inherit nixpkgs; }; }"); check_no("{ outputs = { $0nixpkgs, ... }: nixpkgs; }"); } + + #[test] + fn no_attr() { + check_no("{ $0foo = 1; }"); + } } From 436b66472ce2e869725fedb01d4f43263f240af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Mon, 11 Aug 2025 20:21:47 +0200 Subject: [PATCH 13/17] assists/inline: simplify helper function --- crates/ide/src/ide/assists/inline.rs | 31 +++++++++++----------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 2a8fca5..7c13596 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -11,7 +11,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); - let definition_of = |name: Idx| -> Option<(Expr, ast::AttrpathValue)> { + let definition_of = |name: Idx| -> Option { let defs = source_map.nodes_for_name(name).collect::>(); let ptr = defs.first()?; let grandparent = ptr.to_node(ctx.ast.syntax()).parent()?.parent()?; @@ -22,7 +22,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { if path_value.attrpath()?.attrs().count() > 1 { return None; }; - Some((path_value.value()?, path_value)) + path_value.value() }; let mut rewrites: Vec = vec![]; @@ -32,7 +32,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr)? else { return None; }; - let (definition, _) = definition_of(name)?; + let definition = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { let (parent, parent2, parent3) = { @@ -57,25 +57,24 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { return None; }; - let (definition, _) = definition_of(name)?; + let definition = definition_of(name)?; replace_usage(&mut rewrites, &definition, attr.syntax()); } else if let Some(path_value) = parent2.clone().and_then(ast::AttrpathValue::cast) { + let is_letin = parent3.clone().and_then(ast::LetIn::cast).is_some(); + let is_rec_attr = parent3 + .clone() + .and_then(|x| ast::AttrSet::cast(x)?.rec_token()) + .is_some(); + // Only handle let in or recursive attribute set, a plain attribute set doesn't create a binding. - if parent3.clone().and_then(ast::LetIn::cast).is_none() - && parent3 - .clone() - .and_then(|x| ast::AttrSet::cast(x)?.rec_token()) - .is_none() - { + if !is_letin && !is_rec_attr { return None; } // attr is a definition here let ptr = AstPtr::new(attr.syntax()); let name = source_map.name_for_node(ptr)?; - - let (definition, _) = definition_of(name)?; - + let definition = definition_of(name)?; let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map .node_for_expr(id) @@ -83,18 +82,12 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { _ => None, }); - let is_letin = path_value - .syntax() - .parent() - .and_then(ast::LetIn::cast) - .is_some(); if is_letin { rewrites.push(TextEdit { delete: path_value.syntax().text_range(), insert: Default::default(), }); }; - for usage in usages { replace_usage(&mut rewrites, &definition, &usage); } From e806846d52bfc500cbfe87cba755304804bb6d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Thu, 14 Aug 2025 12:15:55 +0200 Subject: [PATCH 14/17] assists/inline: refactor and allow let inherit in --- crates/ide/src/ide/assists/inline.rs | 43 ++++++++++++---------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 7c13596..dd92a0f 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -11,7 +11,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); - let definition_of = |name: Idx| -> Option { + let definition_of = |name: Idx| -> Option<(Expr, ast::AttrpathValue)> { let defs = source_map.nodes_for_name(name).collect::>(); let ptr = defs.first()?; let grandparent = ptr.to_node(ctx.ast.syntax()).parent()?.parent()?; @@ -22,7 +22,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { if path_value.attrpath()?.attrs().count() > 1 { return None; }; - path_value.value() + Some((path_value.value()?, path_value)) }; let mut rewrites: Vec = vec![]; @@ -32,49 +32,44 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr)? else { return None; }; - let definition = definition_of(name)?; + let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { - let (parent, parent2, parent3) = { + let (parent, _, parent3) = { let mut it = attr.syntax().ancestors(); it.next(); // drop self (it.next()?, it.next(), it.next()) }; + let is_patfield = ast::PatField::cast(parent.clone()).is_some(); + let is_inherit = ast::Inherit::cast(parent.clone()).is_some(); + let is_in_letin = parent3.clone().and_then(ast::LetIn::cast).is_some(); + let is_in_recattr = parent3 + .clone() + .and_then(|x| ast::AttrSet::cast(x)?.rec_token()) + .is_some(); + // `foo` in `{ foo }: …` is considered as an attr. // PatField is a bind site without definition so doesn't make sense here. - if ast::PatField::cast(parent.clone()).is_some() { + if is_patfield { return None; } - // `foo` in { inherit `foo`; } is considered as an attr. - if ast::Inherit::cast(parent.clone()).is_some() - && parent2.clone().and_then(ast::AttrSet::cast).is_some() - { + // `foo` in `{ inherit foo; }` or `let inherit foo; in` is considered as an attr. + if is_inherit { // Attr is an usage here let ptr = AstPtr::new(attr.syntax()); let expr_id = source_map.expr_for_node(ptr)?; let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { return None; }; - let definition = definition_of(name)?; + let (definition, _) = definition_of(name)?; replace_usage(&mut rewrites, &definition, attr.syntax()); - } else if let Some(path_value) = parent2.clone().and_then(ast::AttrpathValue::cast) { - let is_letin = parent3.clone().and_then(ast::LetIn::cast).is_some(); - let is_rec_attr = parent3 - .clone() - .and_then(|x| ast::AttrSet::cast(x)?.rec_token()) - .is_some(); - - // Only handle let in or recursive attribute set, a plain attribute set doesn't create a binding. - if !is_letin && !is_rec_attr { - return None; - } - + } else if is_in_letin || is_in_recattr { // attr is a definition here let ptr = AstPtr::new(attr.syntax()); let name = source_map.name_for_node(ptr)?; - let definition = definition_of(name)?; + let (definition, path_value) = definition_of(name)?; let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map .node_for_expr(id) @@ -82,7 +77,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { _ => None, }); - if is_letin { + if is_in_letin { rewrites.push(TextEdit { delete: path_value.syntax().text_range(), insert: Default::default(), From b422777194eed2f738a9fcd89bf5e26c9f846afd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Thu, 14 Aug 2025 12:19:36 +0200 Subject: [PATCH 15/17] assists/inline: add tests for let inherit in --- crates/ide/src/ide/assists/inline.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index dd92a0f..e0dbc0f 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -192,6 +192,10 @@ mod tests { "let $0foo = 1; bar = 2; in { inherit foo bar; }", expect!["let bar = 2; in { inherit bar; foo = 1; }"], ); + check( + "let $0foo = 1; bar = 2; in let inherit foo bar; in null", + expect!["let bar = 2; in let inherit bar; foo = 1; in null"], + ); } #[test] @@ -204,6 +208,10 @@ mod tests { "let foo = 1; bar = 2; in { inherit $0foo bar; }", expect!["let foo = 1; bar = 2; in { inherit bar; foo = 1; }"], ); + check( + "let foo = 1; bar = 2; in let inherit $0foo bar; in null", + expect!["let foo = 1; bar = 2; in let inherit bar; foo = 1; in null"], + ); } #[test] From 1e0083491507180d7c492f9e3d29e7db5c92cba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sun, 31 Aug 2025 20:33:54 +0800 Subject: [PATCH 16/17] assists/inline: don't assist when binding is recursive This forbids the case of `f: let x = f x; in x' where removing the binding would make the rewrite semantically incorrect. assists/inline: make check ast lookup is recursive --- crates/ide/src/ide/assists/inline.rs | 77 ++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index e0dbc0f..8f39576 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -1,6 +1,8 @@ +use std::sync::Arc; + use super::{AssistKind, AssistsCtx}; -use crate::def::{AstPtr, Name, ResolveResult}; -use crate::TextEdit; +use crate::def::{AstPtr, Name, NameResolution, ResolveResult}; +use crate::{ModuleSourceMap, TextEdit}; use la_arena::Idx; use smol_str::{SmolStr, ToSmolStr}; use syntax::ast::{AstNode, Expr}; @@ -11,7 +13,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let name_res = ctx.db.name_resolution(file_id); let source_map = ctx.db.source_map(file_id); - let definition_of = |name: Idx| -> Option<(Expr, ast::AttrpathValue)> { + let definition_of = |name: Idx| -> Option<(ast::AttrpathValue, ast::Attr, Expr)> { let defs = source_map.nodes_for_name(name).collect::>(); let ptr = defs.first()?; let grandparent = ptr.to_node(ctx.ast.syntax()).parent()?.parent()?; @@ -19,10 +21,11 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { // Only provide assist when there is only one node // i.e. `let a.b = 1; a.c = 2; in a` is not supported - if path_value.attrpath()?.attrs().count() > 1 { - return None; - }; - Some((path_value.value()?, path_value)) + let attrs = path_value.attrpath()?.attrs().collect::>(); + match attrs.as_slice() { + [attr] => Some((path_value.clone(), attr.clone(), path_value.value()?)), + _ => None, + } }; let mut rewrites: Vec = vec![]; @@ -32,7 +35,7 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr)? else { return None; }; - let (definition, _) = definition_of(name)?; + let (_, _, definition) = definition_of(name)?; replace_usage(&mut rewrites, &definition, usage.syntax()); } else if let Some(attr) = ctx.covering_node::() { let (parent, _, parent3) = { @@ -63,13 +66,21 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { let &ResolveResult::Definition(name) = name_res.get(expr_id)? else { return None; }; - let (definition, _) = definition_of(name)?; + let (_, _, definition) = definition_of(name)?; replace_usage(&mut rewrites, &definition, attr.syntax()); } else if is_in_letin || is_in_recattr { // attr is a definition here let ptr = AstPtr::new(attr.syntax()); let name = source_map.name_for_node(ptr)?; - let (definition, path_value) = definition_of(name)?; + let (path_value, binding, definition) = definition_of(name)?; + + // We must not publish assist when the binding is recursive, since it can not be rewritten. + let binding_ptr = AstPtr::new(binding.syntax()); + let needle = source_map.name_for_node(binding_ptr)?; + if usage_count_in(needle, definition.syntax(), &source_map, &name_res) > 0 { + return None; + } + let usages = name_res.iter().filter_map(|(id, res)| match res { &ResolveResult::Definition(def) if def == name => source_map .node_for_expr(id) @@ -103,6 +114,29 @@ pub(super) fn inline(ctx: &mut AssistsCtx<'_>) -> Option<()> { Some(()) } +// Check recursively if one node is contained in another one. +fn usage_count_in( + needle: Idx, + hay: &SyntaxNode, + source_map: &Arc, + name_res: &Arc, +) -> i8 { + let self_reference_amount: i8 = hay + .children() + .map(|child| { + let resolve_result = source_map + .expr_for_node(AstPtr::new(&child)) + .and_then(|expr_id| name_res.get(expr_id)); + + match resolve_result { + Some(&ResolveResult::Definition(name_id)) if name_id == needle => 1, + _ => usage_count_in(needle, &child, source_map, name_res), + } + }) + .sum(); + self_reference_amount +} + // Parenthesize a node properly given the replacement context fn maybe_parenthesize(replacement: &ast::Expr, original: &SyntaxNode) -> SmolStr { let parent = original.parent().and_then(ast::Expr::cast); @@ -231,4 +265,27 @@ mod tests { fn no_attr() { check_no("{ $0foo = 1; }"); } + + #[test] + fn no_recursive_usage() { + // Silly but semantically correct cases + check( + "{ foo = f: let x = f x; in $0x; }", + expect!["{ foo = f: let x = f x; in f x; }"], + ); + check( + "{ foo = f: let x = f $0x; in x; }", + expect!["{ foo = f: let x = f (f x); in x; }"], + ); + check( + "{ foo = f: let x = f x; in { inherit $0x; }; }", + expect!["{ foo = f: let x = f x; in { inherit ; x = f x; }; }"], + ); + + // Cases where rewriting would change the semantics + check_no("{ foo = f: let $0x = f x; in x; }"); + check_no("let $0x = f x; in x"); + check_no("f: rec { $0x = f x; }"); + check_no("let $0y = { y = y; }; in y"); + } } From fc55bddbf8845aa43cd51e6712b1acd56eb7d438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9ana=20=E6=B1=9F?= Date: Sat, 11 Oct 2025 11:09:56 +0800 Subject: [PATCH 17/17] assists/inline: simplify tests --- crates/ide/src/ide/assists/inline.rs | 103 ++++++++++++++------------- 1 file changed, 55 insertions(+), 48 deletions(-) diff --git a/crates/ide/src/ide/assists/inline.rs b/crates/ide/src/ide/assists/inline.rs index 8f39576..98df346 100644 --- a/crates/ide/src/ide/assists/inline.rs +++ b/crates/ide/src/ide/assists/inline.rs @@ -180,60 +180,57 @@ mod tests { define_check_assist!(super::inline); #[test] - fn let_in_ref() { + fn let_in() { + // usage check("let a = 1; in $0a", expect!["let a = 1; in 1"]); check( "let a = x: x; in $0a 1", expect!["let a = x: x; in (x: x) 1"], ); + // definition + check("let $0a = 1; in a", expect!["let in 1"]); + check("let $0a = x: x; in a 1", expect!["let in (x: x) 1"]); } #[test] - fn let_in_def() { - check("let $0a = x: x; in a a", expect!["let in (x: x) (x: x)"]); - } - - #[test] - fn no_let_in_multi() { + fn reject_multi_let_in() { + // usage check_no("let a.b = 1; a.c = 2; in $0a"); - check_no("let a.b$0 = 1; a.c = 2; in a"); + // definition check_no("let $0a.b = 1; a.c = 2; in a"); + check_no("let a.b$0 = 1; a.c = 2; in a"); } #[test] - fn rec_attr_ref() { + fn rec_attr() { + // usage check( - "rec { foo = 1; bar = $0foo; }", - expect!["rec { foo = 1; bar = 1; }"], + "rec { foo = 1; bar = foo; baz = $0foo; }", + expect!["rec { foo = 1; bar = foo; baz = 1; }"], ); - } - - #[test] - fn rec_attr_def() { + check( + "rec { foo = x: x; bar = foo; baz = $0foo; }", + expect!["rec { foo = x: x; bar = foo; baz = x: x; }"], + ); + // definition check( "rec { $0foo = 1; bar = foo; baz = foo; }", expect!["rec { foo = 1; bar = 1; baz = 1; }"], ); + check( + "rec { $0foo = x: x; bar = foo; baz = foo; }", + expect!["rec { foo = x: x; bar = x: x; baz = x: x; }"], + ); } #[test] - fn allow_inherit_usage_def() { - check( - "let $0foo = 1; in { inherit foo; }", - expect!["let in { inherit ; foo = 1; }"], - ); - check( - "let $0foo = 1; bar = 2; in { inherit foo bar; }", - expect!["let bar = 2; in { inherit bar; foo = 1; }"], - ); - check( - "let $0foo = 1; bar = 2; in let inherit foo bar; in null", - expect!["let bar = 2; in let inherit bar; foo = 1; in null"], - ); + fn reject_non_rec_attr() { + check_no("{ $0foo = 1; }"); } #[test] - fn allow_inherit_usage_ref() { + fn let_inherit_in() { + // usage check( "let foo = 1; in { inherit $0foo; }", expect!["let foo = 1; in { inherit ; foo = 1; }"], @@ -246,45 +243,55 @@ mod tests { "let foo = 1; bar = 2; in let inherit $0foo bar; in null", expect!["let foo = 1; bar = 2; in let inherit bar; foo = 1; in null"], ); + + // definition + check( + "let $0foo = 1; in { inherit foo; }", + expect!["let in { inherit ; foo = 1; }"], + ); + check( + "let $0foo = 1; bar = 2; in { inherit foo bar; }", + expect!["let bar = 2; in { inherit bar; foo = 1; }"], + ); + check( + "let $0foo = 1; bar = 2; in let inherit foo bar; in null", + expect!["let bar = 2; in let inherit bar; foo = 1; in null"], + ); } #[test] - fn no_allow_inherit_as_def() { - check_no("let inherit (lib) $0foo; in foo"); + fn reject_inherit_from() { + // usage check_no("let inherit (lib) foo; in $0foo"); + check_no("{ foo = inherit (lib) $0bar; }"); + // definition + check_no("let inherit (lib) $0foo; in foo"); } #[test] - fn no_patfield() { + fn reject_patfield() { + // usage check_no("{ outputs = { nixpkgs, ... }: { inherit $0nixpkgs; }; }"); check_no("{ outputs = { $0nixpkgs, ... }: { inherit nixpkgs; }; }"); + // definition check_no("{ outputs = { $0nixpkgs, ... }: nixpkgs; }"); } #[test] - fn no_attr() { - check_no("{ $0foo = 1; }"); - } - - #[test] - fn no_recursive_usage() { + fn recursive_usage() { // Silly but semantically correct cases + check("f: let x = f x; in $0x", expect!["f: let x = f x; in f x"]); check( - "{ foo = f: let x = f x; in $0x; }", - expect!["{ foo = f: let x = f x; in f x; }"], - ); - check( - "{ foo = f: let x = f $0x; in x; }", - expect!["{ foo = f: let x = f (f x); in x; }"], + "f: let x = f $0x; in x", + expect!["f: let x = f (f x); in x"], ); check( - "{ foo = f: let x = f x; in { inherit $0x; }; }", - expect!["{ foo = f: let x = f x; in { inherit ; x = f x; }; }"], + "f: let x = f x; in { inherit $0x; }", + expect!["f: let x = f x; in { inherit ; x = f x; }"], ); // Cases where rewriting would change the semantics - check_no("{ foo = f: let $0x = f x; in x; }"); - check_no("let $0x = f x; in x"); + check_no("f: let $0x = f x; in x"); check_no("f: rec { $0x = f x; }"); check_no("let $0y = { y = y; }; in y"); }