Skip to content

Commit 192d745

Browse files
mhuisiclaude
authored andcommitted
fix: tactic completion in empty by blocks (#13348)
This PR fixes a bug where tactic auto-completion would produce tactic completion items in the entire trailing whitespace of an empty tactic block. Since #13229 further restricted top-level `by` blocks to be indentation- sensitive, this PR adjusts the logic to only display completion items at a "proper" indentation level. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4304aec commit 192d745

7 files changed

Lines changed: 650 additions & 874 deletions

src/Lean/Server/Completion/SyntheticCompletion.lean

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,13 @@ private partial def isSyntheticTacticCompletion
117117
(cmdStx : Syntax)
118118
: Bool := Id.run do
119119
let hoverFilePos := fileMap.toPosition hoverPos
120-
go hoverFilePos cmdStx 0
120+
go hoverFilePos cmdStx 0 none
121121
where
122122
go
123-
(hoverFilePos : Position)
124-
(stx : Syntax)
125-
(leadingWs : Nat)
123+
(hoverFilePos : Position)
124+
(stx : Syntax)
125+
(leadingWs : Nat)
126+
(leadingTokenTailPos? : Option String.Pos.Raw)
126127
: Bool := Id.run do
127128
match stx.getPos?, stx.getTailPos? with
128129
| some startPos, some endPos =>
@@ -132,16 +133,22 @@ where
132133
if ! isCursorInCompletionRange then
133134
return false
134135
let mut wsBeforeArg := leadingWs
136+
let mut lastArgTailPos? := leadingTokenTailPos?
135137
for arg in stx.getArgs do
136-
if go hoverFilePos arg wsBeforeArg then
138+
if go hoverFilePos arg wsBeforeArg lastArgTailPos? then
137139
return true
138140
-- We must account for the whitespace before an argument because the syntax nodes we use
139141
-- to identify tactic blocks only start *after* the whitespace following a `by`, and we
140142
-- want to provide tactic completions in that whitespace as well.
141143
-- This method of computing whitespace assumes that there are no syntax nodes without tokens
142144
-- after `by` and before the first proper tactic syntax.
143145
wsBeforeArg := arg.getTrailingSize
144-
return isCompletionInEmptyTacticBlock stx
146+
-- Track the tail position of the most recent preceding sibling that has a position so
147+
-- that empty tactic blocks (which lack positions) can locate their opening token (e.g.
148+
-- the `by` keyword) for indentation checking. The tail position lets us distinguish
149+
-- cursors before and after the opener on the opener's line.
150+
lastArgTailPos? := arg.getTailPos? <|> lastArgTailPos?
151+
return isCompletionInEmptyTacticBlock stx lastArgTailPos?
145152
|| isCompletionAfterSemicolon stx
146153
|| isCompletionOnTacticBlockIndentation hoverFilePos stx
147154
| _, _ =>
@@ -150,7 +157,7 @@ where
150157
-- tactic blocks always occur within other syntax with ranges that let us narrow down the
151158
-- search to the degree that we can be sure that the cursor is indeed in this empty tactic
152159
-- block.
153-
return isCompletionInEmptyTacticBlock stx
160+
return isCompletionInEmptyTacticBlock stx leadingTokenTailPos?
154161

155162
isCompletionOnTacticBlockIndentation
156163
(hoverFilePos : Position)
@@ -190,8 +197,47 @@ where
190197
else
191198
none
192199

193-
isCompletionInEmptyTacticBlock (stx : Syntax) : Bool :=
194-
isCursorInProperWhitespace fileMap hoverPos && isEmptyTacticBlock stx
200+
isCompletionInEmptyTacticBlock (stx : Syntax) (leadingTokenTailPos? : Option String.Pos.Raw) : Bool := Id.run do
201+
if ! isCursorInProperWhitespace fileMap hoverPos then
202+
return false
203+
if ! isEmptyTacticBlock stx then
204+
return false
205+
-- Bracketed tactic blocks `{ ... }` are delimited by the braces themselves, so tactics
206+
-- inserted in an empty bracketed block can appear at any column between the braces
207+
-- (`withoutPosition` disables indentation constraints inside `tacticSeqBracketed`).
208+
if stx.getKind == ``Parser.Tactic.tacticSeqBracketed then
209+
let some openEndPos := stx[0].getTailPos?
210+
| return false
211+
let some closeStartPos := stx[2].getPos?
212+
| return false
213+
return openEndPos.byteIdx <= hoverPos.byteIdx && hoverPos.byteIdx <= closeStartPos.byteIdx
214+
return isAtExpectedTacticIndentation leadingTokenTailPos?
215+
216+
-- After an empty tactic opener like `by`, tactics on a subsequent line must be inserted at an
217+
-- increased indentation level (two spaces past the indentation of the line containing the
218+
-- opener token). We mirror that here so that tactic completions are not offered in positions
219+
-- where a tactic could not actually be inserted. On the same line as the opener, completions
220+
-- are allowed only in the trailing whitespace past the opener — cursors earlier on the line
221+
-- belong to the surrounding term, not to the tactic block.
222+
isAtExpectedTacticIndentation (leadingTokenTailPos? : Option String.Pos.Raw) : Bool := Id.run do
223+
let some tokenTailPos := leadingTokenTailPos?
224+
| return true
225+
let hoverFilePos := fileMap.toPosition hoverPos
226+
let tokenTailFilePos := fileMap.toPosition tokenTailPos
227+
if hoverFilePos.line == tokenTailFilePos.line then
228+
return hoverPos.byteIdx >= tokenTailPos.byteIdx
229+
let expectedColumn := countLeadingSpaces (fileMap.lineStart tokenTailFilePos.line) + 2
230+
return hoverFilePos.column == expectedColumn
231+
232+
countLeadingSpaces (pos : String.Pos.Raw) : Nat := Id.run do
233+
let mut p := pos
234+
let mut n : Nat := 0
235+
while ! p.atEnd fileMap.source do
236+
if p.get fileMap.source != ' ' then
237+
break
238+
p := p.next fileMap.source
239+
n := n + 1
240+
return n
195241

196242
isEmptyTacticBlock (stx : Syntax) : Bool :=
197243
stx.getKind == ``Parser.Tactic.tacticSeq && isEmpty stx

tests/server_interactive/completionEmptyBy.lean

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@ example : True := id <|
2828
have : True := by
2929
--^ completion
3030

31+
-- ===== Cursor before `by` on the same line (no completions expected) =====
32+
33+
-- Only the trailing whitespace past an opener token (here `by`) should offer completions. A
34+
-- cursor in whitespace that precedes the opener on the same line must not — that whitespace is
35+
-- part of the surrounding term, not the tactic block.
36+
37+
-- column 18 between `:=` and `by` (cursor is before `by` on the same line)
38+
example : True := by
39+
--^ completion
40+
41+
-- column 4 in leading indent of a line whose only token is `by`
42+
example : True :=
43+
by
44+
--^ completion
45+
46+
-- column 18 before nested `by`, on the same line as `by`
47+
example : True := id <|
48+
have : True := by
49+
--^ completion
50+
3151
-- ===== Top-level `by`, non-indented content following =====
3252

3353
-- column 21 on line below `by`

0 commit comments

Comments
 (0)