Giter VIP home page Giter VIP logo

refurb's Introduction

Refurb

A tool for refurbishing and modernizing Python codebases.

Example

# main.py

for filename in ["file1.txt", "file2.txt"]:
    with open(filename) as f:
        contents = f.read()

    lines = contents.splitlines()

    for line in lines:
        if not line or line.startswith("# ") or line.startswith("// "):
            continue

        for word in line.split():
            print(f"[{word}]", end="")

        print("")

Running:

$ refurb main.py
main.py:3:17 [FURB109]: Use `in (x, y, z)` instead of `in [x, y, z]`
main.py:4:5 [FURB101]: Use `y = Path(x).read_text()` instead of `with open(x, ...) as f: y = f.read()`
main.py:10:40 [FURB102]: Replace `x.startswith(y) or x.startswith(z)` with `x.startswith((y, z))`
main.py:16:9 [FURB105]: Use `print() instead of `print("")`

Installing

$ pipx install refurb
$ refurb file.py folder/

Note Refurb must be run on Python 3.10+, though it can check Python 3.7+ code by setting the --python-version flag.

Explanations For Checks

You can use refurb --explain FURB123, where FURB123 is the error code you are trying to look up. For example:

$ refurb --explain FURB123
Don't cast a variable or literal if it is already of that type. For
example:

Bad:

```
name = str("bob")
num = int(123)
```

Good:

```
name = "bob"
num = 123
```

An online list of all available checks can be viewed here.

Ignoring Errors

Use --ignore 123 to ignore error 123. The error code can be in the form FURB123 or 123. This flag can be repeated.

The FURB prefix indicates that this is a built-in error. The FURB prefix is optional, but for all other errors (ie, ABC123), the prefix is required.

You can also use inline comments to disable errors:

x = int(0)  # noqa: FURB123
y = list()  # noqa

Here, noqa: FURB123 specifically ignores the FURB123 error for that line, and noqa ignores all errors on that line.

You can also specify multiple errors to ignore by separating them with a comma/space:

x = not not int(0)  # noqa: FURB114, FURB123
x = not not int(0)  # noqa: FURB114 FURB123

Enabling/Disabling Checks

Certain checks are disabled by default, and need to be enabled first. You can do this using the --enable ERR flag, where ERR is the error code of the check you want to enable. A disabled check differs from an ignored check in that a disabled check will never be loaded, whereas an ignored check will be loaded, an error will be emitted, and the error will be suppressed.

Use the --verbose/-v flag to get a full list of enabled checks.

The opposite of --enable is --disable, which will disable a check. When --enable and --disable are both specified via the command line, whichever one comes last will take precedence. When using enable and disable via the config file, disable will always take precedence.

Use the --disable-all flag to disable all checks. This allows you to incrementally --enable checks as you see fit, as opposed to adding a bunch of --ignore flags. To use this in the config file, set disable_all to true.

Use the --enable-all flag to enable all checks by default. This allows you to opt into all checks that Refurb (and Refurb plugins) have to offer. This is a good option for new codebases. To use this in a config file, set enable_all to true.

In the config file, disable_all/enable_all is applied first, and then the enable and disable fields are applied afterwards.

Note that disable_all and enable_all are mutually exclusive, both on the command line and in the config file. You will get an error if you try to specify both.

You can also disable checks by category using the #category syntax. For example, --disable "#readability" will disable all checks with the readability category. The same applies for enable and ignore. Also, if you disable an entire category you can still explicitly re-enable a check in that category.

Note that #readability is wrapped in quotes because your shell will interpret the # as the start of a comment.

Setting Python Version

Use the --python-version flag to tell Refurb which version of Python your codebase is using. This should allow for better detection of language features, and allow for better error messages. The argument for this flag must be in the form x.y, for example, 3.10.

The syntax for using this in the config file is python_version = "3.10".

When the Python version is unspecified, Refurb uses whatever version your local Python installation uses. For example, if your python --version is 3.11.5, Refurb uses 3.11, dropping the 5 patch version.

Changing Output Formats

By default everything is outputted as plain text:

file.py:1:5 [FURB123]: Replace `int(x)` with `x`

Here are all of the available formats:

To change the default format use --format XYZ on the command line, or format = "XYZ" in the config file.

Changing Sort Order

By default errors are sorted by filename, then by error code. To change this, use the --sort XYZ flag on the command line, or sort_by = "XYZ" in the config file, where XYZ is one of the following sort modes:

  • filename: Sort files in alphabetical order (the default)
  • error: Sort by error first, then by filename

Overriding Mypy Flags

This is typically used for development purposes, but can also be used to better fine-tune Mypy from within Refurb. Any command line arguments after -- are passed to Mypy. For example:

$ refurb files -- --show-traceback

This tells Mypy to show a traceback if it crashes.

You can also use this in the config file by assigning an array of values to the mypy_args field. Note that any Mypy arguments passed via the command line arguments will override the mypy_args field in the config file.

Configuring Refurb

In addition to the command line arguments, you can also add your settings in the pyproject.toml file. For example, the following command line arguments:

refurb file.py --ignore 100 --load some_module --quiet

Corresponds to the following in your pyproject.toml file:

[tool.refurb]
ignore = [100]
load = ["some_module"]
quiet = true

Now all you need to type is refurb file.py!

Note that the values in the config file will be merged with the values specified via the command line. In the case of boolean arguments like --quiet, the command line arguments take precedence. All other arguments (such as ignore and load) will be combined.

You can use the --config-file flag to tell Refurb to use a different config file from the default pyproject.toml file. Note that it still must be in the same form as the normal pyproject.toml file.

Click here to see some example config files.

Ignore Checks Per File/Folder

If you have a large codebase you might want to ignore errors for certain files or folders, which allows you to incrementally fix errors as you see fit. To do that, add the following to your pyproject.toml file:

# these settings will be applied globally
[tool.refurb]
enable_all = true

# these will only be applied to the "src" folder
[[tool.refurb.amend]]
path = "src"
ignore = ["FURB123", "FURB120"]

# these will only be applied to the "src/util.py" file
[[tool.refurb.amend]]
path = "src/util.py"
ignore = ["FURB125", "FURB148"]

Note that only the ignore field is available in the amend sections. This is because a check can only be enabled/disabled for the entire codebase, and cannot be selectively enabled/disabled on a per-file basis. Assuming a check is enabled though, you can simply ignore the errors for the files of your choosing.

Using Refurb With pre-commit

You can use Refurb with pre-commit by adding the following to your .pre-commit-config.yaml file:

  - repo: https://github.com/dosisod/refurb
    rev: REVISION
    hooks:
      - id: refurb

Replacing REVISION with a version or SHA of your choosing (or leave it blank to let pre-commit find the most recent one for you).

Plugins

Installing plugins for Refurb is very easy:

$ pip install refurb-plugin-example

Where refurb-plugin-example is the name of the plugin. Refurb will automatically load any installed plugins.

To make your own Refurb plugin, see the refurb-plugin-example repository for more info.

Writing Your Own Check

If you want to extend Refurb but don't want to make a full-fledged plugin, you can easily create a one-off check file with the refurb gen command.

Note that this command uses the fzf fuzzy-finder for getting user input, so you will need to install fzf before continuing.

Here is the basic overview for creating a new check using the refurb gen command:

  1. First select the node type you want to accept
  2. Then type in where you want to save the auto generated file
  3. Add your code to the new file

To get an idea of what you need to add to your check, use the --debug flag to see the AST representation for a given file (ie, refurb --debug file.py). Take a look at the files in the refurb/checks/ folder for some examples.

Then, to load your new check, use refurb file.py --load your.path.here

Note that when using --load, you need to use dots in your argument, just like importing a normal python module. If your.path.here is a directory, all checks in that directory will be loaded. If it is a file, only that file will be loaded.

Troubleshooting

If Refurb is running slow, use the --timing-stats flag to diagnose why:

$ refurb file --timing-stats /tmp/stats.json

This will output a JSON file with the following information:

  • Total time Mypy took to parse the modules (a majority of the time usually).
  • Time Mypy spent parsing each module. Useful for finding very large/unused files.
  • Time Refurb spent checking each module. These numbers should be very small (less than 100ms).

Larger files naturally take longer to check, but files that take way too long should be looked into, as an issue might only manifest themselves when a file reaches a certain size.

Disable Color

Color output is enabled by default in Refurb. To disable it, do one of the following:

  • Set the NO_COLOR env var.

  • Use the --no-color flag.

  • Set color = false in the config file.

  • Pipe/redirect Refurb output to another program or file.

Developing / Contributing

Setup

To setup locally run:

$ git clone https://github.com/dosisod/refurb
$ cd refurb
$ make install

Tests can be ran all at once using make, or you can run each tool on its own using make black, make flake8, and so on.

Unit tests can be ran with pytest or make test.

Since the end-to-end (e2e) tests are slow, they are not ran when running make. You will need to run make test-e2e to run them.

Updating Documentation

We encourage people to update the documentation when they see typos and other issues!

With that in mind though, don't directly modify the docs/checks.md file. It is auto-generated and will be overridden when new checks are added. The documentation for checks can be updated by changing the docstrings of in the checks themselves. For example, to update FURB100, change the docstring of the ErrorInfo class in the refurb/checks/pathlib/with_suffix.py file. You can find the file for a given check by grep-ing for code = XYZ, where XYZ is the check you are looking for but with the FURB prefix removed.

Use the --verbose flag with --explain to find the filename for a given check. For example:

$ refurb --explain FURB123 --verbose
Filename: refurb/checks/readability/no_unnecessary_cast.py

FURB123: no-redundant-cast [readability]

...

Why Does This Exist?

I love doing code reviews: I like taking something and making it better, faster, more elegant, and so on. Lots of static analysis tools already exist, but none of them seem to be focused on making code more elegant, more readable, or more modern. That is where Refurb comes in.

Refurb is heavily inspired by clippy, the built-in linter for Rust.

What Refurb Is Not

Refurb is not a style/type checker. It is not meant as a first-line of defense for linting and finding bugs, it is meant for making good code even better.

Comparison To Other Tools

There are already lots of tools out there for linting and analyzing Python code, so you might be wondering why Refurb exists (skepticism is good!). As mentioned above, Refurb checks for code which can be made more elegant, something that no other linters (that I have found) specialize in. Here is a list of similar linters and analyzers, and how they differ from Refurb:

Black: is more focused on the formatting and styling of the code (line length, trailing comas, indentation, and so on). It does a really good job of making other projects using Black look more or less the same. It doesn't do more complex things such as type checking or code smell/anti-pattern detection.

flake8: flake8 is also a linter, is very extensible, and performs a lot of semantic analysis-related checks as well, such as "unused variable", "break outside of a loop", and so on. It also checks PEP8 conformance. Refurb won't try and replace flake8, because chances are you are already using flake8 anyways.

Pylint has a lot of checks which cover a lot of ground, but in general, are focused on bad or buggy code, things which you probably didn't mean to do. Refurb assumes that you know what you are doing, and will try to cleanup what is already there the best it can.

Mypy, Pyright, Pyre, and Pytype are all type checkers, and basically just enforce types, ensures arguments match, functions are called in a type safe manner, and so on. They do much more then that, but that is the general idea. Refurb actually is built on top of Mypy, and uses its AST parser so that it gets good type information.

pyupgrade: Pyupgrade has a lot of good checks for upgrading your older Python code to the newer syntax, which is really useful. Where Refurb differs is that Pyupgrade is more focused on upgrading your code to the newer version, whereas Refurb is more focused on cleaning up and simplifying what is already there.

In conclusion, Refurb doesn't want you to throw out your old tools, since they cover different areas of your code, and all serve a different purpose. Refurb is meant to be used in conjunction with the above tools.

refurb's People

Contributors

bbkgh avatar bzoracler avatar chaoticroman avatar doomedraven avatar dosisod avatar eford36 avatar jairhenrique avatar jdevera avatar kianmeng avatar michaeloliverx avatar sbrugman avatar sobolevn avatar trag1c avatar tylerlaprade avatar yilei avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

refurb's Issues

Consider fixating mypy to <0.990 instead

mypy released version 0.982, presumably a bugfix. We should consider fixating the mypy version here to be <0.990 instead, since bugfixes shouldn't break anything, or am I incorrect?

FURB123 can be misleading

the sample code

def g(l):
    """A generator"""
    for i in l:
        yield i


l = [1, 2, 3]  # initial definition

# l becomes a generator (filtering for ex)
l = (i for i in l)
# or
l = g(l)

# forcing l to be a list and not a generator anymore
l = list(l)

leads to the refurb message :

test.py:8:5 [FURB123]: Replace `list(x)` with `x.copy()`

But the suggested solution is wrong.

Adding partial to the functools category

Overview

As the title suggests... adding partial to the functools category

Proposal

I checked the available errors falling under the functools category in version 1.9.1 and found exactly one item ('use cache').
I think adding partial would be a good addition, both for code legibility and -in some cases at least- performance.

Note: this is one of many items I can think of. If this is useful I could suggest more enhancements.

Re-consider default for FURB120 (don't pass an argument if it's the same as the default value)

This can be bad advice for some APIs. For example:

my_ordered_dict.move_to_end(key1, last=True)
my_ordered_dict.popitem(key2, last=True)

Since these two OrderedDict APIs are notoriously hard for users to remember what the default value of last is, it's preferable to be explicit and always pass last=True, especially for codebases that will be taken over by new, less familiar maintainers over time. This is just one example off the top of my head, but I'm fairly confident that FURB120 is bad advice about as often as it's good advice.

Would you consider flipping FURB120 to be off by default, but enableable via config?

Enhancement: Compile using mypy

Hi again,

So I've noticed that refurb is pretty slow compared to other tools. I would guess its because its using mypy and is not itself compiled. I would therefore suggest investing some time into setting up a compilation using mypyc - this shouldnt be hard to do given the fact that codebase is well typed. The annoying and complex part is going to be the github pipeline to test this in different environments and the release of the wheels. But you should see a x2-x4 performance increase out of the box according to mypyc.

[Bug]: FURB110 not emitted for conditions splitted on multiple lines

The Bug

The following simplified code:

from dataclasses import dataclass

@dataclass()
class Person:
    name: str
    surname: str    


person = Person(name="John", surname="Doe")

print(person.name if person.name else "N/A")
print(
    person.surname
    if person.surname
    else "N/A"
)

Emits the following error:

$ refurb file.py
file.py:12:7 [FURB110]: Use `x or y` instead of `x if x else y`

But it should be emitting the same error also for line 14

Version Info

Refurb: v1.5.0
Mypy: v0.982

Python Version

Python 3.10.8

Config File

# N/A

Extra Info

None

[Question]: Why is FURB131 good in this case?

Overview

I have the following code:

del d["encoding"]

Refurb tells me to change it to the following

d.pop("encoding")

I don't understand why I should use pop. IMO, pop and del have a very different meaning:

  • pop means I want to use the value
  • del means I want to delete the value but explicitly not use it

Could you maybe explain your reasoning?

Proposal

My proposal would be to remove 131. But you probably have a good reason, so this is rather a question than a proposal.

Apart from that, I would love a wiki or documentation that explains some more controversial design decisions.
I believe that is part of any linter/formatter/type checker, even if it requires a lot of effort. Maybe you could use the GitHub wiki (preferably) or GitHub Pages

[Bug]: Improve suggestion for dict({...})

Has your issue already been fixed?

  • Have you checked to see if your issue still exists on the master branch? See the docs for instructions on how to setup a local build of Refurb.
  • Have you looked at the open/closed issues to see if anyone has already reported your issue?

The Bug

The following code:

dict({'xmlns': 'http://www.w3.org/1999/xhtml', 'xml:lang': 'en'})

Emits the following message:

$ refurb file.py
file.py:2:1 [FURB123]: Replace `dict(x)` with `x.copy()`

A correct suggestion would be to just drop the dict(), e.g.

file.py:2:1 [FURB123]: Replace `dict({x})` with `{x}`

Version Info

Refurb: v1.10.0
Mypy: v0.990

Python Version

3.10.9

Config File

# N/A

Extra Info

None

Allow mypy version 1.0

In our project we use both refurb and mypy. Now mypy has recently moved to version 1.0. However, refurb still requires mypy <= 0.99. Is it possible to allow mypy version 1.0 with refurb?

[Bug]: Misleading mypy error for duplicate modules

The Bug

With a reproducer such as:

$ mkdir -p repro/sub
$ touch repro/mod.py
$ touch repro/sub/mod.py
$ tree repro
repro
├── mod.py
└── sub
    └── mod.py

running refurb repro results in:

repro/mod.py: error: Duplicate module named "mod" (also at "repro/sub/mod.py")
repro/mod.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
repro/mod.py: note: Common resolutions include: a) using `--exclude` to avoid checking one of them, b) adding `__init__.py` somewhere, c) using `--explicit-package-bases` or adjusting MYPYPATH

which seems to be a mypy-internal error message: None of the suggested resolutions (except the __init__.py one) make any sense in the context of refurb, and the suggested arguments do not exist.

Version Info

Refurb: v1.5.0
Mypy: v0.982

Python Version

Python 3.10.8

Config File

# N/A

Extra Info

None

FURB109 explanation is technically incorrect

This:

Since tuples cannot change value over time, it is more performant to use
them in `for` loops, generators, etc.:

Is wrong. Besides the fact tuples are not usually more performant than lists, the expression in question actually produces the same byte code:

>>> import dist
>>> def f(x):
...     return x in [1, 2, 3]
>>> dis.dis(f)
  2           0 LOAD_FAST                0 (x)
              2 LOAD_CONST               1 ((1, 2, 3))
              4 CONTAINS_OP              0
              6 RETURN_VALUE
>>> def f(x):
...     return x in (1, 2, 3)
>>> dis.dis(f)
  2           0 LOAD_FAST                0 (x)
              2 LOAD_CONST               1 ((1, 2, 3))
              4 CONTAINS_OP              0
              6 RETURN_VALUE

In general, lists tend to be homogenous variable length "lists" and tuples heterogeneous data.
This actually is potentially faster and is logically the correct thing:

>>> def f(x):
...     return x in {1,2,3}
...
>>> dis.dis(f)
  2           0 LOAD_FAST                0 (x)
              2 LOAD_CONST               1 (frozenset({1, 2, 3}))
              4 CONTAINS_OP              0
              6 RETURN_VALUE

However, it's not equivalent - the items need to be hashable, and the comparison is not quite the same. Sets are also much slower to construct, but as long as they are inline they are directly loaded in byte code. This could be much faster to check, but that depends on the size, and inline is not likely to be that large.

UnicodeDecodeError: 'gbk' codec can't decode byte 0x80 in position 380 when the code includes character Chinese

# main.py

for filename in ["file1.txt", "file2.txt"]:

    with open(filename) as f:
        contents = f.read()

    lines = contents.splitlines()

    for line in lines:
        if not line or line.startswith("# ") or line.startswith("// "):
            continue

        for word in line.split():
            print(f"[{word}]", end="")

        print("")
        print("一些中文")

Running:

Traceback (most recent call last):
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\xxx\anaconda3\envs\refurb\Scripts\refurb.exe\__main__.py", line 7, in <module>
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\__main__.py", line 7, in main
    sys.exit(_main(sys.argv[1:]))
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\main.py", line 174, in main
    errors = run_refurb(settings)
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\main.py", line 117, in run_refurb
    [error for error in errors if not ignored_via_comment(error)],
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\main.py", line 117, in <listcomp>
    [error for error in errors if not ignored_via_comment(error)],
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\main.py", line 65, in ignored_via_comment
    line = get_source_lines(error.filename)[error.line - 1]
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\site-packages\refurb\main.py", line 58, in get_source_lines
    return Path(filepath).read_text().splitlines()
  File "C:\Users\xxx\anaconda3\envs\refurb\lib\pathlib.py", line 1133, in read_text
    return f.read()
UnicodeDecodeError: 'gbk' codec can't decode byte 0x80 in position 380: illegal multibyte sequence

Maybe we should use 'utf-8' but not 'gbk' even in the system with Chinese as default language.

Why "use" and "replace" suggestions?

Neat package! Thank you for making it and proving it under an open source license!

In the example, most of the suggestions were in the style and pattern of "use x instead of y", but one of them was "replace x with y".

I am wondering if there is a reason why these messages are different and if so, what. I think having a consistant pattern for how the information is presented would be wise, but perhaps there is a reason for the difference I am not seeing?

Incorrect `len(x) > 0` suggestion

This suggestion changes the resulting code:

x = ""
is_negative = len(x) > 0 and x[0] == "-"   # Output: False

# Violates [FURB115]: Use `x` instead of `len(x) > 0`

x = ""
is_negative = x and x[0] == "-"   # Output: ""

[Bug]: FURB104 should also warn on os.curdir

The Bug

The following code:

import os

   print(os.curdir)

is not triggering the [FURB104]: Use Path.cwd() instead of `os.getcwd() error. it would be nice if it would catch that variant as well and recommend to use Path.cwd() here as well.

Version Info

Refurb: v1.1.0
Mypy: v0.981

Python Version

Python 3.10.7

Config File

# N/A

Extra Info

None

[Enhancement]: Update startswith to handle not and

Overview

foo = 'abc'
bar = foo.startswith('a') or foo.startswith('b') # warning
bar1 = not foo.startswith('a') and not foo.startswith('b') # no warning

bar1 should be the same as bar2

bar2 = not foo.startswith(('a', 'b'))

Proposal

startswith check should be able to handle multiple not startswith as a single not startswith via "or of ands" boolean logic.

Question: Source of useful rules/heuristics for adding checks

Hi. I want to write and contribute some checks into this repository but i haven't any check/rule at this moment. Is there any written source of this rules? Anyone can introduce some rules/blog posts/existing tools, ... here. I think an aggregation of possible future checks descriptions (e.g as a file in this repo) can increase speed of enriching this useful tool.

FURB107 and multiple exceptions

The suggestion for multiple exceptions is slightly wrong:

skbuild/cmaker.py:417:13 [FURB107]: Use `with suppress((AttributeError, KeyError)): ...` instead of `try: ... except (AttributeError, KeyError): pass`

It should be with suppress(AttributeError, KeyError): ...

I'm enjoying running pipx run refurb src on several projects. :)

Please provide a help text

This is uphill:

$ refurb
refurb: no arguments passed
$ refurb -h      # Let's try the traditional short help switch
refurb: unsupported option "-h"
$ refurb --help  # Long options, then?
refurb: unsupported option "--help"
$ refurb -help   # Go-language style?
refurb: unsupported option "-help"
$ refurb help    # Or perhaps as a command?
refurb: can't read file 'help': No such file or directory
$ # Giving up…

The README mentions --explain, --ignore, --load, and --debug, but it would be really useful to have -h/--help.

I'm not even sure which version I am reporting for, since -V/--version is missing, too.

Please.

Enhancement: Performance Rules

Hi again @dosisod,

One thing I'd like to suggest is adding performance checks.

  1. Certain kinds of loops will benefit from being chained using itertools.chain, and in general itertools offers better performance in many cases.
  2. Other things are using the builtins, filter, map and reduce, which can be significantly faster than simple python code because of pythons native optimizations.
  3. Usage of sets for lookup (in checks, is dramatically faster than list/tuple lookup).

False alarm with FURB121

At least refurb ≤ 1.3.0, FURB121 seems reported even if one of is-a predicates require additional condition, e.g.:

from dataclasses import dataclass

class A:
    pass

@dataclass
class B:
    val: str

obj = A() or B("somewhat")

if isinstance(obj, A) or isinstance(obj, B) and obj.val == "somewhat": # FURB121
    print("yes")
$ refurb repro.py
repro.py:12:20 [FURB121]: Replace `isinstance(x, y) or isinstance(x, z)` with `isinstance(x, (y, z))`

Run `refurb --explain ERR` to further explain an error. Use `--quiet` to silence this message

[Bug]: Wrong FURB145 for numpy arrays

Has your issue already been fixed?

  • Have you checked to see if your issue still exists on the master branch? See the docs for instructions on how to setup a local build of Refurb.
  • Have you looked at the open/closed issues to see if anyone has already reported your issue?

The Bug

The following code:

import numpy as np

np.array([[1, 2, 3]])[:,0]

Emits the following error:

$ refurb file.py
furb.py:-1:0 [FURB145]: Replace `x[:]` with `x.copy()`

There are 2 issues:

  1. Arrays with multiple dimensions can have one dimension indexed with :, but that is not the same as copying the entire array;
  2. The line number in the error message is -1.

Version Info

Refurb: v1.8.0
Mypy: v0.991

(installed from git)

Python Version

Python 3.10.6

Config File

# N/A

Extra Info

None

Add Github Action Support

This tool can be easily packaged and deployed as GitHub action when doing code reviews.
I think all that's required is to

  • add documentation
  • deploy on GitHub action marketplace

Willing to help on this.

FURB111 on lambda unpacking dict

Checking the following code with refurb gives me a FURB111 error and suggests that I drop the lambda:

def f(x, y):
    print(f'{x = }, {y = }')


def g(func):
    func({'x': 1, 'y': 2})


if __name__ == '__main__':
    g(lambda d: f(**d))

However, if the last line is changed to g(f), then the code fails, because the dictionary has not been unpacked.

Reported error is not correct

if a == 1 or a == b and a > 3:
    print(1)

generates hint:

t.py:1:4 [FURB108]: Use x in (y, z) instead of x == y or x == z

Which is not correct due to presence of and which has higher precedence

Configuration option for FURB109 (`in_tuple.py`)?

I like the idea of F109 (I've wanted something like this for over 6 years, see pylint-dev/pylint#1001 😅)! However, my codebase currently somewhat consistently uses lists (given that I mostly see lists for homogenous data, but tuples for heterogenous data).

Due to that, I'd have to disable this checker currently - it'd be nice if there was a way to configure it to assume lists (and maybe sets) instead. Even its description seems to imply that there's no one true choice ("it is best to pick one and stick with it").

[Bug]: Not respecting noqa when more than one code present

Has your issue already been fixed?

  • Have you checked to see if your issue still exists on the master branch? See the docs for instructions on how to setup a local build of Refurb.
  • Have you looked at the open/closed issues to see if anyone has already reported your issue?

The Bug

The following code:

def do_test_path_equality(result: Path, expected: str) -> bool:
    if platform.system() == "Windows":
        expected = expected.split("/")
        expected = [FileNameFormatter._format_for_windows(part) for part in expected]
        expected = Path(*expected)
    else:
        expected = Path(expected)
    return str(result).endswith(str(expected))  # noqa: FURB123,RUF100

Emits the following error:

$ refurb file.py
file.py:8:33 [FURB123]: Replace `str(x)` with `x`

But it should not be emitting an error instance because the line contains noqa for FURB123

Version Info

Refurb: v1.13.0
Mypy: v1.0.0

Python Version

Python 3.10.6

Config File

[tool.refurb]
python-version = [3.9]

Extra Info

Same output from Python 3.11.2 as well.

[Bug]: FURB124 problem with class attributes

The Bug

The following code:

class aClass:
    bAttr = 4
    cAttr = 5

aClass.bAttr == 1 and aClass.cAttr == 2

Emits the following error:

$ refurb file.py
test.py:5:1 [FURB124]: Replace `x == y and x == z` with `x == y == z`

But it should not be emitting an error instance because...

Version Info

Refurb: v1.6.0
Mypy: v0.981

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

[Bug]: FURB108 issues

The Bug

The following code:

d1 = 2
d2 = 3
foo = '1' if (d1 == 1 or d2 == 1) else '2'  # Not flagged


class Foo:
    d = 1
    d1 = 2
    d2 = 3


f = Foo()
foo = '1' if (f.d1 == 1 or f.d2 == 1) else '2'

Emits the following error:

$ refurb refurb2.py
refurb2.py:13:15 [FURB108]: Replace `x == y or x == z` with `x in (y, z)`

Run `refurb --explain ERR` to further explain an error. Use `--quiet` to silence this message

But it should not be emitting an error instance because...

It should flag both cases, not just the class one. Also the error message should be different in this case since to make it valid the check needs rewriting as 1 in (f.d1, f.d2)

Version Info

Refurb: v1.6.0
Mypy: v0.990

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

FURB123 being called when doing a conversion of types.

For code:

        # the changelist kwargs can be an int or a cl
        if isinstance(change, int):
            change = str(change)

I get the following error

[FURB123]: Use `x` instead of `str(x)`

This is doing a type conversion, and should be allowed.

Names for errors?

One thing I really like about mypy and pylint over e.g. flake8 is that it has names for its errors, which can also be used to ignore them.

If I read # noqa: FURB125, I have no idea what that's supposed to mean without having to look things up. Similarly, if I write a config file, it needs lots of comments to point out what checks I'm disabling/enabling there.

If, however, I see # noqa: furb-no-trailing-return, or somesuch, that makes things much cleaner.

Generate visitors automatically

I'd like to take stab at this TODO:

# TODO: Dynamically generate this somehow instead of hardcoding it

in _visitor_mappings.py.

Is that okay with you?

Package as a Flake 8 Plugin

Refurb would make a lovely addition to the Flake 8 toolbox. We're already using that for our project, so it would be nice to add refurb in and get IDE integration and one-stop shopping.

Consider adding issue templates for enhancements?

With #67, opening a new issue shows:

image

while there is a small "Open a blank issue" link at the bottom, this seems to imply that using issues for things other than bugs (e.g. feature requests) is discouraged - which I think isn't the case, looking at other open issues. Perhaps there should be another such template (could mostly be blank, I suppose) for enhancement issues?

[Bug]: FURB108 problem with class attributes

The Bug

The following code:

class MyClass:
    a_attr = 0
    b_attr = 0


if MyClass.a_attr == 0 or MyClass.b_attr == 0:
    pass

Emits the following error:

$ refurb file.py
file.py:6:4 [FURB108]: Replace `x == y or x == z` with `x in (y, z)`

But it should not be emitting an error instance because...

Version Info

Refurb: v1.6.0
Mypy: v0.981

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

FURB108 false positive with and

from enum import Enum

class FooBarBaz(Enum):
FOO = 1
BAR = 2
BAZ = 3

def foo(bar: FooBarBaz, baz: bool):
return bar == FooBarBaz.FOO or (bar == FooBarBaz.BAZ and baz)

refurb_test.py:11:12 [FURB108]: Use x in (y, z) instead of x == y or x == z

I don't see how in could be used in this case.

Improve flexibility of how config is provided

Currently the only config refurb looks for in a file is in the path ./pyproject.toml. Some users need to provide config to refurb in other places.

Suggestion:

  • Accept a command-line argument like --config /path/to/my/refurb.toml
  • If no --config argument is passed, a default value of ./.refurb.toml is used as refurb's dedicated default config file location.
  • For simplicity, refurb can emit an error if config is found in both the resolved --config location as well as the project's pyproject.toml. But ideally, refurb would merge any config found in multiple files within the well-known search path in a well-defined order.

One example from my past experience where the merge behavior would be useful is a Python language sponsor who is responsible for all Python code in a company's monorepo, and who wants to set up refurb as a pre-commit hook for all the Python codebases, specifying a global config file for refurb to use, in addition to any local refurb config that individual codebase owners may later add. The suggested scheme would make it easier for the Python sponsor to adopt refurb company-wide, since the global config could be specified in one place (the same place that the global pre-commit hook is configured), rather than having to update N different pyproject.toml files for each of the N Python codebases.

Thanks for your consideration and for maintaining refurb!

[Bug]: FURB110 issures

The Bug

The following code:

foo = 1
bar = 2
baz = 3
bam = foo if bar else baz  # Not flagged


class Foo:
    foo = 1
    bar = 2
    baz = 3


f = Foo()
bam = f.foo if f.bar else f.baz

Emits the following error:

$ refurb refurb3.py
refurb3.py:16:7 [FURB110]: Replace `x if x else y` with `x or y`

Run `refurb --explain ERR` to further explain an error. Use `--quiet` to silence this message

But it should not be emitting an error instance because...

I'm guessing it is expecting bam = f.bar if f.bar else f.baz ...that would make sense, but not the case here. Also only flagged in class case.

Version Info

Refurb: v1.6.0
Mypy: v0.990

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

[Bug]: FURB110 problem with class attributes

The Bug

The following code:

class aClass:
    bAttr = 4
    cAttr = 5


_ = aClass.bAttr if aClass.cAttr else 5

Emits the following error:

$ refurb file.py
(test.py:6:5 [FURB110]: Replace `x if x else y` with `x or y`)

But it should not be emitting an error instance because...

Version Info

Refurb: v1.6.0
Mypy: v0.981

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

Automated fixing of issues?

I realize this is a "shooting for the sky" idea (and my last issue here for now 😊) - but especially for bigger codebases, it would be amazing if refurb could at least fix some issues automatically, similarly to what e.g. pyupgrade does.

I'd like to adopt refurb for my codebase, but it currently points out some 375 issues - cleaning them all up would require a significant amount of work (though I might end up ignoring some, also see #100). If refurb could fix them for me, that would be amazing!

While I'm not sure if this is even on the horizon with the current architecture being based on mypy (?), I thought it couldn't hurt to ask!

Ensure variable is bound in FURB127

This was happy, just reporting a cleanup:

pymalloc = None
try:
    pymalloc = bool(sysconfig.get_config_var("WITH_PYMALLOC"))
except AttributeError:
    pass

But after making the cleanup:

pymalloc = None
with contextlib.suppress(AttributeError):
    pymalloc = bool(sysconfig.get_config_var("WITH_PYMALLOC"))
skbuild/cmaker.py:436:17 [FURB127]: This variable is redeclared later, and can be removed here

In this case, it's not a huge deal (this is kind of ugly anyway so I'll probably rewrite it later), just thought I'd point it out if it's something you think is fixable and worth fixing.

[Bug]: FURB124 issues

The Bug

The following code:

foo = 1
last_foo = 2
last_foo_bar = 3
if foo == last_foo and last_foo_bar == 10:
    print('no problem')


class Foo:
    foo = 1
    last_foo = 2
    last_foo_bar = 3


f = Foo()
if f.foo == f.last_foo and f.last_foo_bar == 10:
    print('oops')

Emits the following error:

$ refurb refurb1.py
refurb1.py:15:4 [FURB124]: Replace `x == y and x == z` with `x == y == z`

Run `refurb --explain ERR` to further explain an error. Use `--quiet` to silence this message

But it should not be emitting an error instance because...

Version Info

Refurb: v1.6.0
Mypy: v0.990

Python Version

Python 3.11.0

Config File

# N/A

Extra Info

None

False tuple unpacking error

Hi again,

I have a type declarations file. Since the type declarations create cyclical dependencies, there is this weirdness happening there:

if TYPE_CHECKING:
    from starlette.responses import Response as StarletteResponse  # noqa: TC004

    from starlite.config import AppConfig  # noqa: TC004
    from starlite.connection import Request, WebSocket  # noqa: TC004
    from starlite.datastructures import State  # noqa: TC004
    from starlite.handlers import HTTPRouteHandler, WebsocketRouteHandler  # noqa: TC004
    from starlite.response import Response  # noqa: TC004
    from starlite.types.protocols import Logger  # noqa: TC004
else:
    AppConfig = Any
    HTTPRouteHandler = Any
    Request = Any
    Response = Any
    StarletteResponse = Any
    State = Any
    WebSocket = Any
    WebsocketRouteHandler = Any
    Logger = Any

Refurb identifies some of the lines (only some of them for some reason) with X = Any as tuple unpacking, and I get this:

starlite/types/callable_types.py:17:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/callable_types.py:20:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/callable_types.py:23:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/composite.py:19:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/composite.py:22:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/internal_types.py:16:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values
starlite/types/internal_types.py:19:5 [FURB128]: Use tuple unpacking instead of temporary variables to swap values

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.