leanprover-community / mathlib4

The math library of Lean 4
https://leanprover-community.github.io/mathlib4_docs
Apache License 2.0
1.38k stars 308 forks source link

Linter / Formatter Wishlist #7217

Open BoltonBailey opened 1 year ago

BoltonBailey commented 1 year ago

I am compiling a list of things we might like our linter / possible future code formatters / reviewdog to detect/correct/comment on. Feel free to add to this list, or augment items with related issues.

Linters

Style Guide Items

Items not in the style guide which might nevertheless be nice

Formatters

A formatter does not exist yet, but when it does, the above issues should be revisited.


(Thanks to Floris, @MohanadAhmed, Sebastien Gouezel for contributing points)

MohanadAhmed commented 12 months ago

Hello @BoltonBailey Nice list. I like the idea of a formatter that can run automatically and get rid of the small pesky annoyances during review. I am however not optimistic about such a formatter working in Lean4. This is based on the following observation:

White spacing

Here is a list I collected from comments on a PR a while ago:

  1. Variable Type spacing. i.e: white space between variable and colon, white space between colon and type, no whitespace from open bracket(brace) side, no white space from close bracket (brace) side.
    • (A: Type) to (A : Type)
    • (A:Type ) to (A : Type) ... etc.
  2. Spacing between variable declarations:
    • (A : Type)(B : Type) becomes (A : Type) (B : Type)
  3. Space between definition identifier (name) and the variable declarations
    • def myMatrix(m n : Nat): Matrix (Fin m) (Fin n) Nat becomes def myMatrix (m n : Nat) : Matrix (Fin m) (Fin n) Nat
    • Note that this is similar to rule 1 the type of the definition (includes lemmmas and theorems is also separated from the color preceding it and the colon is separated from the identifier and variables preceding it)
  4. Type of definitions and theorems, if they extend beyond the line should be indented by 4 spaces.
  5. Subgoals of suffices should be focused using \.
  6. Infix operators should have whitespace from their operands.
    • B⬝A becomes B ⬝ A
MohanadAhmed commented 12 months ago

Non-Terminal Simp

The second to last item in your current list is "Non-terminal simp". My understanding is a terminal simp is a simp without only and possibly a list of lemmas. If my understanding of this term is correct, then I have a data point against this being something a linter or formatter should warn against.

See this https://github.com/leanprover-community/mathlib4/pull/6052#discussion_r1281812334

digama0 commented 12 months ago

@MohanadAhmed A "terminal" simp means a simp at the end of a tactic block. The example in that comment is of a terminal simp. So the linter warning would be for a simp without only, which is followed by more tactics rather than being the last tactic in a sequence. There are exceptions for this too, for example maybe simp; simp or simp; ring is okay, but the allowed exceptions could presumably be added to the linter.

MohanadAhmed commented 12 months ago

@digama0 Thanks for the correction.

I am curios what is your opinion on how feasible is it to make a formatter in Lean code that runs almost instantly (say like javascript or similar in VSCode) ? Is it reasonable to expect Lean in the near future to instantly parse and reformat say 300 line file?

digama0 commented 12 months ago

Significant architectural changes are required to make a formatter which runs faster than lake build. See https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/quick.20and.20dirty.20mode for context.