Repository navigation
Conversation
…rtsFromSource` `findImportsFromSource` only removed `Init` itself, but its deprecation message recommends `parseImports'` + `ModuleHeader.filterInit`, which removes the whole `Init` namespace. Anyone following the deprecation message on a file with an explicit `Init.*` import (e.g. a `prelude` file importing `Init.Data.Nat.Basic`) gets a silent change in import analysis. Since the broad filtering is the intended behavior of `filterInit` (its docstring documents it as removing every import in the `Init` namespace), this change: - extends the deprecation message to warn that the replacement is not behavior-preserving, and shows how to keep `Init.*` imports with `header.imports.filter (·.module != `Init)`; - documents the difference in the docstrings of `filterInit` and `findImportsFromSource`; - adds regression tests pinning both behaviors (`ImportGraphTest/FilterInit.lean`). Fixes leanprover-community#149
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #149.
findImportsFromSourcefiltered only exactlyInit, but its deprecation message recommendsparseImports'+ModuleHeader.filterInit, which removes the wholeInitnamespace. Anyone following the deprecation message on a file with an explicitInit.*import (e.g. apreludefile importingInit.Data.Nat.Basic, like Mathlib'sMathlib/Data/Fin/Init.lean) gets a silent change in import analysis.Of the two options in the issue, this PR implements the second: the broad behavior of
filterInitis intended — its docstring has explicitly documented "removes every import in theInitnamespace (Inititself andInit.*)" since #154 — so the remaining gap is that the deprecation path did not warn that the replacement is not behavior-preserving.Changes
@[deprecated]message onfindImportsFromSourcenow warns that the suggested replacement is not behavior-preserving, and shows how to keepInit.*imports:header.imports.filter (·.module !=Init)`.filterInitandfindImportsFromSourcenow document the difference explicitly.ImportGraphTest/FilterInit.leanpin down both behaviors:filterInitdrops an explicitInit.*import from apreludefile;filterInitdropsInititself, including the duplicated implicitInitimports that module-system files can produce, and the implicitInitof a plain header;·.module !=Init(the deprecated function's behavior) keeps explicitInit.*` imports.Verification
Locally, both against the branch's pinned toolchain (
leanprover/lean4:v4.35.0) and currentmain's (v4.36.0-rc1):lake buildpasses,lake exe graph --helpworks, andlake build ImportGraphTestpasses for every target except the pre-existingImportGraphTest.AnotherFileWithTransitiveImports, which exceeds my sandbox's memory limit (OOM-kill, exit 137) identically on unmodifiedmain, so it is unrelated to this change. The deprecation warning renders as expected. Nosorry/admitanywhere.