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

fix: ignore def/theorem confusion in docBlame #269

Merged
merged 9 commits into from Sep 25, 2023

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Sep 21, 2023

Some defs are generated by core and should really be theorems; we treat them as such in the docBlame and docBlameThm linters.

This fixes #217.

Some `def`s are generated by core and should really be theorems;
we treat them as such in the `docBlame` and `docBlameThm` linters.
Std/Tactic/Lint/Misc.lean Outdated Show resolved Hide resolved
eric-wieser and others added 2 commits September 21, 2023 15:04
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@digama0
Copy link
Collaborator

digama0 commented Sep 21, 2023

test?

@eric-wieser
Copy link
Contributor Author

There was already a test for docBlame, but as usual there was a bug in the one I didn't test, docBlameThm

test/lint_docBlameThm.lean Outdated Show resolved Hide resolved
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
test/lint_docBlame.lean Outdated Show resolved Hide resolved
Std/Tactic/Lint/Misc.lean Outdated Show resolved Hide resolved
@j-loreaux
Copy link
Contributor

j-loreaux commented Sep 21, 2023

I have thoughts about this that I'm currently formulating, please don't rush to merge. retracted

@semorrison
Copy link
Collaborator

Is there an issue for this on the lean4 repository? I can't find one. It would be good to link to such an issue from the code here, even it is linking to a wontfix!

@eric-wieser
Copy link
Contributor Author

eric-wieser commented Sep 22, 2023

I'm not aware of one. I'd prefer to get this merged soon, as the sooner this is merged the sooner we stop receiving useless comments like

  /-- x is two --/
  x_is_two : x = 2

in mathlib

@digama0
Copy link
Collaborator

digama0 commented Sep 22, 2023

I think you should open a tracking issue and link to it from this PR.

Co-authored-by: Scott Morrison <scott@tqft.net>
@digama0 digama0 merged commit 538a091 into leanprover:main Sep 25, 2023
1 check passed
yuma-mizuno added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 5, 2023
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jan 18, 2024
Created by running `lake exe runLinter --update Mathlib` locally.

Most changes are due to leanprover/std4#269: proof fields in classes or structures do not require a docstring any more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docBlame linter requires docstrings on propositional structure fields
4 participants