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 274 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
  • special typeclass assumptions

    code -r -g  ././././Mathlib/RingTheory/WittVector/Truncated.lean:273
    code -r -g  ././././Mathlib/Topology/Category/Profinite/Product.lean:79
    
  • 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

…eanprover-community/mathlib4 into adomani/unused_variable_command_linter
…eanprover-community/mathlib4 into adomani/unused_variable_command_linter
…eanprover-community/mathlib4 into adomani/unused_variable_command_linter
Jun2M pushed a commit that referenced this pull request Nov 17, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 18, 2024
TobiasLeichtfried pushed a commit that referenced this pull request Nov 21, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 23, 2024
mathlib-bors bot pushed a commit that referenced this pull request Nov 25, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 25, 2024
@grunweg
Copy link
Collaborator

grunweg commented Jan 15, 2025

Adding this for reference: I presume the new async linting also confuses the linter, so would have to be turned off as well, right? See #20755 for reference.

@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 16, 2025
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