Support pre-commit hooks

[bdice]

Describe the Bug

I would like to be able to run pyrefly as a hook in pre-commit (https://pre-commit.com/).

The requirements for a Python pre-commit hook are listed here: https://pre-commit.com/#python

The main requirement that needs to be considered here is: you must be able to pip install . if implemented in the main repository OR you can use a separate repo to declare the hook, which can list pyrefly as a dependency.

For comparison, pre-commit hooks exist for several other type checkers. Sometimes hooks are implemented in the main repository, and sometimes in separate repos. Examples:

• pyre declares .pre-commit-hooks.yaml in the main repo: https://github.com/facebook/pyre-check
• mypy has a semi-official mirror repo supported by the pre-commit authors: https://github.com/pre-commit/mirrors-mypy
• pyright has a separate (third party) repo: https://github.com/RobertCraigie/pyright-python/blob/main/.pre-commit-hooks.yaml
• pytype declares .pre-commit-hooks.yaml in the main repo: https://github.com/google/pytype/blob/main/.pre-commit-hooks.yaml

There are also many more examples of hooks for popular packages here: https://pre-commit.com/hooks.html

Happy to help answer any further questions about how to make this work.

Sandbox Link

No response

(Only applicable for extension issues) IDE Information

No response

[kinto0]

Thanks for the research! Our team seems fairly bearish on pre-commit hooks because when they go wrong they go really wrong.

That being said, if the community wants them we definitely want to support it. With pyrefly's speed up over other checkers, I think it's a great opportunity to make pre-commit hooks more ergonomic.

If you want to submit a PR I see no issues with merging it (as long as it's off by default). The only problem I predict is user frustration with the bugs that we are still working on (pyrefly is still alpha).

[yangdanny97]

@kinto0 as far as I'm aware, the pre-commit hook has to be explicitly installed/added to a repository's pre-commit config before it works, I don't think it will "turn on by default" when pyrefly is installed.

I'm supportive of this; esp since pyrefly is fast enough that it's useful as a pre-commit hook.

@bdice would you be willing to make a PR for this?

[bdice]

@kinto0 Thanks! I had a chat with the Pyrefly team at PyCon. It sounded like most of the concerns were around the general experience with git pre-commit hooks but not the tool named “pre-commit,” which is what I want to support. I typically trigger pre-commit manually as a way to run all the linters, formatters, etc. that have been configured in a repository’s .pre-commit-config.yaml file.

I attempted to work on this locally but wasn’t able to install pyrefly (#217) and decided to wait until the tool is more mature before attempting to adopt it. The main requirement is that pip install . must work from the repo root, and I wasn’t able to build from source.

[bdice]

Thanks @lolpack @yangdanny97! The system hook is a good starting point!

As a follow-up, using a system hook makes it hard to constrain the version of pyrefly that is used (users' systems and CI systems may not have the same version). One of the reasons I advocate for pre-commit hooks comes from its ability to enforce version constraints for all hooks in .pre-commit-config.yaml with the rev: field to choose a tag/release. Using a system hook also makes this incompatible with https://pre-commit.ci, which requires self-installing Python hooks.

Eventually once the install rules are ironed out for this project (issues like #217) so that pip install https://github.com/facebook/pyrefly works out of the box, changing this to work like a Python hook rather than a system hook would be a really worthy improvement.

[lolpack]

Hi @bdice, this is helpful feedback! Let me re-open the issue, but specify the requirement that it should be a self installing pre-commit hook. A few questions:

• From the mypy pre-commit hook, it takes in arguments for args and additional-dependencies. These are pretty critical to type checkers as many folks are including dependencies and at least some minimal config. In your setup are you assuming that the dependencies are available and a configuration file is present? For formatting, this is a lot easier since you can make syntactic changes without an environment needed.
• pre-commit.ci looks pretty nifty! One of the main features I can see is "auto fixing pull requests". Will it do this for type errors? Do you have that feature enabled for type checking?
• Are there other reasons to use pre-commit that we should be aware of?

Again, thank you for the feedback. We'll need to decide where we want to host a Python installable version of this and more context on how people are using pre-commit for type checking would be really helpful.

[github-actions[bot]]

This issue has someone assigned, but has not had recent activity for more than 2 weeks.

If you are still working on this issue, please add a comment so everyone knows. Otherwise, please unassign yourself and allow someone else to take over.

Thank you for your contributions!

[dorschw]

In your setup are you assuming that the dependencies are available and a configuration file is present?

When using system hooks, pre-commit becomes a task runner, assuming the entry point is installed and available.

Are there other reasons to use pre-commit that we should be aware of?

Ensures everybody in the team runs the same configurations, arguments and version of tools (which a py installable is useful) locally and sometimes in CI too.

[bdice]

Sorry for the delayed response. Here are some replies to the questions above.

From the mypy pre-commit hook, it takes in arguments for args and additional-dependencies. These are pretty critical to type checkers as many folks are including dependencies and at least some minimal config. In your setup are you assuming that the dependencies are available and a configuration file is present? For formatting, this is a lot easier since you can make syntactic changes without an environment needed.

Users can specify their own dependencies and flags in the pre-commit config. That's what we do with mypy today. See this example:

https://github.com/rapidsai/cudf/blob/f7aca052e5fa0c4c6cfd5b8fbdf1ee64f4b0c349/.pre-commit-config.yaml#L32-L44

pre-commit.ci looks pretty nifty! One of the main features I can see is "auto fixing pull requests". Will it do this for type errors? Do you have that feature enabled for type checking?

pre-commit just launches the tool. Autofixing depends on the tool and flags. Since I haven't gotten very far with pyrefly yet, I don't know if pyrefly can "autofix" errors and write back to the filesystem, but I don't see any "fix" options in the docs. A good example to reference is the ruff-pre-commit README which has examples with and without the --fix flag. https://github.com/astral-sh/ruff-pre-commit

I don't know off the top of my head which type checkers do/don't support autofixing missing or incorrect types.

Are there other reasons to use pre-commit that we should be aware of?

Exactly what @dorschw said above -- we want to enforce exact consistency between every user system and CI runner configuration that can be controlled with a single configuration file (.pre-commit-config.yaml). Relying on a preinstalled version of pyrefly on the system that is consistent across all developers' systems and CI would be difficult to maintain and enforce, especially given the high velocity of releases for the project. The ideal scenario is that developers never even have to know what pyrefly is or how to install it, because it's entirely automated by pre-commit. We like having near-zero-cost for additional tooling and code quality enforcement through pre-commit.

Again, thank you for the feedback. We'll need to decide where we want to host a Python installable version of this and more context on how people are using pre-commit for type checking would be really helpful.

Happy to help! I know that LSPs and VSCode integration and such are a high priority, but we have such a diverse developer base that we cannot enforce LSP usage. Commit-time checks or CI checks are our compromise. Develop with whatever tools you want, and let the upstream project standardize the rules for code acceptance with pre-commit (locally and in CI).

[github-actions[bot]]

This issue has someone assigned, but has not had recent activity for more than 2 weeks.

If you are still working on this issue, please add a comment so everyone knows. Otherwise, please unassign yourself and allow someone else to take over.

Thank you for your contributions!

[cas--]

This was a source of confusion for me when setting up pre-commit where pyrefly docs expect the dependency to be installed to the system which is the opposite of what you expect since pre-commit tool is meant to handle tool versioning and isolation for ease of running dev running and CI.

The most common way to handle pre-commit python hooks and install pypi wheel dependencies seems to be to create a separate repo e.g. ruff-pre-commit with a mirror script to ensure the pyproject points to the latest release. The added benefit that this also avoids cloning the entire unneeded project repo.

For now I have the following hook that can be manually version bumped when required

repos:
  - repo: local
    hooks:
      - id: pyrefly-typecheck
        name: pyrefly check
        entry: pyrefly check
        language: python
        types_or: [python, pyi]
        require_serial: true
        additional_dependencies: ["pyrefly==0.27.0"]

Note that I removed pass_filenames: false because we usually do want to pass only the filenames that are part of the commit and not all files within project (might revisit if speed difference is negligible for large project). This is used in conjunction with pre-commit run --all-files when run in CI.

[lolpack]

@cas-- @bdice Thank you both for the feedback and apologies for the delay on my side. It looks like someone created a pyrefly-precommit standalone repo which allows it to run without being available on the path already: https://github.com/UserNobody14/pyrefly-precommit

Would this configuration solve a good chunk of your issues? We can prioritize creating an officially supported one on our side once we have clarity on what features it should support. It sounds like we need:

• [ ] Standalone install
• [ ] Ability to pass individual files and whole project

Is the assumption that you would want type checking for the project to be migrated to Pyrefly such that a pyrefly config would be present in your project? Are there other blockers to using Pyrefly in your project that we should be aware of?

[bdice]

That sounds like a good starting point! I'm not aware of any other blockers yet, because I haven't pursued Pyrefly any further after getting blocked by the lack of a pre-commit hook.

[cas--]

@lolpack Thanks for getting back

That is useful to know there is a third-party repo but I don't see any automation to keep up with releases and the tags don't match the release version (e.g. latest is 0.0.3). This means pinning a version in pre-commit config would not be obvious which version of pyrefly is being used. For now a local hook works and happy to wait for official version.

Are there other blockers to using Pyrefly in your project that we should be aware of?

Yes actually a couple of point related to running pre-commit.

Specify venv location

With (pyright-python) we need to workaround the pre-commit isolated environment which was easy with venv config options:

[tool.pyright]
include = ["src"]
exclude = ["**/__pycache__", "**/.*", "**/development"]
venvPath = "."
venv = ".venv"

I have migrated to the following but it's not ideal having to specify full site-packages path instead of venv.

[tool.pyrefly]
project-includes = ["src"]
project-excludes = [
  "**/development",
  "**/.[!/.]*",
  "**/*venv/**",
]
site-package-path = [".venv/lib/python3.12/site-packages"]

I tried with python-interpreter = ".venv/bin/python3" but that didn't seem to work from within pre-commit.

Individual files ignore exclude pattern

The ability to pass files is being hampered by limitation to pyrefly check that ignores project-excludes. So from the above config python files development are not being excluded and it took me ages to determine why until I found this is the command help:

Usage: pyrefly check [OPTIONS] [FILES]... [FILES]... When supplied, project_excludes in any config files loaded for these files to check are ignored, and we use the default excludes unless overridden with the --project-excludes flag

This can be replicated without pre-commit to the following example with 16 expected errors.

❯ uvx pyrefly check
...
INFO 16 errors (5 ignored)

Passing files ignores the exclude pattern so we see additional errors from development python files :

❯ git ls-files -- '*.py' | xargs uvx pyrefly check
ERROR Could not find import of `test` [import-error]
  --> /home/user/project/development/test.py:14:1
...
INFO 29 errors (5 ignored)

[lolpack]

@bdice here is a supported version of pre-commit which will allow you to pass in the version of pyrefly and have it install pyrefly for you: https://github.com/facebook/pyrefly-pre-commit

@cas-- It looks like you want to be able to pass the name of the of the venv, but have pyrefly figure out where site packages belongs based on the OS/env?

[RFLeijenaar]

@lolpack, I followed the docs for pre-commit integration: https://pyrefly.org/en/docs/installation/#pre-commit-integration.

When running pre-commit, I get the following warning:

[WARNING] Unexpected key(s) present on https://github.com/facebook/pyrefly: skip_install

It seems the issue comes from this line in .pre-commit-hooks.yaml:

skip_install: true

As far as I can tell, skip_install is not part of the documented pre-commit hook metadata keys, so it’s triggering the warning.

Since you’ve now created a dedicated repository for pre-commit hooks (https://github.com/facebook/pyrefly-pre-commit), which doesn’t produce this warning, it might make sense to:

1. Remove the .pre-commit-hooks.yaml from the main pyrefly repository, and
2. Update the documentation to point to the new repository.

This would help prevent confusion and ensure users get a clean pre-commit setup.

But perhaps you are still awaiting a response from the others before making any further changes.

[RFLeijenaar]

@cas-- regarding specifying the venv location:

If pyrefly needs to access to the user-installed venv, you might as well add pyrefly as a versioned dev dependency and use the pyrefly-typecheck-system hook. That said, there might be another reason to use pyrefly-typecheck-specific-version that I am not aware of.

From my understanding, the main purpose of the hook pyrefly-typecheck-specific-version is to ensure isolation. So, if you would run git clone followed by pre-commit run it should just work. In this way it can be more easily integrated in CI.

It was a bit unclear to me how the hook gets the necessary dependencies, but I see that you must specify them in:

additional_dependencies: ["pyrefly==0.30.0"] # Specifiy the version of pyrefly to install

Maybe the docs could clarify that additional_dependencies isn’t just for pinning the pyrefly version, but also for including any project-specific typing dependencies. It might be even better if these dependencies can be grabbed from the pyproject.toml, but I can see how this might complicate things.

In line with this, perhaps pyrefly-typecheck-specific-version is better named pyrefly-typecheck-isolated or something along those lines?

[lolpack]

@RFLeijenaar thanks for the feedback. I will update the docs on the GH repo and close this ticket. Feel free to open a PR to add a new runner rule for pyrefly-typecheck-isolated if it's more clear, I know some people are using the pyrefly-typecheck-specific-version already so I don't want to break backwards compat.

[KotlinIsland]

fyi, prek and lefthook are great alternatives to pre-commit