Skip to content

Commit 19337f2

Browse files
committed
feat(scripts/lint-style): select linters with Lakefile options (#24953)
This PR modifies the lint-style script to enable or disable specific linters by specifying them with Lean options, just like the other style linters. Linters can be selected by setting the corresponding option in the lakefile of the project. (Since we don't actually run any of the code in the project to be linted, `set_option` calls will be ignored.) More precisely, we use Lake to parse the Lean options from the root project and any default libs and executables. This should correspond to the most common way linter options are set, project-wide. This PR uses a single option to enable/disable all of the Python-based linters. Passing options to the Python program could also work, but sounds like a lot of work for something we want to get rid of in the longer term. Testing routine for this PR: ```bash touch scripts/undocumented.lean lake exe lint-style -- Receive style linter error $EDITOR lakefile.lean -- Remove the line ⟨`linter.allScriptsDocumented, true⟩, lake exe lint-style -- See that the style linter error is gone. ```
1 parent 15e31b9 commit 19337f2

File tree

4 files changed

+127
-40
lines changed

4 files changed

+127
-40
lines changed

Mathlib/Tactic/Linter/TextBased.lean

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ or for downstream projects to allow a gradual adoption of this linter.
3232
An executable running all these linters is defined in `scripts/lint-style.lean`.
3333
-/
3434

35-
open System
35+
open Lean.Linter System
3636

3737
namespace Mathlib.Linter.TextBased
3838

@@ -195,23 +195,33 @@ return an array of all style errors with line numbers. If possible,
195195
also return the collection of all lines, changed as needed to fix the linter errors.
196196
(Such automatic fixes are only possible for some kinds of `StyleError`s.)
197197
-/
198-
abbrev TextbasedLinter := Array String → Array (StyleError × ℕ) × (Option (Array String))
198+
abbrev TextbasedLinter := Lean.Options → Array String →
199+
Array (StyleError × ℕ) × (Option (Array String))
199200

200201
/-! Definitions of the actual text-based linters. -/
201202
section
202203

203204
/-- Lint on any occurrences of the string "Adaptation note:" or variants thereof. -/
204-
def adaptationNoteLinter : TextbasedLinter := fun lines ↦ Id.run do
205+
register_option linter.adaptationNote : Bool := { defValue := true }
206+
207+
@[inherit_doc linter.adaptationNote]
208+
def adaptationNoteLinter : TextbasedLinter := fun opts lines ↦ Id.run do
209+
unless getLinterValue linter.adaptationNote opts do return (#[], none)
210+
205211
let mut errors := Array.mkEmpty 0
206212
for h : idx in [:lines.size] do
207213
-- We make this shorter to catch "Adaptation note", "adaptation note" and a missing colon.
208214
if lines[idx].containsSubstr "daptation note" then
209215
errors := errors.push (StyleError.adaptationNote, idx + 1)
210216
return (errors, none)
211217

212-
213218
/-- Lint a collection of input strings if one of them contains trailing whitespace. -/
214-
def trailingWhitespaceLinter : TextbasedLinter := fun lines ↦ Id.run do
219+
register_option linter.trailingWhitespace : Bool := { defValue := true }
220+
221+
@[inherit_doc linter.trailingWhitespace]
222+
def trailingWhitespaceLinter : TextbasedLinter := fun opts lines ↦ Id.run do
223+
unless getLinterValue linter.trailingWhitespace opts do return (#[], none)
224+
215225
let mut errors := Array.mkEmpty 0
216226
let mut fixedLines : Vector String lines.size := lines.toVector
217227
for h : idx in [:lines.size] do
@@ -221,9 +231,13 @@ def trailingWhitespaceLinter : TextbasedLinter := fun lines ↦ Id.run do
221231
fixedLines := fixedLines.set idx line.trimRight
222232
return (errors, if errors.size > 0 then some fixedLines.toArray else none)
223233

224-
225234
/-- Lint a collection of input strings for a semicolon preceded by a space. -/
226-
def semicolonLinter : TextbasedLinter := fun lines ↦ Id.run do
235+
register_option linter.whitespaceBeforeSemicolon : Bool := { defValue := true }
236+
237+
@[inherit_doc linter.whitespaceBeforeSemicolon]
238+
def semicolonLinter : TextbasedLinter := fun opts lines ↦ Id.run do
239+
unless getLinterValue linter.whitespaceBeforeSemicolon opts do return (#[], none)
240+
227241
let mut errors := Array.mkEmpty 0
228242
let mut fixedLines := lines
229243
for h : idx in [:lines.size] do
@@ -256,7 +270,7 @@ def allLinters : Array TextbasedLinter := #[
256270
Return a list of all unexpected errors, and, if some errors could be fixed automatically,
257271
the collection of all lines with every automatic fix applied.
258272
`exceptions` are any pre-existing style exceptions for this file. -/
259-
def lintFile (path : FilePath) (exceptions : Array ErrorContext) :
273+
def lintFile (opts : Lean.Options) (path : FilePath) (exceptions : Array ErrorContext) :
260274
IO (Array ErrorContext × Option (Array String)) := do
261275
let mut errors := #[]
262276
-- Whether any changes were made by auto-fixes.
@@ -280,7 +294,7 @@ def lintFile (path : FilePath) (exceptions : Array ErrorContext) :
280294
let mut changed := lines
281295

282296
for lint in allLinters do
283-
let (err, changes) := lint changed
297+
let (err, changes) := lint opts changed
284298
allOutput := allOutput.append (Array.map (fun (e, n) ↦ #[(ErrorContext.mk e n path)]) err)
285299
-- TODO: auto-fixes do not take style exceptions into account
286300
if let some c := changes then
@@ -291,52 +305,65 @@ def lintFile (path : FilePath) (exceptions : Array ErrorContext) :
291305
(allOutput.flatten.filter (fun e ↦ (e.find?_comparable exceptions).isNone))
292306
return (errors, if changes_made then some changed else none)
293307

308+
/-- Enables the old Python-based style linters. -/
309+
register_option linter.pythonStyle : Bool := { defValue := true }
310+
294311
/-- Lint a collection of modules for style violations.
295312
Print formatted errors for all unexpected style violations to standard output;
296313
correct automatically fixable style errors if configured so.
297314
Return the number of files which had new style errors.
315+
`opts` contains the options defined in the Lakefile, determining which linters to enable.
298316
`nolints` is a list of style exceptions to take into account.
299317
`moduleNames` are the names of all the modules to lint,
300318
`mode` specifies what kind of output this script should produce,
301319
`fix` configures whether fixable errors should be corrected in-place. -/
302-
def lintModules (nolints : Array String) (moduleNames : Array Lean.Name) (style : ErrorFormat)
303-
(fix : Bool) : IO UInt32 := do
320+
def lintModules (opts : Lean.Options) (nolints : Array String) (moduleNames : Array Lean.Name)
321+
(style : ErrorFormat) (fix : Bool) : IO UInt32 := do
304322
let styleExceptions := parseStyleExceptions nolints
305323
let mut numberErrorFiles : UInt32 := 0
306324
let mut allUnexpectedErrors := #[]
307325
for module in moduleNames do
308326
-- Convert the module name to a file name, then lint that file.
309327
let path := mkFilePath (module.components.map toString)|>.addExtension "lean"
310328

311-
let (errors, changed) := ← lintFile path styleExceptions
329+
let (errors, changed) := ← lintFile opts path styleExceptions
312330
if let some c := changed then
313331
if fix then
314332
let _ := ← IO.FS.writeFile path ("\n".intercalate c.toList)
315333
if errors.size > 0 then
316334
allUnexpectedErrors := allUnexpectedErrors.append errors
317335
numberErrorFiles := numberErrorFiles + 1
318336

319-
-- Run the remaining python linters. It is easier to just run on all files.
320-
-- If this poses an issue, I can either filter the output
321-
-- or wait until lint-style.py is fully rewritten in Lean.
322-
let args := if fix then #["--fix"] else #[]
323-
let output ← IO.Process.output { cmd := "./scripts/print-style-errors.sh", args := args }
324-
if output.exitCode != 0 then
325-
numberErrorFiles := numberErrorFiles + 1
326-
IO.eprintln s!"error: `print-style-error.sh` exited with code {output.exitCode}"
327-
IO.eprint output.stderr
328-
else if output.stdout != "" then
329-
numberErrorFiles := numberErrorFiles + 1
330-
IO.eprint output.stdout
337+
-- Passing Lean options to Python files seems like a lot of work for something we want to
338+
-- run entirely inside of Lean in the end anyway.
339+
-- So for now, we enable/disable all of them with a single switch.
340+
if getLinterValue linter.pythonStyle opts then
341+
-- Run the remaining python linters. It is easier to just run on all files.
342+
-- If this poses an issue, I can either filter the output
343+
-- or wait until lint-style.py is fully rewritten in Lean.
344+
let args := if fix then #["--fix"] else #[]
345+
let output ← IO.Process.output { cmd := "./scripts/print-style-errors.sh", args := args }
346+
if output.exitCode != 0 then
347+
numberErrorFiles := numberErrorFiles + 1
348+
IO.eprintln s!"error: `print-style-error.sh` exited with code {output.exitCode}"
349+
IO.eprint output.stderr
350+
else if output.stdout != "" then
351+
numberErrorFiles := numberErrorFiles + 1
352+
IO.eprint output.stdout
331353
formatErrors allUnexpectedErrors style
332354
if allUnexpectedErrors.size > 0 then
333355
IO.eprintln s!"error: found {allUnexpectedErrors.size} new style error(s)"
334356
return numberErrorFiles
335357

358+
/-- Verify that all modules are named in `UpperCamelCase` -/
359+
register_option linter.modulesUpperCamelCase : Bool := { defValue := true }
360+
336361
/-- Verifies that all modules in `modules` are named in `UpperCamelCase`
337362
(except for explicitly discussed exceptions, which are hard-coded here).
338363
Return the number of modules violating this. -/
339-
def modulesNotUpperCamelCase (modules : Array Lean.Name) : IO Nat := do
364+
def modulesNotUpperCamelCase (opts : Lean.Options) (modules : Array Lean.Name) : IO Nat := do
365+
unless getLinterValue linter.modulesUpperCamelCase opts do return 0
366+
340367
-- Exceptions to this list should be discussed on zulip!
341368
let exceptions := [
342369
`Mathlib.Analysis.CStarAlgebra.lpSpace,

MathlibTest/ModuleCasing.lean

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,19 @@ open Mathlib.Linter.TextBased
88

99
/-- Some unit tests for `modulesNotUpperCamelCase` -/
1010
def testModulesNotUpperCamelCase : IO Unit := do
11-
assert!((← modulesNotUpperCamelCase #[]) == 0)
12-
assert!((← modulesNotUpperCamelCase #[`Mathlib.Fine]) == 0)
13-
assert!((← modulesNotUpperCamelCase #[`Mathlib.AlsoFine_]) == 0)
14-
assert!((← modulesNotUpperCamelCase #[`Mathlib.NotFine_.Foo]) == 1)
11+
-- Explicitly enable the linter, although it is enabled by default.
12+
let opts : Lean.Options := linter.modulesUpperCamelCase.set {} true
1513

16-
assert!((← modulesNotUpperCamelCase #[`bad_module]) == 1)
17-
assert!((← modulesNotUpperCamelCase #[`GoodName, `bad_module, `bad_module]) == 2)
18-
assert!((← modulesNotUpperCamelCase #[`Mathlib.BadModule__]) == 1)
19-
assert!((← modulesNotUpperCamelCase #[`Mathlib.lowerCase]) == 1)
20-
assert!((← modulesNotUpperCamelCase #[`Mathlib.snake_case]) == 1)
14+
assert!((← modulesNotUpperCamelCase opts #[]) == 0)
15+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.Fine]) == 0)
16+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.AlsoFine_]) == 0)
17+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.NotFine_.Foo]) == 1)
18+
19+
assert!((← modulesNotUpperCamelCase opts #[`bad_module]) == 1)
20+
assert!((← modulesNotUpperCamelCase opts #[`GoodName, `bad_module, `bad_module]) == 2)
21+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.BadModule__]) == 1)
22+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.lowerCase]) == 1)
23+
assert!((← modulesNotUpperCamelCase opts #[`Mathlib.snake_case]) == 1)
2124

2225
/--
2326
info: error: module name 'Mathlib.NotFine_.Foo' is not in 'UpperCamelCase': it should be 'Mathlib.NotFine.Foo' instead

lakefile.lean

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ require "leanprover-community" / "plausible" @ git "main"
2626
/-- These options are used as `leanOptions`, prefixed by `` `weak``, so that
2727
`lake build` uses them, as well as `Archive` and `Counterexamples`. -/
2828
abbrev mathlibOnlyLinters : Array LeanOption := #[
29+
`linter.allScriptsDocumented, true⟩,
30+
`linter.checkInitImports, true⟩,
2931
-- The `docPrime` linter is disabled: https://github.com/leanprover-community/mathlib4/issues/20560
3032
`linter.docPrime, false⟩,
3133
`linter.hashCommand, true⟩,
@@ -44,7 +46,7 @@ abbrev mathlibOnlyLinters : Array LeanOption := #[
4446
`linter.style.multiGoal, true⟩,
4547
`linter.style.openClassical, true⟩,
4648
`linter.style.refine, true⟩,
47-
`linter.style.setOption, true
49+
`linter.style.setOption, true,
4850
]
4951

5052
/-- These options are passed as `leanOptions` to building mathlib, as well as the
@@ -129,6 +131,9 @@ lean_exe shake where
129131
/-- `lake exe lint-style` runs text-based style linters. -/
130132
lean_exe «lint-style» where
131133
srcDir := "scripts"
134+
supportInterpreter := true
135+
-- Executables which import `Lake` must set `-lLake`.
136+
weakLinkArgs := #["-lLake"]
132137

133138
/--
134139
`lake exe pole` queries the Mathlib speedcenter for build times for the current commit,

scripts/lint-style.lean

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Released under Apache 2.0 license as described in the file LICENSE.
44
Authors: Michael Rothgang
55
-/
66

7+
import Lake.CLI.Main
78
import Lean.Elab.ParseImportsFast
89
import Batteries.Data.String.Basic
910
import Mathlib.Tactic.Linter.TextBased
@@ -20,7 +21,7 @@ In addition, this checks that
2021
- every file in `scripts` is documented in its top-level README.
2122
-/
2223

23-
open Cli Mathlib.Linter.TextBased System.FilePath
24+
open Cli Lean.Linter Mathlib.Linter.TextBased System.FilePath
2425

2526
/-- Parse all imports in a text file at `path` and return just their names:
2627
this is just a thin wrapper around `Lean.parseImports'`.
@@ -36,12 +37,57 @@ def explicitImports : Array Lean.Name := #[`Batteries, `Std]
3637
def eraseExplicitImports (names : Array Lean.Name) : Array Lean.Name :=
3738
explicitImports.foldl Array.erase names
3839

40+
/-- Get the root package of the Lake workspace we are running in. -/
41+
def getWorkspaceRoot : IO Lake.Package := do
42+
let (elanInstall?, leanInstall?, lakeInstall?) ← Lake.findInstall?
43+
let config ← Lake.MonadError.runEIO <| Lake.mkLoadConfig { elanInstall?, leanInstall?, lakeInstall? }
44+
let some workspace ← Lake.loadWorkspace config |>.toBaseIO
45+
| throw <| IO.userError "failed to load Lake workspace"
46+
return workspace.root
47+
48+
/-- Convert the options that Lake knows into the option that Lean knows. -/
49+
def toLeanOptions (opts : Array Lake.LeanOption) : Lean.Options :=
50+
opts.foldl (init := Lean.Options.empty) fun opts o =>
51+
-- Strip off the `weak.` prefix, like Lean does when parsing command line arguments.
52+
if o.name.getRoot == `weak
53+
then
54+
opts.insert (o.name.replacePrefix `weak Lean.Name.anonymous) o.value.toDataValue
55+
else
56+
opts.insert o.name o.value.toDataValue
57+
58+
/-- Determine the `Lean.Options` from the Lakefile of the current project.
59+
60+
We have to do this since style linters do not run in the `CoreM`/`CommandElabM` monads,
61+
and so they do not get access to the options in scope.
62+
63+
Please do not confuse this with the Lean options at the moment that `lint-style` was compiled.
64+
-/
65+
def getLakefileLeanOptions : IO Lean.Options := do
66+
let root ← getWorkspaceRoot
67+
-- Some projects declare options in the root package.
68+
let rootOpts := root.leanOptions
69+
-- Other projects, like Mathlib, declare options in the targets.
70+
-- Here we use the default targets, since that probably contains the modules we'll be linting.
71+
let defaultOpts := root.defaultTargets.flatMap fun target ↦
72+
if let some lib := root.findLeanLib? target then
73+
lib.config.leanOptions
74+
else if let some exe := root.findLeanExe? target then
75+
exe.config.leanOptions
76+
else
77+
#[]
78+
return toLeanOptions (rootOpts ++ defaultOpts)
79+
80+
/-- Check that `Mathlib.Init` is transitively imported in all of Mathlib -/
81+
register_option linter.checkInitImports : Bool := { defValue := false }
82+
3983
/-- Check that `Mathlib.Init` is transitively imported in all of Mathlib.
4084
Moreover, every file imported in `Mathlib.Init` should in turn import the `Header` linter
4185
(except for the header linter itself, of course).
4286
Return the number of modules which violated one of these rules.
4387
-/
44-
def missingInitImports : IO Nat := do
88+
def missingInitImports (opts : Lean.Options) : IO Nat := do
89+
unless getLinterValue linter.checkInitImports opts do return 0
90+
4591
-- Find any file in the Mathlib directory which does not contain any Mathlib import.
4692
-- We simply parse `Mathlib.lean`, as CI ensures this file is up to date.
4793
let allModuleNames := eraseExplicitImports (← findImports "Mathlib.lean")
@@ -82,10 +128,14 @@ def missingInitImports : IO Nat := do
82128
return missing.size
83129
return 0
84130

131+
/-- Verify that every file in the `scripts` directory is documented in `scripts/README.md` -/
132+
register_option linter.allScriptsDocumented : Bool := { defValue := false }
85133

86134
/-- Verifies that every file in the `scripts` directory is documented in `scripts/README.md`.
87135
Return the number of undocumented scripts. -/
88-
def undocumentedScripts : IO Nat := do
136+
def undocumentedScripts (opts : Lean.Options) : IO Nat := do
137+
unless getLinterValue linter.allScriptsDocumented opts do return 0
138+
89139
-- Retrieve all scripts (except for the `bench` directory).
90140
let allScripts ← (walkDir "scripts" fun p ↦ pure (p.components.getD 1 "" != "bench"))
91141
let allScripts := allScripts.erase ("scripts" / "bench")|>.erase ("scripts" / "README.md")
@@ -105,6 +155,8 @@ def undocumentedScripts : IO Nat := do
105155

106156
/-- Implementation of the `lint-style` command line program. -/
107157
def lintStyleCli (args : Cli.Parsed) : IO UInt32 := do
158+
let opts ← getLakefileLeanOptions
159+
108160
let style : ErrorFormat := match args.hasFlag "github" with
109161
| true => ErrorFormat.github
110162
| false => ErrorFormat.humanReadable
@@ -124,8 +176,8 @@ def lintStyleCli (args : Cli.Parsed) : IO UInt32 := do
124176
-- (For syntax linters, such a bug actually occurred in mathlib.)
125177
-- This script is re-run each time, hence is immune to such issues.
126178
let nolints ← IO.FS.lines ("scripts" / "nolints-style.txt")
127-
let numberErrors := (← lintModules nolints allModuleNames style fix)
128-
+ (← missingInitImports) + (← undocumentedScripts) + (← modulesNotUpperCamelCase allModuleNames)
179+
let numberErrors := (← lintModules opts nolints allModuleNames style fix)
180+
+ (← missingInitImports opts) + (← undocumentedScripts opts) + (← modulesNotUpperCamelCase opts allModuleNames)
129181
-- If run with the `--fix` argument, return a zero exit code.
130182
-- Otherwise, make sure to return an exit code of at most 125,
131183
-- so this return value can be used further in shell scripts.

0 commit comments

Comments
 (0)