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: a non-terminal simp linter #11246

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Mathlib.lean
Original file line number Diff line number Diff line change
Expand Up @@ -3593,6 +3593,7 @@ import Mathlib.Tactic.Monotonicity.Attr
import Mathlib.Tactic.Monotonicity.Basic
import Mathlib.Tactic.Monotonicity.Lemmas
import Mathlib.Tactic.MoveAdd
import Mathlib.Tactic.NonTerminalSimp
import Mathlib.Tactic.NoncommRing
import Mathlib.Tactic.Nontriviality
import Mathlib.Tactic.Nontriviality.Core
Expand Down
1 change: 1 addition & 0 deletions Mathlib/Tactic.lean
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import Mathlib.Tactic.Monotonicity.Attr
import Mathlib.Tactic.Monotonicity.Basic
import Mathlib.Tactic.Monotonicity.Lemmas
import Mathlib.Tactic.MoveAdd
import Mathlib.Tactic.NonTerminalSimp
import Mathlib.Tactic.NoncommRing
import Mathlib.Tactic.Nontriviality
import Mathlib.Tactic.Nontriviality.Core
Expand Down
1 change: 1 addition & 0 deletions Mathlib/Tactic/Basic.lean
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Std
import Mathlib.Tactic.PPWithUniv
import Mathlib.Tactic.ExtendDoc
import Mathlib.Tactic.Lemma
import Mathlib.Tactic.NonTerminalSimp
import Mathlib.Tactic.TypeStar

set_option autoImplicit true
Expand Down
93 changes: 93 additions & 0 deletions Mathlib/Tactic/NonTerminalSimp.lean
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/-
Copyright (c) 2024 Damiano Testa. All rights reserved.
Released under Apache 2.0 license as described in the file LICENSE.
Authors: Damiano Testa
-/
import Lean.Elab.Command
import Lean.Linter.Util
import Std.Data.Array.Basic

/-!
# The non-terminal `simp` linter

The non-terminal `simp` linter makes sure that `simp` is not used as a finishing tactic.
If you want to use `simp [...]` followed by other tactics, then replace `simp [...]` by
* a `suffices \"expr after simp\" by simpa` line;
* the output of `simp? [...]`, so that the final code contains `simp only [...]`;
* something else that does not involve `simp`!

The linter equates "non-terminal" with "does not strictly decrease the number of goals".

## Implementation detail

The code in this linter is just a very small modification of the code for the
`unreachableTactic` linter.
-/

open Lean Elab

namespace Mathlib.Linter

/-- The non-terminal `simp` linter makes sure that `simp` is not used as a finishing tactic. -/
register_option linter.nonTerminalSimp : Bool := {
defValue := true
descr := "enable the 'non-terminal `simp`' linter"
}

namespace nonTerminalSimp

/-- `onlyOrNotSimp stx` if `stx` is syntax for `simp` *without* `only`, then returns `false` else
returs `true`. -/
def onlyOrNotSimp : Syntax → Bool
| .node _info `Lean.Parser.Tactic.simp #[_, _, _, only?, _, _] => only?[0].getAtomVal == "only"
| _ => true

/-- The monad for collecting `simp`s that are not `simp only`. -/
abbrev M := StateRefT (HashMap String.Range Syntax) IO

mutual
/-- Search for `simp`s that
* are not `simp only` and
* do not close a goal.

Add such `simp`s to the state. -/
partial def addNonSimpOnlysList (trees : PersistentArray InfoTree) : M Unit :=
trees.forM addNonSimpOnlys

@[inherit_doc addNonSimpOnlysList]
partial def addNonSimpOnlys : InfoTree → M Unit
| .node i c => do
if let .ofTacticInfo i := i then
let non_terminal_simp? := (! onlyOrNotSimp i.stx) &&
(! i.goalsAfter.length < i.goalsBefore.length)
match i.stx.getRange? true, non_terminal_simp? with
| some r, true => modify (·.insert r i.stx)
| _, _ => pure ()
addNonSimpOnlysList c
| .context _ t => addNonSimpOnlys t
| .hole _ => pure ()

end

/-- Gets the value of the `linter.nonTerminalSimp` option. -/
def getLinterHash (o : Options) : Bool := Linter.getLinterValue linter.nonTerminalSimp o

/-- The main entry point to the unreachable tactic linter. -/
def nonTerminalSimpLinter : Linter where run := withSetOptionIn fun _stx => do
unless getLinterHash (← getOptions) && (← getInfoState).enabled do
return
if (← get).messages.hasErrors then
return
let (_, map) ← (addNonSimpOnlysList (← getInfoTrees)).run {}
let simps := map.toArray
let key (r : String.Range) := (r.start.byteIdx, (-r.stop.byteIdx : Int))
let mut last : String.Range := ⟨0, 0⟩
for (r, stx) in let _ := @lexOrd; let _ := @ltOfOrd.{0}; simps.qsort (key ·.1 < key ·.1) do
if last.start ≤ r.start && r.stop ≤ last.stop then continue
Linter.logLint linter.nonTerminalSimp stx
"non-terminal simp: consider replacing it with\n\
* `suffices \"expr after simp\" by simpa`\n\
* the output of `simp?`\n"
last := r

initialize addLinter nonTerminalSimpLinter
8 changes: 4 additions & 4 deletions scripts/noshake.json
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@
["Mathlib.CategoryTheory.Functor.Basic"],
"Mathlib.Tactic.CategoryTheory.Coherence":
["Mathlib.Tactic.CategoryTheory.MonoidalComp"],
"Mathlib.Tactic.Basic": ["Std"],
"Mathlib.Tactic.Basic": ["Mathlib.Tactic.NonTerminalSimp", "Std"],
"Mathlib.Tactic.Attr.Register": ["Lean.Meta.Tactic.Simp.SimpTheorems"],
"Mathlib.Tactic.ArithMult": ["Mathlib.Tactic.ArithMult.Init"],
"Mathlib.RingTheory.Subsemiring.Order":
Expand All @@ -299,9 +299,9 @@
"Mathlib.Logic.Equiv.Defs": ["Mathlib.Init.Data.Bool.Lemmas"],
"Mathlib.LinearAlgebra.Matrix.Transvection": ["Mathlib.Data.Matrix.DMatrix"],
"Mathlib.LinearAlgebra.AffineSpace.Basic": ["Mathlib.Algebra.AddTorsor"],
"Mathlib.Lean.Expr.Basic": ["Std.Logic"],
"Mathlib.Lean.Expr.ExtraRecognizers": ["Mathlib.Data.Set.Defs"],
"Mathlib.Lean.Meta": ["Std.Logic"],
"Mathlib.Lean.Expr.ExtraRecognizers": ["Mathlib.Data.Set.Defs"],
"Mathlib.Lean.Expr.Basic": ["Std.Logic"],
"Mathlib.Init.Data.Nat.GCD": ["Std.Data.Nat.Gcd"],
"Mathlib.Init.Data.Int.Basic": ["Std.Data.Int.Order"],
"Mathlib.Init.Core": ["Std.Classes.Dvd"],
Expand Down Expand Up @@ -331,4 +331,4 @@
"Mathlib.Algebra.Category.Ring.Basic":
["Mathlib.CategoryTheory.ConcreteCategory.ReflectsIso"],
"Mathlib.Algebra.Algebra.Subalgebra.Order":
["Mathlib.Algebra.Module.Submodule.Order"]}}
["Mathlib.Algebra.Module.Submodule.Order"]}}
Loading