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

Support for refurb #1348

Open
guyrosin opened this issue Dec 23, 2022 · 27 comments
Open

Support for refurb #1348

guyrosin opened this issue Dec 23, 2022 · 27 comments
Labels
plugin Implementing a known but unsupported plugin

Comments

@guyrosin
Copy link

It would be nice to have support for refurb (https://github.com/dosisod/refurb) - a great tool for simplifying and modernizing Python code. It currently includes 48 checks.

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule plugin Implementing a known but unsupported plugin and removed rule Implementing or modifying a lint rule labels Dec 24, 2022
@sbrugman
Copy link
Contributor

sbrugman commented Jan 26, 2023

builtin

  • FURB113 use-list-extend: Use x.extend(...) instead of repeatedly calling x.append()
  • FURB131 no-del:
  • FURB135 no-ignored-dict-items: (PERF102)
  • FURB148 no-ignored-enumerate-items:
  • FURB142 no-set-for-loop:
  • FURB145 no-slice-copy: Replace x[:] with x.copy()
  • FURB105 simplify-print: Replace print("") with print()
  • FURB132 use-set-discard: Replace if x in s: s.remove(x) with s.discard(x)
  • FURB137 simplify-comprehension:
  • FURB154 simplify-global-and-nonlocal:
  • FURB121(PLR1701/SIM101) use-isinstance-issubclass-tuple:
  • FURB136 use-min-max:
  • FURB122 use-writelines: Replace for line in lines: f.write(line) with f.writelines(lines)
  • FURB161 use-bit-count: bin(0b1010).count("1")
  • FURB166 use-int-base-zero: int(x[2:], 2) -> int(x, 0)
  • FURB173 use-dict-union: {"k": "v", **d} -> {"k": "v"} | d

contextlib

  • FURB107(SIM105) use-with-suppress:

datetime

  • FURB162 simplify-fromisoformat
  • FURB176 (DTZ003/DTZ004) unreliable-utc-usage

decimal

  • FURB157: simplify-decimal-ctor: Decimal("123") -> Decimal(123)
  • FURB164: no-from-float: Decimal.from_float(x) -> Decimal(x)

flow

  • FURB133 no-redundant-continue: Continue is redundant here
  • FURB125 no-redundant-return: Return is redundant here
  • FURB127 no-with-assign: This variable is redeclared later, and can be removed here
  • FURB126 simplify-return:

function

  • FURB120 use-implicit-default: Don't pass an argument if it is the same as the default value

functools

  • FURB134(UP033) use-cache: Replace @lru_cache(maxsize=None) with @cache

iterable

  • FURB129 simplify-readlines: Replace f.readlines() with f
  • FURB109 use-consistent-in-bracket:

itertools

  • FURB140 use-starmap: Replace f(...) for ... in x with starmap(f, x)
  • FURB179 use-chain-from-iterable

logical

  • FURB124 use-comparison-chain:
  • FURB108 use-in-oper: (SIM109)
  • FURB110 use-or-oper: Replace x if x else y with x or y

math

  • FURB152 use-math-constant:
  • FURB163: simplify-math-log: math.log(x, 10) -> math.log10(x)

pathlib

  • FURB104 use-pathlib-cwd: Replace os.getcwd() with Path.cwd()
  • FURB141 use-pathlib-exists:
  • FURB146(PTH113) use-pathlib-is-funcs:
  • FURB150(PTH102) use-pathlib-mkdir:
  • FURB147 no-path-join:
  • FURB117 use-pathlib-open:
  • FURB101 use-pathlib-read-text-read-bytes:
  • FURB153(PTH201) simplify-path-constructor: Replace Path(".") with Path()
  • FURB151 use-pathlib-touch:
  • FURB144(PTH107) use-pathlib-unlink:
  • FURB100 use-pathlib-with-suffix: Use Path(x).with_suffix(y) instead of slice and concat
  • FURB103 use-pathlib-write-text-write-bytes:
  • FURB172: use-suffix
  • FURB155 (PTH202)

readability

  • FURB130(SIM118) no-in-dict-keys:
  • FURB114(SIM208) no-double-not: Replace not not x with bool(x)
  • FURB149 no-bool-literal-compare ([refurb] - add bool-literal-compare with fix (FURB149) #9539)
  • FURB115 no-len-compare:
  • FURB143 no-default-or:
  • FURB123 no-redundant-cast:
  • FURB138(PERF401) use-list-comprehension: Consider using list comprehension
  • FURB111 use-func-name: Replace lambda x: f(x) with f
  • FURB112 use-literal: (C408)
  • FURB118 use-operator:
  • FURB128 use-tuple-unpack-swap: Use tuple unpacking instead of temporary variables to swap values
  • FURB160: no-redundant-assignment: x = x
  • FURB165: no-temp-class-object
  • FURB168: no-isinstance-type-none: isinstance(x, type(None)) -> x is None
  • FURB169: no-is-type-none: type(x) is type(None) -> x is None
  • FURB171: no-single-item-in: x in (,y) -> x == y

string

  • FURB106 use-expandtabs:
  • FURB116 use-fstring-number-format:
  • FURB139 no-multiline-lstrip: Replace """\n...""".lstrip() with """\..."""
  • FURB102(PIE810) use-startswith-endswith-tuple:
  • FURB119 use-fstring-format:
  • FURB156: use-string-charsets
  • FURB159: simplify-strip

FastAPI

  • FURB175: simplify-fastapi-query

Misc

  • FURB174: simplify-token-function

Pattern matching

  • FURB158: simplify-as-pattern-with-builtin: case str() as x: -> case str(x):

Regex

  • FURB167: use-long-regex-flag: re.I -> re.IGNORECASE
  • FURB170: use-regex-pattern-methods

@sbrugman
Copy link
Contributor

@charliermarsh refurb leverages type information from mypy for many of its checks (it actually extends mypy).
For instance, no_del looks if the variable is of type list of dict to determine the violation.

Are there any examples of rules where ruff keeps track of the type of a variable? And perhaps broader, what is your view on type inference in ruff?

@charliermarsh charliermarsh added the needs-decision Awaiting a decision from a maintainer label Jul 10, 2023
charliermarsh pushed a commit that referenced this issue Jul 20, 2023
…(`PTH201`) (#5833)

Reviving #2348 step by step

Pt 2. PTH201: Path Constructor Default Argument

- rule originates from `refurb`:
#1348
- Using PTH201 rather than FURBXXX to keep all pathlib logic together
@dosisod
Copy link
Contributor

dosisod commented Jul 22, 2023

Hi, creator of Refurb here! I've added a bunch of checks since this issue was opened, so I would like to give an updated list of what's new. I also went ahead and checked all the existing items on the list to see which ones have been completed and need to be marked off. The docs explaining each of the checks can be found here.

These Refurb checks are already in Ruff and can be checked off:

  • FURB102 (PIE810)
  • FURB114 (SIM208)
  • FURB121 (PLR1701/SIM101)
  • FURB138 (PERF401)
  • FURB144 (PTH107)
  • FURB146 (PTH113)
  • FURB150 (PTH102)
  • These are new checks that will need to be added to the list (see below)
  • FURB155 (PTH202)
  • FURB176 (DTZ003/DTZ004)

In the process I found a bunch of rules in Ruff that are either giving false positives or aren't emitting errors for all the tests in the Refurb test suite. I wouldn't consider these 100% merged, but I'll list them here anyways and will open issues individually later:

  • FURB107 (SIM105)
  • FURB108 (PLR1714)
  • FURB112 (C408/UP018)
  • FURB119 (RUF010)
  • FURB123 (UP018)
  • FURB125 (PLR1711)
  • FURB126 (RET505)
  • FURB130 (SIM118)
  • FURB135 (PERF102)
  • FURB137 (C400/C401/C403/C411)

The rest are new checks that have been added since this issue was opened, and don't tie to any existing rule in Ruff. I tried to maintain the same format as the comment by @sbrugman

builtin

  • FURB161: use-bit-count: bin(0b1010).count("1")
  • FURB166: use-int-base-zero: int(x[2:], 2) -> int(x, 0)
  • FURB173: use-dict-union: {"k": "v", **d} -> {"k": "v"} | d

Datetime

  • FURB162: simplify-fromisoformat
  • FURB176 (DTZ003/DTZ004)

Decimal

  • FURB157: simplify-decimal-ctor: Decimal("123") -> Decimal(123)
  • FURB164: no-from-float: Decimal.from_float(x) -> Decimal(x)

FastAPI

  • FURB175: simplify-fastapi-query

Math

  • FURB163: simplify-math-log: math.log(x, 10) -> math.log10(x)

Misc

  • FURB174: simplify-token-function

Pathlib

  • FURB172: use-suffix
  • FURB155 (PTH202)

Pattern matching

  • FURB158: simplify-as-pattern-with-builtin: case str() as x: -> case str(x):

Readability

  • FURB160: no-redundant-assignment: x = x
  • FURB165: no-temp-class-object
  • FURB168: no-isinstance-type-none: isinstance(x, type(None)) -> x is None
  • FURB169: no-is-type-none: type(x) is type(None) -> x is None
  • FURB171: no-single-item-in: x in (,y) -> x == y

Regex

  • FURB167: use-long-regex-flag: re.I -> re.IGNORECASE
  • FURB170: use-regex-pattern-methods

String

  • FURB156: use-string-charsets
  • FURB159: simplify-strip

That's it! Feel free to hide/edit this comment once the original comment gets updated.

@sbrugman
Copy link
Contributor

@dosisod Thanks for the extensive review, I've updated the tracking. The rules that are nearly implemented because of the different behaviours on some tests cases we will mark as complete as they are fixed.

Currently working on FURB101 and FURB103

@SavchenkoValeriy
Copy link
Contributor

Hey there folks, I'm working on FURB113.

@SavchenkoValeriy
Copy link
Contributor

Also implemented FURB131 and FURB132.

@charliermarsh WDYT? Do you think it's worth it? Thanks!

charliermarsh pushed a commit that referenced this issue Aug 28, 2023
## Summary

As an initial effort with replicating `refurb` rules (#1348 ), this PR
adds support for
[FURB113](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/list_extend.py)
and adds a new category of checks.

## Test Plan

I included a new test + checked that all other tests pass.
charliermarsh pushed a commit that referenced this issue Aug 29, 2023
## Summary

This PR is a continuation of #6702 and me replicating `refurb` rules
(#1348). It adds support for
[FURB131](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/no_del.py.

## Test Plan

I included a new test + checked that all other tests pass.
charliermarsh pushed a commit that referenced this issue Aug 29, 2023
## Summary

This PR is a continuation of #6897 and #6702 and me replicating `refurb`
rules (#1348). It adds support for
[FURB132](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/set_discard.py)

## Test Plan

I included a new test + checked that all other tests pass.
@dosisod
Copy link
Contributor

dosisod commented Jan 25, 2024

Released a (small) update with a few notable changes:

New checks:

  • FURB187 use-reverse: x = reversed(x), x = x[::-1] -> x.reverse()
  • FURB188 use-remove-prefix-or-suffix: f[:-4] if f.endswith(".txt") else f -> f.removesuffix(".txt")
    • removeprefix()/removesuffix() is Python 3.9+

Both of the above checks improved performance when using the suggested changes, at least in the micro benchmarks I ran.

Improved checks:

  • FURB118: lambda x: (x[2], x[0]) -> operator.itemgetter(2, 0)
  • FURB111/PLW0108: lambda: 0.0 -> float
    • PLW0108 used to cover FURB111, but this new change means PLW0108 no longer fully covers FURB111. This new change is similar to a new Pyupgrade check, but that one only checks for lambdas in defaultdict()s, whereas mine has no such restriction.

Also, there are a lot of scattered check-lists throughout this issue, perhaps we could consolidate them into the comment at the top? At the very least I could consolidate my comments into one, if that helps.

@Skylion007
Copy link
Contributor

Isn't FURB142 already implemented as PLC0208?

@boolean-light
Copy link
Contributor

Working on FURB164.

charliermarsh added a commit that referenced this issue Mar 30, 2024
## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Implement FURB164 in the issue #1348.
Relevant Refurb docs is here:
https://github.com/dosisod/refurb/blob/v2.0.0/docs/checks.md#furb164-no-from-float

I've changed the name from `no-from-float` to
`verbose-decimal-fraction-construction`.

## Test Plan

<!-- How was it tested? -->

I've written it in the `FURB164.py`.

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
charliermarsh pushed a commit that referenced this issue Apr 2, 2024
## Summary
implement int_on_sliced_str (FURB166) lint
- #1348
- [original
lint](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/use_int_base_zero.py)

## Test Plan
cargo test
charliermarsh pushed a commit that referenced this issue Apr 7, 2024
…`) (#10526)

## Summary
Lint about function like expressions which are equivalent to
`operator.itemgetter`.
See:
#1348 (comment)

## Test Plan
cargo test
dhruvmanila added a commit that referenced this issue Apr 11, 2024
## Summary

Implement `write-whole-file` (`FURB103`), part of #1348. This is largely
a copy and paste of `read-whole-file` #7682.

## Test Plan

Text fixture added.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Glyphack pushed a commit to Glyphack/ruff that referenced this issue Apr 12, 2024
…`) (astral-sh#10526)

## Summary
Lint about function like expressions which are equivalent to
`operator.itemgetter`.
See:
astral-sh#1348 (comment)

## Test Plan
cargo test
Glyphack pushed a commit to Glyphack/ruff that referenced this issue Apr 12, 2024
## Summary

Implement `write-whole-file` (`FURB103`), part of astral-sh#1348. This is largely
a copy and paste of `read-whole-file` astral-sh#7682.

## Test Plan

Text fixture added.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
charliermarsh pushed a commit that referenced this issue Apr 26, 2024
## Summary

Adds `FURB116`

See #1348 

## Test Plan

`cargo test`
charliermarsh pushed a commit that referenced this issue Jun 8, 2024
dhruvmanila added a commit that referenced this issue Nov 7, 2024
## Summary

Implementation for one of the rules in
#1348
Refurb only deals only with classes with a single base, however the rule
is valid for any base.
(`str, Enum` is common prior to `StrEnum`)

## Test Plan

`cargo test`

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Implementing a known but unsupported plugin
Projects
None yet
Development

No branches or pull requests

10 participants