From 932bceb49ad7eb7697792667da02e26a3f6b8fae Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 1 Apr 2026 20:50:11 +0200 Subject: [PATCH 1/2] Fix LSP completion on CEL receivers --- private/buf/buflsp/completion_cel.go | 80 +++++++++++++++++-- private/buf/buflsp/completion_cel_test.go | 44 +++++++++- .../testdata/completion/cel_completion.proto | 21 +++++ 3 files changed, 137 insertions(+), 8 deletions(-) diff --git a/private/buf/buflsp/completion_cel.go b/private/buf/buflsp/completion_cel.go index 36b821bbd2..92686fcfe5 100644 --- a/private/buf/buflsp/completion_cel.go +++ b/private/buf/buflsp/completion_cel.go @@ -222,10 +222,11 @@ func getCELCompletionItems( // either right after a dot ("this.") or typing a member name ("this.ci"). // Returns the receiver expression, the typed prefix (possibly empty), and ok=true. // Examples: -// - "this." → ("this", "", true) -// - "this.ci" → ("this", "ci", true) -// - "size(" → ("", "", false) -// - "si" → ("", "", false) +// - "this." → ("this", "", true) +// - "this.ci" → ("this", "ci", true) +// - "this.create_time < this.up" → ("this", "up", true) +// - "size(" → ("", "", false) +// - "si" → ("", "", false) func celParseMemberAccess(celContent string) (receiverExpr, memberPrefix string, ok bool) { // Peel off the identifier being typed at the end of celContent. wordStart := len(celContent) @@ -241,14 +242,81 @@ func celParseMemberAccess(celContent string) (receiverExpr, memberPrefix string, return "", "", false } - // Receiver expression is everything before the dot. - receiver := strings.TrimRight(trimmed[:len(trimmed)-1], " \t\r\n") + // Extract just the receiver expression before the dot by scanning backwards. + // The receiver can contain identifiers, dots (chained access), balanced brackets, + // and balanced parens (function calls). Operators and other tokens are boundaries. + receiver := celExtractReceiver(trimmed[:len(trimmed)-1]) if receiver == "" { return "", "", false } return receiver, prefix, true } +// celExtractReceiver scans backwards from the end of s to extract the receiver +// expression for a member access. It handles dot chains (this.field), bracket +// indexing (this.items[0]), and function calls (size(x)). Stops at operator +// boundaries, commas, and unbalanced punctuation. +// +// String literals inside bracket expressions (e.g. this.items["key with spaces"]) +// are not parsed; the scanner treats bracket contents as raw bytes. +func celExtractReceiver(s string) string { + s = strings.TrimRight(s, " \t\r\n") + if s == "" { + return "" + } + i := len(s) + for i > 0 { + c := s[i-1] + switch { + case celIsIdentChar(c): + for i > 0 && celIsIdentChar(s[i-1]) { + i-- + } + case c == '.': + i-- + case c == ']': + var ok bool + if i, ok = celScanBalanced(s, i, '[', ']'); !ok { + return "" + } + case c == ')': + var ok bool + if i, ok = celScanBalanced(s, i, '(', ')'); !ok { + return "" + } + // Include the function name before '('. + for i > 0 && celIsIdentChar(s[i-1]) { + i-- + } + default: + // Strip leading whitespace that may sit between the operator + // boundary and the start of the receiver expression. + return strings.TrimLeft(s[i:], " \t\r\n") + } + } + return s[i:] +} + +// celScanBalanced scans backwards from position i in s to find the matching +// open delimiter for the close delimiter at s[i-1]. Returns the new position +// and true on success, or (0, false) if the delimiters are unbalanced. +func celScanBalanced(s string, i int, open, close byte) (int, bool) { + depth := 1 + i-- + for i > 0 && depth > 0 { + if s[i-1] == close { + depth++ + } else if s[i-1] == open { + depth-- + } + i-- + } + if depth != 0 { + return 0, false + } + return i, true +} + // celParseHasArg detects if the cursor is inside the argument of a has() macro call. // The has() macro takes a single select expression: has(receiver.field). // Returns the content after the last unqualified "has(" when the cursor is still diff --git a/private/buf/buflsp/completion_cel_test.go b/private/buf/buflsp/completion_cel_test.go index cacdf833b3..7b8bbf1511 100644 --- a/private/buf/buflsp/completion_cel_test.go +++ b/private/buf/buflsp/completion_cel_test.go @@ -57,7 +57,10 @@ func TestCELCompletion(t *testing.T) { // 206: ` expression: "this.items[0]."` (IndexAccessHolder — indexed into list, yields element fields) // 210: ` expression: "this.locations[\"key\"]."` (IndexAccessHolder — indexed into map, yields value fields) // 222: ` expression: "this.items.filter(item, item.zip_code > 0).all(addr, addr."` (ChainedComprehensionHolder) - // 232: ` expression: "in"` (InOperatorHolder — "in" is an operator, not a function) + // 235: ` expression: "this."` (MultiThisHolder — first this, message fields) + // 239: ` expression: "this.create_time < this."` (MultiThisHolder — second this after operator) + // 243: ` expression: "this.create_time < this.up"` (MultiThisHolder — second this with prefix) + // 253: ` expression: "in"` (InOperatorHolder — "in" is an operator, not a function) tests := []struct { name string line uint32 @@ -451,12 +454,49 @@ func TestCELCompletion(t *testing.T) { }, expectedNotContains: []string{"size", "all", "true", "false", "null"}, }, + { + // MultiThisHolder: first `this.` in the expression. Cursor at closing `"` of `"this."`. + // `this` refers to MultiThisHolder; field completions should include create_time and update_time. + name: "multi_this_first_dot", + line: 235, + character: 22, // closing `"` after `this.` + expectedContains: []string{ + "create_time", + "update_time", + }, + expectedNotContains: []string{"true", "false", "null"}, + }, + { + // MultiThisHolder: second `this.` after `<` operator. Cursor at closing `"` of + // `"this.create_time < this."`. The second `this.` must still resolve to + // MultiThisHolder and offer field completions. + name: "multi_this_second_dot", + line: 239, + character: 41, // closing `"` after `this.create_time < this.` + expectedContains: []string{ + "create_time", + "update_time", + }, + expectedNotContains: []string{"true", "false", "null"}, + }, + { + // MultiThisHolder: second `this.` with prefix "up" after `<` operator. + // Cursor at closing `"` of `"this.create_time < this.up"`. Should filter + // to update_time only. + name: "multi_this_prefix", + line: 243, + character: 43, // closing `"` after `this.create_time < this.up` + expectedContains: []string{ + "update_time", + }, + expectedNotContains: []string{"create_time", "true", "false", "null"}, + }, { // Cursor at closing `"` of `"in"` — prefix "in". // `in` is a CEL binary membership operator ("value in list"), not a // callable function. It must NOT appear as a function completion "in()". name: "in_operator_not_function_completion", - line: 232, + line: 253, character: 19, // closing `"` after `in` expectedNotContains: []string{"in"}, }, diff --git a/private/buf/buflsp/testdata/completion/cel_completion.proto b/private/buf/buflsp/testdata/completion/cel_completion.proto index 00d86df00c..e6f20d102e 100644 --- a/private/buf/buflsp/testdata/completion/cel_completion.proto +++ b/private/buf/buflsp/testdata/completion/cel_completion.proto @@ -224,6 +224,27 @@ message ChainedComprehensionHolder { }; } +// MultiThisHolder exercises completions when `this.` appears multiple times +// in a single CEL expression separated by an operator. The second `this.` +// must still resolve to the message type and offer field completions. +message MultiThisHolder { + google.protobuf.Timestamp create_time = 1; + google.protobuf.Timestamp update_time = 2; + + option (buf.validate.message).cel = { + id: "multi.this.first" + expression: "this." + }; + option (buf.validate.message).cel = { + id: "multi.this.second" + expression: "this.create_time < this." + }; + option (buf.validate.message).cel = { + id: "multi.this.prefix" + expression: "this.create_time < this.up" + }; +} + // InOperatorHolder verifies that the "in" membership operator does not appear // as a callable function completion ("in()") when typed as a prefix. // "in" is a binary infix operator ("value in list"), not a function call. From d4e7a47f020c0a9b9a897de9e56e25f8f0021e9a Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 1 Apr 2026 20:55:31 +0200 Subject: [PATCH 2/2] Fix lint --- private/buf/buflsp/completion_cel.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/private/buf/buflsp/completion_cel.go b/private/buf/buflsp/completion_cel.go index 92686fcfe5..efa640c6d3 100644 --- a/private/buf/buflsp/completion_cel.go +++ b/private/buf/buflsp/completion_cel.go @@ -298,15 +298,16 @@ func celExtractReceiver(s string) string { } // celScanBalanced scans backwards from position i in s to find the matching -// open delimiter for the close delimiter at s[i-1]. Returns the new position +// open delimiter for the closing delimiter at s[i-1]. Returns the new position // and true on success, or (0, false) if the delimiters are unbalanced. -func celScanBalanced(s string, i int, open, close byte) (int, bool) { +func celScanBalanced(s string, i int, open, closeDelim byte) (int, bool) { depth := 1 i-- for i > 0 && depth > 0 { - if s[i-1] == close { + switch s[i-1] { + case closeDelim: depth++ - } else if s[i-1] == open { + case open: depth-- } i--