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

Show a better warning for missing pyre dependency #132

Closed
chdsbd opened this issue Sep 19, 2020 · 5 comments
Closed

Show a better warning for missing pyre dependency #132

chdsbd opened this issue Sep 19, 2020 · 5 comments

Comments

@chdsbd
Copy link

chdsbd commented Sep 19, 2020

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?

@isidentical
Copy link
Contributor

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?

@jimmylai
Copy link
Contributor

Some rules rely on Pyre to provide inferred type (e.g. AwaitAsyncCallRule) to work.

class AwaitAsyncCallRule(CstLintRule):

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.

@isidentical
Copy link
Contributor

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.

@jimmylai
Copy link
Contributor

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 jimmylai reopened this Sep 20, 2020
@zsol zsol changed the title Exception raised when running fixit on clean repository Show a better warning for missing pyre dependency Sep 20, 2020
@amyreese
Copy link
Member

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!

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

No branches or pull requests

4 participants