Skip to content

feat: add a syntax linter version of the impossibleInstance and nonClassInstance linters #1717

Open
MoritzBeroRoos wants to merge 77 commits intoleanprover-community:mainfrom
MoritzBeroRoos:convert_linter
Open

feat: add a syntax linter version of the impossibleInstance and nonClassInstance linters #1717
MoritzBeroRoos wants to merge 77 commits intoleanprover-community:mainfrom
MoritzBeroRoos:convert_linter

Conversation

@MoritzBeroRoos
Copy link
Copy Markdown
Contributor

Currently we have the impossibleInstance and nonClassInstance linters which are environment linters in batteries.

  • impossibleInstance warns when it detects an instance with arguments that are not inferable for typeclass synthesis.
  • nonClassInstance warns when a non-class-valued function is declared as instance.

These linters are especially useful for beginners who don't yet understand how typeclass synthesis works.
But since these linters currently are environment linters they only fire in CI or when manually using #lint.
However beginners don't typically have CI set up for their pet projects and mostly don't know about #lint.

This PR adresses this issue by adding syntax linter versions of these two linters.
These will automatically check every declaration and thus be of much more use to beginners.

@github-actions github-actions bot added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Mar 14, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 14, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 14, 2026
Copy link
Copy Markdown
Collaborator

@fgdorais fgdorais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice, thanks!

@MoritzBeroRoos
Copy link
Copy Markdown
Contributor Author

One question i have is:
Do syntax linters also run in CI?
If yes, do we delete the environment linters?

mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 14, 2026
Co-authored-by: François G. Dorais <fgdorais@gmail.com>
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 16, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 16, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Mar 16, 2026
@MoritzBeroRoos MoritzBeroRoos requested a review from fgdorais March 16, 2026 20:59
thorimur and others added 2 commits April 5, 2026 14:53
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Apr 6, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Apr 6, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Apr 6, 2026
@MoritzBeroRoos
Copy link
Copy Markdown
Contributor Author

MoritzBeroRoos commented Apr 6, 2026

Sorry, i had a very busy last week and could only return to this today.

Thank you @thorimur, your rewrite is very nice!
I have merged your rewrite and benched the new version here
TLDR: It is about 2.5x the speed of my original approach!

I have added @thorimur to the authors of the linter file.

@fgdorais
Copy link
Copy Markdown
Collaborator

fgdorais commented Apr 6, 2026

There is no mechanism for delegation in batteries, but I informally delegate this PR to @thorimur and @Vierkantor. Once you both approve this PR, notify me and I will do a final check (just to be sure, I trust you!) before merging. Thanks for your generous help! (I could use more of that help from time to time 😃 )

@mathlib-merge-conflicts mathlib-merge-conflicts bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Apr 7, 2026
@thorimur
Copy link
Copy Markdown
Contributor

thorimur commented Apr 7, 2026

P.S. I think the merge conflict here might be most easily fixed by merging the branch of #1754 into this PR, which resolves the merge conflict from #1734 landing :) (Ideally we should also probably merge #1754 into main first! :) )

mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Apr 7, 2026
mathlib-nightly-testing bot added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Apr 7, 2026
@mathlib-merge-conflicts mathlib-merge-conflicts bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review This PR is ready for review; the author thinks it is ready to be merged. breaks-mathlib

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants