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: the unusedVariableCommand linter #17715

Draft
wants to merge 211 commits into
base: master
Choose a base branch
from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Oct 14, 2024

This is still work-in-progress, but also help-wanted!

The linter reliably flags unused variables: so far, only examples fool it (and that is simply because I did not implement a fix, as there are very few variables that are only used in examples in mathlib).

The work-in-progress part refers to the fact that the linter uses an IO.Ref to keep track of variables and this does not work well with editing the file. The linter works on a fresh parse of a file, but becomes out-of-sync with every edit.

The help-wanted is to ask for help to make mathlib compliant with the linter, by checking out this branch and PR-ing a few variable removals!

Zulip discussion


Known issues:

  • automatic namespacing and nonrec may cause difficulty when the newly introduced declaration is preferred in the term, rather than the original one (see examples here

    code -r -g  ././././Mathlib/Analysis/SpecialFunctions/Arsinh.lean:183
    code -r -g  ././././Mathlib/Probability/Independence/Basic.lean:368

    where, for instance, ContinuousOn.arsinh is used internally instead of Real.arsinh).

  • mutual declarations confuse the linter

    code -r -g  ././././Mathlib/SetTheory/Lists.lean:313
  • code -r -g Mathlib/CategoryTheory/EffectiveEpi/Comp.lean:133:29

  • universe annotations confuse the linter:

    code -r -g  ././././Mathlib/Topology/Category/CompactlyGenerated.lean:53
    code -r -g  ././././Mathlib/AlgebraicTopology/CechNerve.lean:46
  • inductives confusing the linter

    code -r -g  ././././Mathlib/Deprecated/Subgroup.lean:389
    code -r -g  ././././Mathlib/FieldTheory/PerfectClosure.lean:55
  • defs confusing the linter

    code -r -g  ././././Mathlib/Data/MLList/BestFirst.lean:104
  • not sure what is going on here:

    code -r -g  ././././Mathlib/RingTheory/WittVector/IsPoly.lean:95
    code -r -g  ././././Mathlib/Probability/Independence/Basic.lean:368
    code -r -g  ././././Mathlib/Analysis/Calculus/FDeriv/Star.lean:30

Open in Gitpod

mathlib-bors bot pushed a commit that referenced this pull request Oct 24, 2024
This is just the removal of the variable `D`, followed by the renaming of `E --> D`, `F --> E`, `G --> F`, all happening automatically.

#17715
mathlib-bors bot pushed a commit that referenced this pull request Oct 25, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 25, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 28, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large-import Automatically added label for PRs with a significant increase in transitive imports t-linter Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants