From ae13101073ca05aa8ea44b468280ea7d885785dc Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 24 May 2024 11:19:30 -0500 Subject: [PATCH 1/4] Tell lsp about variable bindings in match statements --- lsp/nls/src/usage.rs | 28 +++++++++++++++++++ lsp/nls/tests/inputs/completion-match.ncl | 24 ++++++++++++++++ ...__tests__inputs__completion-match.ncl.snap | 7 +++++ 3 files changed, 59 insertions(+) create mode 100644 lsp/nls/tests/inputs/completion-match.ncl create mode 100644 lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap diff --git a/lsp/nls/src/usage.rs b/lsp/nls/src/usage.rs index 68903fd2a7..b82db2685d 100644 --- a/lsp/nls/src/usage.rs +++ b/lsp/nls/src/usage.rs @@ -162,6 +162,34 @@ impl UsageLookup { TraverseControl::ContinueWithScope(new_env) } + Term::App(f, value) => { + if let Term::Match(data) = f.as_ref() { + for branch in &data.branches { + let mut new_env = env.clone(); + for (path, ident, _field) in branch.pattern.bindings() { + let def = Def::Let { + ident: ident.into(), + value: value.clone(), + path: path.into_iter().map(|x| x.ident()).collect(), + }; + new_env.insert_def(def.clone()); + self.add_sym(def); + } + self.fill(&branch.body, &new_env); + if let Some(guard) = &branch.guard { + self.fill(guard, &new_env); + } + } + + // We've already traversed the branch bodies. We don't want to continue + // traversal because that will traverse them again. But we need to traverse + // the value we're matching on. + self.fill(value, env); + TraverseControl::SkipBranch + } else { + TraverseControl::Continue + } + } Term::Var(id) => { let id = LocIdent::from(*id); if let Some(def) = env.get(&id.ident) { diff --git a/lsp/nls/tests/inputs/completion-match.ncl b/lsp/nls/tests/inputs/completion-match.ncl new file mode 100644 index 0000000000..67b19fbae4 --- /dev/null +++ b/lsp/nls/tests/inputs/completion-match.ncl @@ -0,0 +1,24 @@ +### /input.ncl +let p = 1 in +{ + foo = { bar = 1 }, + baz, +} |> match { + 'Bar payload => p, + x => x.foo.ba, + { foo = x } if x.ba == 1 => 3, +} +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 5, character = 18 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 6, character = 14 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 7, character = 20 } diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap new file mode 100644 index 0000000000..c8329025b9 --- /dev/null +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap @@ -0,0 +1,7 @@ +--- +source: lsp/nls/tests/main.rs +expression: output +--- +[p, payload, std] +[bar] +[bar] From e36a20141c45f8a5471b7b63844293358a8275ac Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 24 May 2024 18:12:01 -0500 Subject: [PATCH 2/4] Also handle unapplied matches --- lsp/nls/src/usage.rs | 63 +++++++++++++++++------ lsp/nls/tests/inputs/completion-match.ncl | 13 +++++ 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/lsp/nls/src/usage.rs b/lsp/nls/src/usage.rs index b82db2685d..4e455abf78 100644 --- a/lsp/nls/src/usage.rs +++ b/lsp/nls/src/usage.rs @@ -4,7 +4,7 @@ use nickel_lang_core::{ environment::Environment as GenericEnvironment, identifier::Ident, position::RawSpan, - term::{RichTerm, Term, Traverse, TraverseControl}, + term::{MatchData, RichTerm, Term, Traverse, TraverseControl}, }; use crate::{field_walker::Def, identifier::LocIdent, pattern::Bindings}; @@ -90,6 +90,46 @@ impl UsageLookup { self.syms.insert(def.ident(), def); } + // In general, a match is like a function in that it needs to be applied before we + // know what's being matched on. So for example, in + // ``` + // match { x => x.ba } + // ``` + // we can't do much to auto-complete "ba". But it's very common in practice for the match to + // be applied immediately, like + // ``` + // y |> match { x => x.ba } + // ``` + // and in this case we can look into `y` to extract completions for "ba". + // + // This is a long-winded way of saying that we can treat pattern bindings like we treat + // function bindings (if we don't know where the application is) or like we treat let bindings + // (if we know what the match gets applied to). Here, `value` is the value that the match + // is applied to, if we know it. + fn fill_match(&mut self, env: &Environment, data: &MatchData, value: Option<&RichTerm>) { + for branch in &data.branches { + let mut new_env = env.clone(); + for (path, ident, _field) in branch.pattern.bindings() { + let def = match value { + Some(v) => Def::Let { + ident: ident.into(), + value: v.clone(), + path: path.into_iter().map(|x| x.ident()).collect(), + }, + None => Def::Fn { + ident: ident.into(), + }, + }; + new_env.insert_def(def.clone()); + self.add_sym(def); + } + self.fill(&branch.body, &new_env); + if let Some(guard) = &branch.guard { + self.fill(guard, &new_env); + } + } + } + fn fill(&mut self, rt: &RichTerm, env: &Environment) { rt.traverse_ref( &mut |term: &RichTerm, env: &Environment| { @@ -164,22 +204,7 @@ impl UsageLookup { } Term::App(f, value) => { if let Term::Match(data) = f.as_ref() { - for branch in &data.branches { - let mut new_env = env.clone(); - for (path, ident, _field) in branch.pattern.bindings() { - let def = Def::Let { - ident: ident.into(), - value: value.clone(), - path: path.into_iter().map(|x| x.ident()).collect(), - }; - new_env.insert_def(def.clone()); - self.add_sym(def); - } - self.fill(&branch.body, &new_env); - if let Some(guard) = &branch.guard { - self.fill(guard, &new_env); - } - } + self.fill_match(env, data, Some(value)); // We've already traversed the branch bodies. We don't want to continue // traversal because that will traverse them again. But we need to traverse @@ -190,6 +215,10 @@ impl UsageLookup { TraverseControl::Continue } } + Term::Match(data) => { + self.fill_match(env, data, None); + TraverseControl::SkipBranch + } Term::Var(id) => { let id = LocIdent::from(*id); if let Some(def) = env.get(&id.ident) { diff --git a/lsp/nls/tests/inputs/completion-match.ncl b/lsp/nls/tests/inputs/completion-match.ncl index 67b19fbae4..687317913d 100644 --- a/lsp/nls/tests/inputs/completion-match.ncl +++ b/lsp/nls/tests/inputs/completion-match.ncl @@ -7,6 +7,9 @@ let p = 1 in 'Bar payload => p, x => x.foo.ba, { foo = x } if x.ba == 1 => 3, + # In this next one, the match isn't applied so there will be no completions for the `blah`, + # but there should be a completion for `p`. + y => match { payload => p.blah }, } ### [[request]] ### type = "Completion" @@ -22,3 +25,13 @@ let p = 1 in ### type = "Completion" ### textDocument.uri = "file:///input.ncl" ### position = { line = 7, character = 20 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 10, character = 26 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 10, character = 29 } From 42b9c53708e23f794a39c3dd675df7d631f5c0d8 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 24 May 2024 18:12:15 -0500 Subject: [PATCH 3/4] insta review --- .../main__lsp__nls__tests__inputs__completion-match.ncl.snap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap index c8329025b9..c927d42e08 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match.ncl.snap @@ -5,3 +5,5 @@ expression: output [p, payload, std] [bar] [bar] +[p, payload, std, y] +[] From 0cd74c8ca6f61277c3c64a2e19e0c4dbfd18014f Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 24 May 2024 18:20:12 -0500 Subject: [PATCH 4/4] Add a test getting match branch completions from type inference --- lsp/nls/tests/inputs/completion-match-typed.ncl | 7 +++++++ ...sp__nls__tests__inputs__completion-match-typed.ncl.snap | 5 +++++ 2 files changed, 12 insertions(+) create mode 100644 lsp/nls/tests/inputs/completion-match-typed.ncl create mode 100644 lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap diff --git a/lsp/nls/tests/inputs/completion-match-typed.ncl b/lsp/nls/tests/inputs/completion-match-typed.ncl new file mode 100644 index 0000000000..8d3eb43301 --- /dev/null +++ b/lsp/nls/tests/inputs/completion-match-typed.ncl @@ -0,0 +1,7 @@ +### /input.ncl +fun x => match { y => y.fo } +: { foo: Number, fo: Number } -> Number +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 0, character = 26 } diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap new file mode 100644 index 0000000000..a79a763ad9 --- /dev/null +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap @@ -0,0 +1,5 @@ +--- +source: lsp/nls/tests/main.rs +expression: output +--- +[fo, foo]