Skip to content

Commit b835401

Browse files
grunwegjoneugster
andcommitted
refactor(lint-style): make sure to read the nolints file outside of t… (#19384)
…he linter implementation This way, we categorically prevent cache invalidation bugs caused by lake replaying unchanged inputs, and thus not reaction to changes in the linter exceptions. For *syntax linters*, such a bug actually occurred in mathlib. (With the current implementation of text-based linters, this issue won't occur; this is merely a defensive change.) Suggested in #19275. Co-authored-by: Jon Eugster <eugster.jon@gmail.com>
1 parent dd2e6e3 commit b835401

File tree

2 files changed

+13
-6
lines changed

2 files changed

+13
-6
lines changed

Mathlib/Tactic/Linter/TextBased.lean

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,13 @@ def lintFile (path : FilePath) (exceptions : Array ErrorContext) :
276276
Print formatted errors for all unexpected style violations to standard output;
277277
correct automatically fixable style errors if configured so.
278278
Return the number of files which had new style errors.
279-
`moduleNames` are all the modules to lint,
279+
`nolints` is a list of style exceptions to take into account.
280+
`moduleNames` are the names of all the modules to lint,
280281
`mode` specifies what kind of output this script should produce,
281282
`fix` configures whether fixable errors should be corrected in-place. -/
282-
def lintModules (moduleNames : Array Lean.Name) (style : ErrorFormat) (fix : Bool) : IO UInt32 := do
283-
-- Read the `nolints` file, with manual exceptions for the linter.
284-
let nolints ← IO.FS.lines ("scripts" / "nolints-style.txt")
283+
def lintModules (nolints : Array String) (moduleNames : Array Lean.Name) (style : ErrorFormat)
284+
(fix : Bool) : IO UInt32 := do
285285
let styleExceptions := parseStyleExceptions nolints
286-
287286
let mut numberErrorFiles : UInt32 := 0
288287
let mut allUnexpectedErrors := #[]
289288
for module in moduleNames do

scripts/lint-style.lean

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,15 @@ def lintStyleCli (args : Cli.Parsed) : IO UInt32 := do
109109
allModuleNames := allModuleNames.append (← findImports s)
110110
-- Note: since "Batteries" is added explicitly to "Mathlib.lean", we remove it here manually.
111111
allModuleNames := allModuleNames.erase `Batteries
112-
let mut numberErrors ← lintModules allModuleNames style fix
112+
113+
-- Read the `nolints` file, with manual exceptions for the linter.
114+
-- NB. We pass these lints to `lintModules` explicitly to prevent cache invalidation bugs:
115+
-- if the text-based linter read the file itself, replaying a cached build of that
116+
-- file could re-use an outdated version of the nolints file.
117+
-- (For syntax linters, such a bug actually occurred in mathlib.)
118+
-- This script is re-run each time, hence is immune to such issues.
119+
let nolints ← IO.FS.lines ("scripts" / "nolints-style.txt")
120+
let mut numberErrors ← lintModules nolints allModuleNames style fix
113121
if ← checkInitImports then numberErrors := numberErrors + 1
114122
if !(← allScriptsDocumented) then numberErrors := numberErrors + 1
115123
-- If run with the `--fix` argument, return a zero exit code.

0 commit comments

Comments
 (0)