Skip to content

Commit

Permalink
fix: don't use info nodes before cursor for completion (#3778)
Browse files Browse the repository at this point in the history
This fixes an issue where the completion would use info nodes before the
cursor for computing completions.

Fixes #3462.

ToDo:
- [x] Fix test failures for completions that previously worked by
accident (cc: @Kha)
- [x] stage0 update

---------

Co-authored-by: Sebastian Ullrich <[email protected]>
  • Loading branch information
mhuisi and Kha authored Apr 2, 2024
1 parent eacb179 commit ecf0459
Show file tree
Hide file tree
Showing 99 changed files with 38,683 additions and 34,372 deletions.
6 changes: 3 additions & 3 deletions src/Lean/Elab/App.lean
Original file line number Diff line number Diff line change
Expand Up @@ -1199,8 +1199,8 @@ private def elabAppLValsAux (namedArgs : Array NamedArg) (args : Array Arg) (exp
let rec loop : Expr → List LVal → TermElabM Expr
| f, [] => elabAppArgs f namedArgs args expectedType? explicit ellipsis
| f, lval::lvals => do
if let LVal.fieldName (ref := fieldStx) (targetStx := targetStx) .. := lval then
addDotCompletionInfo targetStx f expectedType? fieldStx
if let LVal.fieldName (fullRef := fullRef) .. := lval then
addDotCompletionInfo fullRef f expectedType?
let hasArgs := !namedArgs.isEmpty || !args.isEmpty
let (f, lvalRes) ← resolveLVal f lval hasArgs
match lvalRes with
Expand Down Expand Up @@ -1340,7 +1340,7 @@ private partial def elabAppFn (f : Syntax) (lvals : List LVal) (namedArgs : Arra
let elabFieldName (e field : Syntax) := do
let newLVals := field.identComponents.map fun comp =>
-- We use `none` in `suffix?` since `field` can't be part of a composite name
LVal.fieldName comp comp.getId.getString! none e
LVal.fieldName comp comp.getId.getString! none f
elabAppFn e (newLVals ++ lvals) namedArgs args expectedType? explicit ellipsis overloaded acc
let elabFieldIdx (e idxStx : Syntax) := do
let some idx := idxStx.isFieldIdx? | throwError "invalid field index"
Expand Down
2 changes: 1 addition & 1 deletion src/Lean/Elab/BuiltinCommand.lean
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ def elabRunMeta : CommandElab := fun stx =>
pure ()

@[builtin_command_elab «set_option»] def elabSetOption : CommandElab := fun stx => do
let options ← Elab.elabSetOption stx[1] stx[2]
let options ← Elab.elabSetOption stx[1] stx[3]
modify fun s => { s with maxRecDepth := maxRecDepth.get options }
modifyScope fun scope => { scope with opts := options }

Expand Down
4 changes: 2 additions & 2 deletions src/Lean/Elab/BuiltinTerm.lean
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,9 @@ private def mkSilentAnnotationIfHole (e : Expr) : TermElabM Expr := do
popScope

@[builtin_term_elab «set_option»] def elabSetOption : TermElab := fun stx expectedType? => do
let options ← Elab.elabSetOption stx[1] stx[2]
let options ← Elab.elabSetOption stx[1] stx[3]
withTheReader Core.Context (fun ctx => { ctx with maxRecDepth := maxRecDepth.get options, options := options }) do
elabTerm stx[4] expectedType?
elabTerm stx[5] expectedType?

@[builtin_term_elab withAnnotateTerm] def elabWithAnnotateTerm : TermElab := fun stx expectedType? => do
match stx with
Expand Down
2 changes: 1 addition & 1 deletion src/Lean/Elab/Command.lean
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ first evaluates any local `set_option ... in ...` clauses and then invokes `cmd`
partial def withSetOptionIn (cmd : CommandElab) : CommandElab := fun stx => do
if stx.getKind == ``Lean.Parser.Command.in &&
stx[0].getKind == ``Lean.Parser.Command.set_option then
let opts ← Elab.elabSetOption stx[0][1] stx[0][2]
let opts ← Elab.elabSetOption stx[0][1] stx[0][3]
Command.withScope (fun scope => { scope with opts }) do
withSetOptionIn cmd stx[1]
else
Expand Down
2 changes: 1 addition & 1 deletion src/Lean/Elab/InfoTree/Types.lean
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ structure CommandInfo extends ElabInfo where
/-- A completion is an item that appears in the [IntelliSense](https://code.visualstudio.com/docs/editor/intellisense)
box that appears as you type. -/
inductive CompletionInfo where
| dot (termInfo : TermInfo) (field? : Option Syntax) (expectedType? : Option Expr)
| dot (termInfo : TermInfo) (expectedType? : Option Expr)
| id (stx : Syntax) (id : Name) (danglingDot : Bool) (lctx : LocalContext) (expectedType? : Option Expr)
| dotId (stx : Syntax) (id : Name) (lctx : LocalContext) (expectedType? : Option Expr)
| fieldId (stx : Syntax) (id : Name) (lctx : LocalContext) (structName : Name)
Expand Down
2 changes: 1 addition & 1 deletion src/Lean/Elab/SetOption.lean
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def elabSetOption (id : Syntax) (val : Syntax) : m Options := do
let ref ← getRef
-- For completion purposes, we discard `val` and any later arguments.
-- We include the first argument (the keyword) for position information in case `id` is `missing`.
addCompletionInfo <| CompletionInfo.option (ref.setArgs (ref.getArgs[0:2]))
addCompletionInfo <| CompletionInfo.option (ref.setArgs (ref.getArgs[0:3]))
let optionName := id.getId.eraseMacroScopes
let decl ← IO.toEIO (fun (ex : IO.Error) => Exception.error ref ex.toString) (getOptionDecl optionName)
pushInfoLeaf <| .ofOptionInfo { stx := id, optionName, declName := decl.declName }
Expand Down
4 changes: 2 additions & 2 deletions src/Lean/Elab/Tactic/BuiltinTactic.lean
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ private def getOptRotation (stx : Syntax) : Nat :=
popScope

@[builtin_tactic Parser.Tactic.set_option] def elabSetOption : Tactic := fun stx => do
let options ← Elab.elabSetOption stx[1] stx[2]
let options ← Elab.elabSetOption stx[1] stx[3]
withTheReader Core.Context (fun ctx => { ctx with maxRecDepth := maxRecDepth.get options, options := options }) do
evalTactic stx[4]
evalTactic stx[5]

@[builtin_tactic Parser.Tactic.allGoals] def evalAllGoals : Tactic := fun stx => do
let mvarIds ← getGoals
Expand Down
10 changes: 5 additions & 5 deletions src/Lean/Elab/Term.lean
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ builtin_initialize termElabAttribute : KeyedDeclsAttribute TermElab ← mkTermEl
inductive LVal where
| fieldIdx (ref : Syntax) (i : Nat)
/-- Field `suffix?` is for producing better error messages because `x.y` may be a field access or a hierarchical/composite name.
`ref` is the syntax object representing the field. `targetStx` is the target object being accessed. -/
| fieldName (ref : Syntax) (name : String) (suffix? : Option Name) (targetStx : Syntax)
`ref` is the syntax object representing the field. `fullRef` includes the LHS. -/
| fieldName (ref : Syntax) (name : String) (suffix? : Option Name) (fullRef : Syntax)

def LVal.getRef : LVal → Syntax
| .fieldIdx ref _ => ref
Expand Down Expand Up @@ -1409,9 +1409,9 @@ private partial def elabTermAux (expectedType? : Option Expr) (catchExPostpone :
trace[Elab.step.result] result
pure result

/-- Store in the `InfoTree` that `e` is a "dot"-completion target. -/
def addDotCompletionInfo (stx : Syntax) (e : Expr) (expectedType? : Option Expr) (field? : Option Syntax := none) : TermElabM Unit := do
addCompletionInfo <| CompletionInfo.dot { expr := e, stx, lctx := (← getLCtx), elaborator := .anonymous, expectedType? } (field? := field?) (expectedType? := expectedType?)
/-- Store in the `InfoTree` that `e` is a "dot"-completion target. `stx` should cover the entire term. -/
def addDotCompletionInfo (stx : Syntax) (e : Expr) (expectedType? : Option Expr) : TermElabM Unit := do
addCompletionInfo <| CompletionInfo.dot { expr := e, stx, lctx := (← getLCtx), elaborator := .anonymous, expectedType? } (expectedType? := expectedType?)

/--
Main function for elaborating terms.
Expand Down
2 changes: 1 addition & 1 deletion src/Lean/Linter/MissingDocs.lean
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def checkRegisterSimpAttr : SimpleHandler := mkSimpleHandler "simp attr"
@[builtin_missing_docs_handler «in»]
def handleIn : Handler := fun _ stx => do
if stx[0].getKind == ``«set_option» then
let opts ← Elab.elabSetOption stx[0][1] stx[0][2]
let opts ← Elab.elabSetOption stx[0][1] stx[0][3]
withScope (fun scope => { scope with opts }) do
missingDocs.run stx[2]
else
Expand Down
7 changes: 3 additions & 4 deletions src/Lean/Parser/Command.lean
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ match against a quotation in a command kind's elaborator). -/
@[builtin_term_parser low] def quot := leading_parser
"`(" >> withoutPosition (incQuotDepth (many1Unbox commandParser)) >> ")"


@[builtin_command_parser]
def moduleDoc := leading_parser ppDedent <|
"/-!" >> commentBody >> ppLine
Expand Down Expand Up @@ -236,7 +235,7 @@ def «structure» := leading_parser
"init_quot"
def optionValue := nonReservedSymbol "true" <|> nonReservedSymbol "false" <|> strLit <|> numLit
@[builtin_command_parser] def «set_option» := leading_parser
"set_option " >> ident >> ppSpace >> optionValue
"set_option " >> identWithPartialTrailingDot >> ppSpace >> optionValue
def eraseAttr := leading_parser
"-" >> rawIdent
@[builtin_command_parser] def «attribute» := leading_parser
Expand Down Expand Up @@ -324,7 +323,7 @@ It makes the given namespaces available in the term `e`.
It sets the option `opt` to the value `val` in the term `e`.
-/
@[builtin_term_parser] def «set_option» := leading_parser:leadPrec
"set_option " >> ident >> ppSpace >> Command.optionValue >> " in " >> termParser
"set_option " >> identWithPartialTrailingDot >> ppSpace >> Command.optionValue >> " in " >> termParser
end Term

namespace Tactic
Expand All @@ -336,7 +335,7 @@ but it opens a namespace only within the tactics `tacs`. -/
/-- `set_option opt val in tacs` (the tactic) acts like `set_option opt val` at the command level,
but it sets the option only within the tactics `tacs`. -/
@[builtin_tactic_parser] def «set_option» := leading_parser:leadPrec
"set_option " >> ident >> ppSpace >> Command.optionValue >> " in " >> tacticSeq
"set_option " >> identWithPartialTrailingDot >> ppSpace >> Command.optionValue >> " in " >> tacticSeq
end Tactic

end Parser
Expand Down
7 changes: 7 additions & 0 deletions src/Lean/Parser/Extra.lean
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ You can use `TSyntax.getId` to extract the name from the resulting syntax object
@[run_builtin_parser_attribute_hooks] def ident : Parser :=
withAntiquot (mkAntiquot "ident" identKind) identNoAntiquot

-- `optional (checkNoWsBefore >> "." >> checkNoWsBefore >> ident)`
-- can never fully succeed but ensures that the identifier
-- produces a partial syntax that contains the dot.
-- The partial syntax is sometimes useful for dot-auto-completion.
@[run_builtin_parser_attribute_hooks] def identWithPartialTrailingDot :=
ident >> optional (checkNoWsBefore >> "." >> checkNoWsBefore >> ident)

-- `ident` and `rawIdent` produce the same syntax tree, so we reuse the antiquotation kind name
@[run_builtin_parser_attribute_hooks] def rawIdent : Parser :=
withAntiquot (mkAntiquot "ident" identKind) rawIdentNoAntiquot
Expand Down
6 changes: 1 addition & 5 deletions src/Lean/Parser/Module.lean
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ namespace Parser

namespace Module
def «prelude» := leading_parser "prelude"
-- `optional (checkNoWsBefore >> "." >> checkNoWsBefore >> ident)`
-- can never fully succeed but ensures that `import (runtime)? <ident>.`
-- produces a partial syntax that contains the dot.
-- The partial syntax is useful for import dot-auto-completion.
def «import» := leading_parser "import " >> optional "runtime" >> ident >> optional (checkNoWsBefore >> "." >> checkNoWsBefore >> ident)
def «import» := leading_parser "import " >> optional "runtime" >> identWithPartialTrailingDot
def header := leading_parser optional («prelude» >> ppLine) >> many («import» >> ppLine) >> ppLine
/--
Parser for a Lean module. We never actually run this parser but instead use the imperative definitions below that
Expand Down
16 changes: 8 additions & 8 deletions src/Lean/Server/Completion.lean
Original file line number Diff line number Diff line change
Expand Up @@ -659,10 +659,10 @@ private def tacticCompletion (params : CompletionParams) (ctx : ContextInfo) : I
return some { items := sortCompletionItems items, isIncomplete := true }

private def findCompletionInfoAt?
(fileMap : FileMap)
(hoverPos : String.Pos)
(infoTree : InfoTree)
: Option (HoverInfo × ContextInfo × CompletionInfo) :=
(fileMap : FileMap)
(hoverPos : String.Pos)
(infoTree : InfoTree)
: Option (HoverInfo × ContextInfo × CompletionInfo) :=
let ⟨hoverLine, _⟩ := fileMap.toPosition hoverPos
match infoTree.foldInfo (init := none) (choose hoverLine) with
| some (hoverInfo, ctx, Info.ofCompletionInfo info) =>
Expand All @@ -677,7 +677,8 @@ where
(info : Info)
(best? : Option (HoverInfo × ContextInfo × Info))
: Option (HoverInfo × ContextInfo × Info) :=
if !info.isCompletion then best?
if !info.isCompletion then
best?
else if info.occursInside? hoverPos |>.isSome then
let headPos := info.pos?.get!
let ⟨headPosLine, _⟩ := fileMap.toPosition headPos
Expand All @@ -695,15 +696,14 @@ where
else if let some (HoverInfo.inside _, _, _) := best? then
-- We assume the "inside matches" have precedence over "before ones".
best?
else if let some d := info.occursBefore? hoverPos then
else if info.occursDirectlyBefore hoverPos then
let pos := info.tailPos?.get!
let ⟨line, _⟩ := fileMap.toPosition pos
if line != hoverLine then best?
else match best? with
| none => (HoverInfo.after, ctx, info)
| some (_, _, best) =>
let dBest := best.occursBefore? hoverPos |>.get!
if d < dBest || (d == dBest && info.isSmaller best) then
if info.isSmaller best then
(HoverInfo.after, ctx, info)
else
best?
Expand Down
8 changes: 4 additions & 4 deletions src/Lean/Server/InfoUtils.lean
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ def Info.isSmaller (i₁ i₂ : Info) : Bool :=
| some _, none => true
| _, _ => false

def Info.occursBefore? (i : Info) (hoverPos : String.Pos) : Option String.Pos := do
let tailPos i.tailPos?
guard (tailPos ≤ hoverPos)
return hoverPos - tailPos
def Info.occursDirectlyBefore (i : Info) (hoverPos : String.Pos) : Bool := Id.run do
let some tailPos := i.tailPos?
| return false
return tailPos == hoverPos

def Info.occursInside? (i : Info) (hoverPos : String.Pos) : Option String.Pos := do
let headPos ← i.pos?
Expand Down
60 changes: 48 additions & 12 deletions stage0/src/kernel/type_checker.cpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions stage0/src/kernel/type_checker.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions stage0/stdlib/Lean/Class.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ecf0459

Please sign in to comment.