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

Linting tools #687

Closed
cipher1024 opened this issue Feb 5, 2019 · 12 comments
Closed

Linting tools #687

cipher1024 opened this issue Feb 5, 2019 · 12 comments
Assignees
Labels
RFC Request for comment

Comments

@cipher1024
Copy link
Collaborator

The goal is to sanitize the mathlib code base but also, the code base of any project depending on it.

Here is a short list of code smells that can be detected automatically:

  • unnecessary imports
  • unused parameters or assumption
  • unused lemmas
  • overlapping type class instances
  • simp rules that may cause simp loops

These could be implemented by a combination of @digama0's olean parser and some Lean meta programming.

@cipher1024 cipher1024 added the RFC Request for comment label Feb 5, 2019
@cipher1024 cipher1024 self-assigned this Feb 5, 2019
@cipher1024
Copy link
Collaborator Author

@spl, @robertylewis, @rwbarton, @digama0, @johoelzl any thoughts?

@spl
Copy link
Collaborator

spl commented Feb 18, 2019

It all sounds good to me!

@jcommelin
Copy link
Member

Another thing that can be detected: duplicate lemmas

@cipher1024
Copy link
Collaborator Author

Would you recommend detecting duplicata via syntactic equality modulo variable renaming?

@jcommelin
Copy link
Member

I have no clue what is best... if the versions are next to each other, and one has explicit arguments and the other doesn't, maybe it is on purpose. But if they are in different files... it probably is a mistake. So we need some heuristics...

@cipher1024
Copy link
Collaborator Author

I just built a tool for detecting spurious imports using @digama0's parser: https://github.com/cipher1024/olean-rs

I used it to remove quite a few unneeded imports: #878

@PatrickMassot
Copy link
Member

Is there any documentation?

@cipher1024
Copy link
Collaborator Author

not much for now except when you call olean-rs -h:

Usage: olean-rs path/to/file.olean [options]

Options:
    -D, --dump FILE     dump olean parse
    -L                  give location of lean library
    -d, --deps lean.name
                        view all dependents of the target file
    -p DIR              set current working directory
    -u, --unused lean.name
                        list unused imports
    -l lean.name        test lexer
    -t lean.name        testing
    -m, --makefile      generate a makefile to build project
    -h, --help          print this help menu

@cipher1024
Copy link
Collaborator Author

What I've been doing in mathlib/src is lean-rs -m and then make all -j 3. It builds every file in mathlib and, every time, it checks the imports. It has the benefit that, if you change the imports, you can pick up right where you left off after.

@cipher1024
Copy link
Collaborator Author

... now I'm realizing that that doc is wrong :/

@cipher1024
Copy link
Collaborator Author

It should be

Usage: olean-rs [options]

Options:
    -D, --dump FILE     dump olean parse
    -L                  give location of lean library
    -d, --deps FILE
                        view all dependents of the target file
    -p DIR              set current working directory
    -u, --unused FILE
                        list unused imports
    -l lean.name        test lexer
    -t lean.name        testing
    -m, --makefile      generate a makefile to build and check project
    -h, --help          print this help menu

@robertylewis
Copy link
Member

I had forgotten about this issue. Some of this is now in the "internal" linter, and perhaps more could be added (#1924).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comment
Projects
None yet
Development

No branches or pull requests

5 participants