Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: structure auto-completion & partial InfoTrees #5835

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

mhuisi
Copy link
Contributor

@mhuisi mhuisi commented Oct 25, 2024

This PR adds auto-completion for the fields of structure instance notation. Specifically, querying the completions via Ctrl+Space in the whitespace of a structure instance notation will now bring up the full list of fields. Whitespace structure completion can be enabled for custom syntax by wrapping the parser for the list of fields in a structInstFields parser.

This PR also makes completions of partial structure fields in the bracketed structure instance notation work again (e.g. completing a in { a }). These were previously broken by the introduction of set notation.

In order to facilitate language server support for ambiguous syntax where all associated elaborators fail, this PR introduces the notion of an InfoTree choice node and an InfoTree PartialTermInfo node. Choice nodes are produced by the elaborator when several overloaded elaborators fail and contain the InfoTrees of each failed elaborator as subtrees. PartialTermInfos are generated when an individual elaborator fails to produce an Expr so that the subtrees produced by that elaborator can be retained as children of that PartialTermInfo.

Since the InfoTree contains more similar CompletionInfos after the introduction of choice nodes, this PR reworks the CompletionInfo selection to be more effective at selecting the best CompletionInfo amongst multiple similar ones.

Finally, this PR splits Completion.lean into several files and fixes #5807.

Breaking changes

  • Lean.Elab.withInfoContext' and Lean.Elab.Term.withInfoContext' gained a new parameter mkInfoOnError that is used to produce an InfoTree node when the elaborator x fails. In cases where mkInfo produces a TermInfo using Lean.Elab.Term.mkTermInfo, Lean.Elab.Term.withTermInfoContext' can be used instead of Lean.Elab.Term.withInfoContext'.
  • Lean.Elab.Info gained two new alternatives: ofPartialTermInfo and ofChoiceInfo.
  • The id field of the fieldId alternative of Lean.Elab.CompletionInfo has been turned into an Option with none representing a full field completion.
  • The syntax of structInst and whereStructInst have been adjusted to use the structInstFields parser.

Details of other changes

  • Init/Prelude.lean and Init/Meta.lean: Add two new functions getTrailing? and getTrailingTailPos? to work with the trailing whitespace of a SourceInfo / Syntax.
  • Lean/Elab/App.lean:
    • Adjust dot completion of a.b.c so that the CompletionInfos for a.b.c reference the syntax a, a.b and a.b.c respectively, instead of all referencing a.b.c. Before, these CompletionInfos were indistinguishable and we only chose the right one by accident.
    • Adjust mergeFailures to produce an InfoTree with an ofChoiceInfo root node and the InfoTrees of all failed elaborators as subtrees instead of throwing them away.
  • Lean/Elab/BuiltinTerm.lean: Adjust elabCompletion so that it always produces an identifier CompletionInfo, even when elaborating the left hand side of a dot completion succeeds and we also add a dot CompletionInfo to the InfoTree. With the changes to the CompletionInfo selection in the language server, the language server will now always pick the correct one of these two. Before, we had a special case in the language server dot completion where under certain circumstances we would treat it a dot completion as an identifier completion instead - with this change, this is now not necessary anymore.
  • Lean/Elab/MutualDef.lean: Adjust expandWhereStructInst to set reasonable SourceInfos for the produced bracketed structure instance notation. The structure field completion benefits from this SourceInfo.
  • Lean/Server/RequestHandling.lean: Adjust handleCompletion so that in the - completion case, we also emit CompletionInfoData. This fixes PANIC at Lean.Lsp.CompletionItem.getFileSource! Lean.Server.CompletionItemData:34:22: no data param on completion item  #5807.

@mhuisi mhuisi requested a review from Kha October 25, 2024 09:24
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Oct 25, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 25, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f0c190239a5e2d7f99413da0c54a575a0d0353b6 --onto 193b6f2bec332ac0bce33e10856a96163d4be456. (2024-10-25 09:39:39)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f0c190239a5e2d7f99413da0c54a575a0d0353b6 --onto 4ee44ceb1d77f16b9b81bc655bc8b318cd6e8c4d. (2024-10-30 09:18:51)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f0c190239a5e2d7f99413da0c54a575a0d0353b6 --onto 38c39482f40536b864a9b43c718e10e8413b26e5. (2024-10-30 15:00:29)
  • 💥 Mathlib branch lean-pr-testing-5835 build failed against this PR. (2024-11-18 10:47:45) View Log
  • 💥 Mathlib branch lean-pr-testing-5835 build failed against this PR. (2024-11-18 13:17:11) View Log
  • ✅ Mathlib branch lean-pr-testing-5835 has successfully built against this PR. (2024-11-18 16:03:19) View Log

@digama0
Copy link
Collaborator

digama0 commented Oct 25, 2024

Does this work after def foo : MyStruct where and package foo where?

@mhuisi
Copy link
Contributor Author

mhuisi commented Oct 25, 2024

Does this work after def foo : MyStruct where and package foo where?

"Yes" to the first question, see the test file: https://github.com/mhuisi/lean4/blob/mhuisi/constructor-field-completion-2/tests/lean/interactive/completionStructureInstance.lean

It doesn't work in the second case because Lake config declarations are a separate command with its own syntax and its own elaborator. For now, whitespace completions are specific to certain syntaxes (before macro expansion).

@digama0
Copy link
Collaborator

digama0 commented Oct 25, 2024

Does this make it easy to implement the corresponding feature in lake? This has been a long standing feature request.

@mhuisi
Copy link
Contributor Author

mhuisi commented Oct 25, 2024

The whitespace completion triggers on the Lean.Parser.Command.whereStructInst parser. I don't know whether it makes sense for Lake to move its config declarations to this parser. (cc @tydeu)

@tydeu
Copy link
Member

tydeu commented Oct 28, 2024

@mhuisi It don't think using whereStructInst directly is likely viable in Lake. Is there some standard way to emulate this behavior in a different elaborator?

@mhuisi
Copy link
Contributor Author

mhuisi commented Oct 28, 2024

@mhuisi It don't think using whereStructInst directly is likely viable in Lake. Is there some standard way to emulate this behavior in a different elaborator?

No, there is not.

@tydeu
Copy link
Member

tydeu commented Oct 28, 2024

@mhuisi Ah, so this new completion is hardcoded to the parser and is not a matter of creating the right info-tree nodes?

(I guess that is what you meant by "whitespace completions are specific to certain syntax" -- I was not fully sure whether that was simply due to support rather than hardcoding.)

@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch from 13ae691 to f1801c5 Compare October 30, 2024 14:40
@mhuisi
Copy link
Contributor Author

mhuisi commented Oct 30, 2024

@tydeu @digama0 I've adjusted the implementation so that Lake can enable support for whitespace structure completion.

src/Lean/Elab/App.lean Outdated Show resolved Hide resolved
src/Lean/Elab/Extra.lean Outdated Show resolved Hide resolved
src/Lean/Elab/Term.lean Outdated Show resolved Hide resolved
src/Lean/Parser/Term.lean Outdated Show resolved Hide resolved
@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch from 0901c0e to ec0bf8c Compare November 6, 2024 17:16
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 7, 2024
@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch 2 times, most recently from e0d72c6 to b3bff80 Compare November 8, 2024 15:27
@mhuisi mhuisi added the changelog-server Language server, widgets, and IDE extensions label Nov 8, 2024
@Kha Kha requested a review from kim-em as a code owner November 11, 2024 17:07
@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch from cf91976 to bc8d566 Compare November 15, 2024 17:12
@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch from a33afd6 to 357faba Compare November 18, 2024 10:20
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 18, 2024
@leanprover-community-bot leanprover-community-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Nov 18, 2024
@mhuisi mhuisi force-pushed the mhuisi/constructor-field-completion-2 branch from 357faba to 1b7bb15 Compare November 18, 2024 12:48
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 18, 2024
mhuisi added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 18, 2024
@leanprover-community-bot leanprover-community-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Nov 18, 2024
@Kha Kha merged commit 4bef358 into leanprover:master Nov 19, 2024
19 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
#5835 contains a brittle test that uses an FVar ID, which caused a
failure on master. This PR changes that test to use a declaration
instead.
github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
This PR fixes a regression where goals that don't exist were being
displayed. The regression was triggered by #5835 and originally caused
by #4926.

Bug originally reported at
https://leanprover.zulipchat.com/#narrow/channel/270676-lean4/topic/tactic.20doesn't.20change.20primary.20goal.20state/near/488957772.

The cause of this issue was that #5835 made certain `SourceInfo`s
canonical, which was directly transferred to several `TacticInfo`s by
#4926. The goal state selection mechanism would then pick up these extra
`TacticInfo`s.

The approach taken by this PR is to ensure that the `SourceInfo` that is
being transferred by #4926 is noncanonical.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR changelog-server Language server, widgets, and IDE extensions toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
5 participants