Skip to content

Add pyright to tests #5048

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
srittau opened this issue Feb 22, 2021 · 7 comments · Fixed by #5059
Closed

Add pyright to tests #5048

srittau opened this issue Feb 22, 2021 · 7 comments · Fixed by #5059
Labels
project: policy Organization of the typeshed project

Comments

@srittau
Copy link
Collaborator

srittau commented Feb 22, 2021

I propose to add pyright to our CI test suite. pyright is a stable type checker and probably the fastest to implement new typing features. Also:

  • pyright is frequently used as it is the standard type checker for the popular Visual Studio Code IDE/editor.
  • pyright has many useful warnings, not found in other type checkers.
  • Not running pyright as part of our CI complicates the job for pyright's maintainers as evidenced by the frequent clean-up PRs by Eric Traut. This will be even more of an issue when pyright decides not to ship third-party stubs.

I think the discussion in #5035 shows that adding pyright to CI is uncontroversial. Cc'ing @JelleZijlstra @hauntsaninja and @erictraut.

@srittau srittau added the project: policy Organization of the typeshed project label Feb 22, 2021
@srittau srittau changed the title Add pylance to tests Add pyrigh to tests Feb 22, 2021
@srittau srittau changed the title Add pyrigh to tests Add pyright to tests Feb 22, 2021
@srittau
Copy link
Collaborator Author

srittau commented Feb 22, 2021

(And of course by "pylance" I meant "pyright". Corrected it above.)

@erictraut
Copy link
Contributor

I think this is a great idea. If there are no objections, we (the pyright contributors) will work on a PR that incorporates the command-line version of pyright into a CI test for typeshed. We'll let you know if we need help with any of the specifics.

@srittau
Copy link
Collaborator Author

srittau commented Feb 22, 2021

@erictraut FWIW I have taken a stab at a general framework for this in #5051. I'd be glad if your team takes over, and you can use elements from that PR as you feel appropriate. The main thing I'd like to see is that it keeps the general idea of having a tests/pyright_test.py script to run the tests. The test should also support a tests/pyright_exclude_list.text file, like the other type checkers do.

@erictraut
Copy link
Contributor

That's a great start. Thanks!

Pyright uses a configuration file called "pyrightconfig.json" to control which diagnostic rules are enabled, which directories should be included/excluded, etc. This config file is expected to be at the root directory of the project. Would it be OK if we added this file at the root of typeshed and used that instead of a text-based exclude list? If that isn't acceptable, we could auto-generate the pyrightconfig.json file as part of the test script.

@srittau
Copy link
Collaborator Author

srittau commented Feb 22, 2021

Pyright uses a configuration file called "pyrightconfig.json" to control which diagnostic rules are enabled, which directories should be included/excluded, etc. This config file is expected to be at the root directory of the project. Would it be OK if we added this file at the root of typeshed and used that instead of a text-based exclude list? If that isn't acceptable, we could auto-generate the pyrightconfig.json file as part of the test script.

Sounds fine to me.

@srittau
Copy link
Collaborator Author

srittau commented Feb 23, 2021

A small additional thought: tests/pyright_exclude_list.txt could contain a small note pointing to pyrightconfig.json so people unfamiliar with pyright know where to look.

@jakebailey
Copy link
Contributor

jakebailey commented Feb 23, 2021

I opened #5059 which is based on #5051 with the PR comments addressed, and will update it with a more finalized config.

@srittau srittau closed this as completed Feb 23, 2021
@srittau srittau reopened this Feb 23, 2021
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

Successfully merging a pull request may close this issue.

3 participants