Skip to content

Add stricter linting to CI #996

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

Closed
JelleZijlstra opened this issue Mar 14, 2017 · 4 comments
Closed

Add stricter linting to CI #996

JelleZijlstra opened this issue Mar 14, 2017 · 4 comments
Labels
project: policy Organization of the typeshed project

Comments

@JelleZijlstra
Copy link
Member

There are several common kinds of mistakes in typeshed code that can be detected by a static linter. I wrote a small tool the other day to detect some: https://github.com/JelleZijlstra/stublint. It currently detects certain problems with platform and version checks, typevars that aren't marked private, extraneous code inside functions, and some missing annotations.

How should we incorporate these checks in to typeshed's CI? A few options:

  • Extend https://github.com/ambv/flake8-pyi with more checks (currently it doesn't add any stub-specific checks). This would automatically get us flake8 core features like control over what error codes to enable, but it would require me to rewrite much of stublint's checks.
  • Move stublint's code into typeshed and run it in CI. Using a single repo makes it easier to coordinate changes: we're seeing pretty frequently that changes to some subset of mypy, typing, and typeshed need to be coordinated. Putting typeshed's linter in the same repo avoids that kind of difficulty. It also makes it easier to justify writing checks that are very specific to typeshed, but overall it seems better to stick to a single linting tool (flake8) to run in CI.

I feel like extending flake8-pyi is the better option, but would like to hear from others.

@ambv
Copy link
Contributor

ambv commented Mar 14, 2017

I like to set everything up as flake8 plugins as this enables us to do the following:

  • configuration in a single place
  • ability to disable certain codes for a particular project
  • one command to run for all checks
  • free integration with code editors and online review tools (like Phabricator which we use at Facebook)

You wouldn't have to rewrite that much, you're still going to use your visitor. You just need to assign error codes and adopt the class API that a plugin expects. Look here for instance how this is implemented:

@JelleZijlstra
Copy link
Member Author

Thanks, sounds good. I'll start sending you some PRs.

@JelleZijlstra
Copy link
Member Author

A couple of ideas we came up with during the sprint:

  • Parse the CPython documentation to find which functions are documented in a module. Compare that with the actual stubs.
  • Use the CPython test suite to find out which types functions are expected to accept. Possible implementation: to find what types f() accepts, mock it with a wrapper that records the types it's called with and whether the f() throws an error or returns a value with those types.

@srittau srittau added the project: policy Organization of the typeshed project label Oct 28, 2018
@JelleZijlstra
Copy link
Member Author

We have flake8-pyi and stubtest now, this issue doesn't add much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: policy Organization of the typeshed project
Projects
None yet
Development

No branches or pull requests

4 participants