astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.9k stars 1.02k forks source link

Implement remaining `pycodestyle` rules #2402

Open charliermarsh opened 1 year ago

charliermarsh commented 1 year ago

Note: some of the checked-off rules are still gated behind the logical_lines feature flag. To see the list of rules enabled in the current release, refer to the docs.

E1 Indentation

E2 Whitespace

E3 Blank line

E4 Import

E5 Line length

E7 Statement

E9 Runtime

W1 Indentation warning

W2 Whitespace warning

W3 Blank line warning

W5. Line break warning

W6 Deprecation warning

charliermarsh commented 1 year ago

I'm happy to support these, but I likely won't enable them by default -- I'd like them to be opt-in.

charliermarsh commented 1 year ago

(Also that tabulation is incomplete, but I'll finish it up later!)

charliermarsh commented 1 year ago

\cc @saadmk11

charliermarsh commented 1 year ago

I've updated this lint to include all the current pydocstyle rules, less those that are ignored in the default configuration and those that deal with pre-Python 3.7 code.

charliermarsh commented 1 year ago

First we need #1130.

charliermarsh commented 1 year ago

So far I have seven more rules done in #1130.

pierrepo commented 1 year ago

Thanks for this @charliermarsh I'm using a lot pycodestyle and pydocstyle with my students to teach them PEP8 and PEP257. Using ruff will be a great plus.

charliermarsh commented 1 year ago

1130 is up to 14 rules. Just trying to knock out a few each days.

charliermarsh commented 1 year ago

Knocked out eight more rules behind the feature flag.

charliermarsh commented 1 year ago

Four more done.

charliermarsh commented 1 year ago

One more in #3225.

charliermarsh commented 1 year ago

One more in https://github.com/charliermarsh/ruff/pull/3249.

charliermarsh commented 1 year ago

One more in https://github.com/charliermarsh/ruff/pull/3344.

kapilt commented 1 year ago

rust newb, if someone wants to try this feature set, how do we build ruff with logical-line feature flag support and these style rules enabled?

[update] this seems to do it, following along on contributing.md

git clone https://github.com/charliermarsh/ruff.git
cd ruff
cargo build --all-features --release
cp target/release/ruff ~/bin
spaceone commented 1 year ago

rust newb, if someone wants to try this feature set, how do we build ruff with logical-line feature flag support and these style rules enabled?

@charliermarsh I would like to have a pip release for ruff with that special feature as pip/setup.py "package option" / "extra"/ "extras_require" enabled. So that you can call pip install ruff[logical-line] and have this enabled. Maybe it's possible to achieve that?

charliermarsh commented 1 year ago

I'm hoping that we can finish and release this soon enough that it's not worth shipping an extra_requires variant. (And if not, I'd probably rather ship with a command-line or feature flag rather than an extras, so that it's available regardless of how you install Ruff.)

jamesbraza commented 1 year ago

This conquest is awesome! I see many of these as being checked (e.g. E2 whitespace is done).

Just a friendly request that these get eventually added to the Rules in docs: https://beta.ruff.rs/docs/rules/#error-e

calumy commented 1 year ago

Just a friendly request that these get eventually added to the Rules in docs: https://beta.ruff.rs/docs/rules/#error-e

I believe this will be added when #3689 lands, as documenting a rule before it has been enabled will potentially cause some confusion.

hoel-bagard commented 1 year ago

I'll take a look at E303.

EDIT: I'm doing all of E3 Blank line since they are closely related.

devbis commented 1 year ago

E241 and E242 is missing in the list for pycodestyle.

E241 | multiple spaces after ',' E242 | tab after ','

Can you please add it to the checks?

charliermarsh commented 1 year ago

I believe those are intentionally omitted -- we're not planning to implement the rules that are omitted from pycodestyle's default configuration ("not unanimously accepted, and PEP 8 does not enforce them"), at least not to start.

hoel-bagard commented 1 year ago

I'm looking into E122 (and I'll follow with the other E1 rules after unless someones is working on them).

MartinRoth commented 1 year ago

Hello,

I just stumbled about a peculiar behaviour of E714. I had fixed the lint but forgot to remove the first not, i.e. I changed the first if statement to the second below and ruff (I am using ruff version v0.0.275 and v.0.0.277) didn't complain anymore.

a = 8

if not a is None:
    print(a)

if not a is not None:
    print(a)

I think the lint should also check for the second version, or am I wrong?

spaceone commented 1 year ago

I'm hoping that we can finish and release this soon enough that it's not worth shipping an extra_requires variant. (And if not, I'd probably rather ship with a command-line or feature flag rather than an extras, so that it's available regardless of how you install Ruff.)

I think this will take a lot of extra time right? Maybe it's possible to add that command line flag already?

ksunden commented 11 months ago

Gentle voice of support for E122 and E302 (the latter being included in #4694 which has been in draft for a while.) (ping @hoel-bagard, as both author of that PR and as the one who said you were looking into E122)

These are the last two rules from Matplotlib's flake8 config. Once available, I'd look into swapping over CI/pre-commit hooks to ruff (and then perhaps look into adding more checks, though for now have gone with "parity")

We do not use black, so these are checks which fall into the category of "black would just handle them".

hoel-bagard commented 11 months ago

@ksunden Sorry for taking a long time, for E302, I have started to see if I could find a way to handle comments with the tools currently present in ruff (otherwise a new tool/way of tracking tokens will need to be implemented as suggested in the PR). I was a bit busy recently, but I should be able to spend more time on it in the near future.

For E122, I've just edited my comment saying I was looking into it. A while ago I asked a question on the discord, but did not get an answer (and then I went back to working on the E302 rule and forgot about it). I could restart working on it assuming that having a token emitted for the backslash is fine, in which case it shouldn't take too much time.

I'm looking into adding the E122 rule of pycodestyle (https://www.flake8rules.com/rules/E122.html). It's a rule that checks for indentation within continuation lines.

Implementing it using logical lines seemed natural (and that's what pycodestyle does). It seems like it would work for continuation lines made using parentheses since a NonLogicalNewline is emitted for the new line. However, when using a backslash (case covered by pycodestyle), no NonLogicalNewline token is created, rendering it impossible to know if there is a newline within the logical line.

Could a NonLogicalNewline be emitted when there is a backslash, or does the rule need to be implemented using physical lines ?

Example with parentheses:

print("E122", (
"dent"))

Example using a backslash:

if some_very_very_very_long_variable_name or var \
or another_very_long_variable_name:
    raise Exception()
Daku24 commented 10 months ago

Is there an update on when E3 Blank line will get released?

hoel-bagard commented 10 months ago

I've been doing the E122 rule. I'm at a point where it's working, but I need to clean things up and add the autofix and the config options. I'll go back to the E3 rule once that's done.

Edit:

Avasam commented 9 months ago

W391 is checked but I don't see the rule in Ruff

hoel-bagard commented 6 months ago

@charliermarsh Since #9266 got merged, the E3 rules can be checked here.

spaceone commented 6 months ago

Are meanwhile all above checked rules now available, at least behind the preview flag?

charliermarsh commented 6 months ago

@spaceone - Yup, they're all behind --preview.

buhtz commented 6 months ago

How to handle problems with --preview error codes? Report them here or open an extra issue?

E303 (too-many-blank-lines) not working on this file for example:

# foo.py
class Foo:
    def __init__(self):
        pass

    def two(self):
        pass

Calling ruff 0.2.2 via ruff foo.py --preview.

hoel-bagard commented 6 months ago

@buhtz Have you selected the E303 rule ? For example with:

ruff foo.py --preview --select E3
buhtz commented 6 months ago

@buhtz Have you selected the E303 rule ? For example with:

ruff foo.py --preview --select E3

No, I have not. As discussed here and how I understand the docs the --preview flag to select all "preview" (unstable, new) rules.

EDIT: IMHO the docs are misleading.

$ ruff foo.py --preview --select E303
foo.py:7:5: E303 [*] Too many blank lines (2)
  |
7 |     def two(self):
  |     ^^^ E303
8 |         pass
  |
  = help: Remove extraneous blank line(s)

Found 1 error.
[*] 1 fixable with the `--fix` option.
baco commented 6 months ago

@buhtz Have you selected the E303 rule ? For example with:

ruff foo.py --preview --select E3

No, I have not. As discussed here and how I understand the docs the --preview flag to select all "preview" (unstable, new) rules.

Well... that's the issue. As for now, all the E3 rules have just arrived and are on their testing period, hence, the only way to try them is with the --preview flag.

You can see that here, in the Rules, and see that all the rules available under the --preview are marked with 🧪 (a test tube emoji). And the rules you are looking for: image all have the test tube emoji

buhtz commented 6 months ago

the only way to try them is with the --preview flag.

No, you are wrong. That flag alone does not help. You have to combine --preview with --select. That is the missing key fact here. I know you know it. But some readers might misunderstand that. Wording is very important here.

baco commented 6 months ago

Ahhh, ok, ok, that's right, the combination of both flags. --preview doesn't turn on all the “unstable rules”. The flag rather stops filtering out rules matching the “selection rules” (if that's a thing) that happen to be unstable.

Is that what you are pointing out here?

buhtz commented 6 months ago

Is that what you are pointing out here?

Exactly.

kaddkaka commented 5 months ago

Note: some of the checked-off rules are still gated behind the logical_lines feature flag. To see the list of rules enabled in the current release, refer to the docs.

Is this still true? Is there a better way than cross referencing https://docs.astral.sh/ruff/rules/#pycodestyle-e-w with the list in this issue?

Where can I find info about "logical_lines"? A search in the docs gave 0 matches.

MichaReiser commented 5 months ago

Note: some of the checked-off rules are still gated behind the logical_lines feature flag. To see the list of rules enabled in the current release, refer to the docs.

No, that's no longer the case. You can opt in by using --preview and enabling the desired rules (e.g. --extend-select=E3)

All checked rules in this issue are implemented as stable or preview rules and the rules use the same codes as pycodestyle. So there should be no need to cross-reference except if you need to know if the rule is preview only.

Daku24 commented 5 months ago

Anyone know how to use the preview E3 Blank line rules in vscode with the ruff extension. I have the preview version installed in vscode.

dhruvmanila commented 5 months ago

@Daku24 By preview, we mean https://docs.astral.sh/ruff/preview/, you don't need to have the preview version of ruff-vscode extension. If you have a local config for Ruff, you can enable preview and include E3:

[tool.ruff.lint]
preview = true
extend-select = ["E3"]

If you don't have a Ruff config file or can't create or edit it, you can use lint.args.

I hope this helps :)

Alex-ley-scrub commented 2 months ago

This is awesome work 👏

Once E11, E12, and E13 rules are added with automatic fixes, if we run something like this:

ruff check foo.py --fix --preview --select E1

would we would get the ~same result as something like this?:

autopep8 foo.py --in-place [--aggressive] 

If so, that would be amazing! autopep8 is really good at fixing broken indentation and adding this to ruff would be 😍

E1 rules don't have automatic fixes at the minute - is that just a case of priority/time or is there a technical challenge / performance concern / reluctance for this to be default behaviour etc. as somewhat discussed in https://github.com/astral-sh/ruff/pull/1130