Skip to content

Commit

Permalink
Kim's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
adomani committed Oct 28, 2024
1 parent 6944589 commit 29ea27e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 21 deletions.
27 changes: 14 additions & 13 deletions Mathlib/Tactic/Linter/MinImports.lean
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ namespace Mathlib.Linter

/--
`ImportState` is the structure keeping track of the data that the `minImports` linter uses.
* `ig` is the import graph of the current file.
* `minImps` is the `NameSet` of minimal imports to build the file up to the current command.
* `impsSize` is the number of transitive imports to build the file up to the current command.
* `importGraph` is the import graph of the current file.
* `minImports` is the `NameSet` of minimal imports to build the file up to the current command.
* `importSize` is the number of transitive imports to build the file up to the current command.
-/
structure ImportState where
ig : Option (NameMap (Array Name)) := none
minImps : NameSet := {}
impsSize : Nat := 0
importGraph : Option (NameMap (Array Name)) := none

Check failure on line 38 in Mathlib/Tactic/Linter/MinImports.lean

View workflow job for this annotation

GitHub Actions / Build

Mathlib.Linter.ImportState.importGraph definition missing documentation string
minImports : NameSet := {}

Check failure on line 39 in Mathlib/Tactic/Linter/MinImports.lean

View workflow job for this annotation

GitHub Actions / Build

Mathlib.Linter.ImportState.minImports definition missing documentation string
importSize : Nat := 0

Check failure on line 40 in Mathlib/Tactic/Linter/MinImports.lean

View workflow job for this annotation

GitHub Actions / Build

Mathlib.Linter.ImportState.importSize definition missing documentation string
deriving Inhabited

/--
Expand Down Expand Up @@ -83,9 +83,10 @@ def minImportsLinter : Linter where run := withSetOptionIn fun stx ↦ do
let env ← getEnv
-- the first time `minImportsRef` is read, it has `ig = none`; in this case, we set it to
-- be the `importGraph` for the file.
if (← minImportsRef.get).ig.isNone then minImportsRef.modify ({· with ig := env.importGraph})
if (← minImportsRef.get).importGraph.isNone then
minImportsRef.modify ({· with importGraph := env.importGraph})
let impState ← minImportsRef.get
let (importsSoFar, oldCumulImps) := (impState.minImps, impState.impsSize)
let (importsSoFar, oldCumulImps) := (impState.minImports, impState.importSize)
-- when the linter reaches the end of the file or `#exit`, it gives a report
if #[``Parser.Command.eoi, ``Lean.Parser.Command.exit].contains stx.getKind then
let explicitImportsInFile : NameSet :=
Expand Down Expand Up @@ -117,14 +118,14 @@ def minImportsLinter : Linter where run := withSetOptionIn fun stx ↦ do
let currImpArray := currImports.toArray.qsort Name.lt
if currImpArray != #[] &&
currImpArray ≠ importsSoFar.toArray.qsort Name.lt then
let newCumulImps := (importsBelow env.importGraph tot).size
minImportsRef.set {minImps := currImports, impsSize := newCumulImps}
let newCumulImps := (importsBelow (impState.importGraph.getD env.importGraph) tot).size
minImportsRef.modify ({· with minImports := currImports, importSize := newCumulImps})
let new := currImpArray.filter (!importsSoFar.contains ·)
let redundant := importsSoFar.toArray.filter (!currImports.contains ·)
Linter.logLint linter.minImports stx
Linter.logLint linter.minImports stx <|
m!"Imports increased by {newCumulImps - oldCumulImps} to\n{currImpArray}\n\n\
Now redundant: {redundant}\n\n\
New imports: {new}\n"
New imports: {new}\n" ++
if redundant.isEmpty then m!"" else m!"\nNow redundant: {redundant}\n"

initialize addLinter minImportsLinter

Expand Down
10 changes: 2 additions & 8 deletions test/MinImports.lean
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ lemma hi (n : ℕ) : n = n := by extract_goal; rfl
warning: Imports increased by 398 to
[Init.Guard, Mathlib.Data.Int.Notation]
Now redundant: []
New imports: [Init.Guard, Mathlib.Data.Int.Notation]
note: this linter can be disabled with `set_option linter.minImports false`
Expand All @@ -100,8 +98,6 @@ set_option linter.minImports false in
warning: Imports increased by 398 to
[Init.Guard, Mathlib.Data.Int.Notation]
Now redundant: []
New imports: [Init.Guard, Mathlib.Data.Int.Notation]
note: this linter can be disabled with `set_option linter.minImports false`
Expand All @@ -120,8 +116,6 @@ set_option linter.minImports true
warning: Imports increased by 967 to
[Mathlib.Tactic.Linter.MinImports]
Now redundant: []
New imports: [Mathlib.Tactic.Linter.MinImports]
note: this linter can be disabled with `set_option linter.minImports false`
Expand All @@ -133,10 +127,10 @@ note: this linter can be disabled with `set_option linter.minImports false`
warning: Imports increased by 424 to
[Mathlib.Tactic.FunProp.Attr, Mathlib.Tactic.NormNum.Basic]
Now redundant: [Mathlib.Tactic.Linter.MinImports]
New imports: [Mathlib.Tactic.FunProp.Attr, Mathlib.Tactic.NormNum.Basic]
Now redundant: [Mathlib.Tactic.Linter.MinImports]
note: this linter can be disabled with `set_option linter.minImports false`
-/
#guard_msgs in
Expand Down

0 comments on commit 29ea27e

Please sign in to comment.