Fixit icon indicating copy to clipboard operation
Fixit copied to clipboard

Show a better warning for missing pyre dependency

Open chdsbd opened this issue 3 years ago • 4 comments

When running fixit I encountered the following error:

python3 -m fixit.cli.run_rules
Scanning 1 files
Testing 21 rules

Encountered exception <class 'Exception'> for the following paths:
./main.py
Running `pyre start` may solve the issue.
main.py:4:1
    E305: expected 2 blank lines after class or function definition, found 1

Found 1 reports in 1 files in 0.33 seconds.

My example project has a single python file, main.py, with the following content:

def main() -> None:
    print("hello")

if __name__ == '__main__':
    main()

Here's the repository for reference: https://github.com/chdsbd/fixit-example

Unrelated, but I was surprised to see messages about pyre and flake8. Can fixit be run without these?

chdsbd avatar Sep 19 '20 01:09 chdsbd

The problem is that, on get_repo_caches it can't find the cache, and it gives a warning, but the custom handler shows the warnings as an exception since (exc_info set to True). I'm not sure whether that is intentional or not, so maybe @jimmylai can help?

isidentical avatar Sep 19 '20 12:09 isidentical

Some rules rely on Pyre to provide inferred type (e.g. AwaitAsyncCallRule) to work. https://github.com/Instagram/Fixit/blob/18365db024258817f7f30006886c14ce8f07bbd5/fixit/rules/await_async_call.py#L16

Missing Pyre doesn't impact the use of Fixit. If you don't want to install Pyre, you can simply ignore the exception message. It's a suggestion, not an error.

Flake8 is included in Fixit via Flake8PseudoLintRule. https://fixit.readthedocs.io/en/latest/rules/Flake8PseudoLintRule.html

So Fixit actually work fine on your example code. There was no other suggestions from Fixit specific rules. If you run the same command on code has some issues that Fixit currently supports, it should provide you extra suggestions and autofixes.

Closing this since there is real no issue.

jimmylai avatar Sep 19 '20 23:09 jimmylai

If you don't want to install Pyre, you can simply ignore the exception message. It's a suggestion, not an error.

Wouldn't it be more feasible (especially targeted for new users who have no idea about this, since most linters don't require additional type-checkers to start in that same environment) to ignore this errors, and just give a couple of warnings at the end.

Current:

Encountered exception <class 'Exception'> for the following paths:
./main.py
Running `pyre start` may solve the issue.
main.py:4:1
    E305: expected 2 blank lines after class or function definition, found 1

Found 1 reports in 1 files in 0.33 seconds.

IMHO a better:

main.py:4:1
    E305: expected 2 blank lines after class or function definition, found 1

-> Some rules were disabled since pyre is not found on the current environment
Found 1 reports in 1 files in 0.33 seconds.

isidentical avatar Sep 20 '20 07:09 isidentical

That makes sense especially Pyre is not listed as required dependency of Fixit. Add a better suggestion at the end of the CLI run is a better user experience (instead of the current exception). Reopen to keep track this action item.

jimmylai avatar Sep 20 '20 16:09 jimmylai

Hey folks, we just released Fixit 2.0, which includes major changes to the core frameworks, linting engine, API, and CLI. In light of this new release, this issue is likely to be resolved or no longer relevant. Please take a look at the new user guide or the upgrade guide for more information on how to migrate your projects to the new version.

If you believe this issue is still relevant or still occurring in the latest version, please feel free to open a new issue, or reply here with more details.

Thank you!

amyreese avatar Jul 17 '23 20:07 amyreese