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

Implement flake8-pie #1543

Closed
7 tasks done
sbdchd opened this issue Jan 2, 2023 · 8 comments
Closed
7 tasks done

Implement flake8-pie #1543

sbdchd opened this issue Jan 2, 2023 · 8 comments
Labels
good first issue Good for newcomers plugin Implementing a known but unsupported plugin

Comments

@sbdchd
Copy link
Contributor

sbdchd commented Jan 2, 2023

There might be some overlap with these rules and pylint / flake8-simplify, but I think I eliminated most of the dupes / unnecessary rules:

  • PIE810: single-starts-ends-with
    Instead of calling startswith or endswith on the same string for multiple prefixes, pass the prefixes as a tuple in a single startswith or endswith call.
# error
foo.startswith("foo") or foo.startswith("bar")

# error
foo.endswith("foo") or foo.endswith("bar")

# error
foo.startswith("foo") or str.startswith(foo, "bar")

# ok
foo.startswith(("foo",  "bar"))

# ok
foo.endswith(("foo",  "bar"))

# ok
foo.startswith("foo") or foo.endswith("bar")

pass is unnecessary when definining a class or function with an empty body.

# error
class BadError(Exception):
    """
    some doc comment
    """
    pass

def foo() -> None:
    """
    some function
    """
    pass

# ok
class BadError(Exception):
    """
    some doc comment
    """

def foo() -> None:
    """
    some function
    """

Finds duplicate definitions for the same field, which can occur in large ORM model definitions.

# error
class User(BaseModel):
    email = fields.EmailField()
    # ...80 more properties...
    email = fields.EmailField()

# ok
class User(BaseModel):
    email = fields.EmailField()
    # ...80 more properties...

By default the stdlib enum allows multiple field names to map to the same value, this lint requires each enum value be unique.

# error
class Foo(enum.Enum):
    A = "A"
    B = "B"
    C = "C"
    D = "C"

# ok
class Foo(enum.Enum):
    A = "A"
    B = "B"
    C = "C"
    D = "D"

Check for unnecessary dict unpacking.

# error
{**foo, **{"bar": 10}}

# ok
{**foo, "bar": 10}

As long as the keys of the dict are valid Python identifier names, we can safely remove the surrounding dict.

# error
foo(**{"bar": True})

# ok
foo(bar=True)
foo(**buzz)
foo(**{"bar foo": True})

lambda: [] is equivalent to the builtin list

# error
@dataclass
class Foo:
    foo: List[str] = field(default_factory=lambda: [])

# ok
@dataclass
class Foo:
    foo: List[str] = field(default_factory=list)

Do these all sound good / are there any I should leave out? I skipped a few different ones from the original package that were too specific / awkward.

Also skipped rules that overlapped with:

@charliermarsh
Copy link
Member

charliermarsh commented Jan 2, 2023

Yeah these are all quite nice. I can try to implement one of them tomorrow to lay the groundwork for the plugin and we can run from there.

@charliermarsh
Copy link
Member

(Thanks for this thoughtful collation.)

@charliermarsh charliermarsh added the plugin Implementing a known but unsupported plugin label Jan 2, 2023
@charliermarsh
Copy link
Member

I didn't realize you actually wrote that plugin, very cool.

@ljesparis
Copy link
Contributor

ljesparis commented Jan 16, 2023

I was passing by and was wondering if can i write some of these ? I'm actually learning rust, but, these plugins look easy enough.

I actually got around to developing the prefer_unique_enums rule last night 😅 and i made a mistake with a PR so i closed it 😓

@charliermarsh
Copy link
Member

@ljesparis - Yes please, thanks!

charliermarsh pushed a commit that referenced this issue Jan 17, 2023
I accept any suggestion. By the way, I have a doubt, I have checked and all flake8-pie plugins can be fixed by ruff, but is it necessary that this one is also fixed automatically ?

rel #1543
charliermarsh pushed a commit that referenced this issue Jan 23, 2023
Warn about things like `foo(**{"bar": True})` which is equivalent to `foo(bar=True)`

rel: #1879
rel: #1543
charliermarsh pushed a commit that referenced this issue Jan 23, 2023
Checks for unnecessary spreads, like `{**foo, **{"bar": True}}`
rel: #1879
rel: #1543
@charliermarsh
Copy link
Member

@sbdchd - Are you planning to do PIE810 to close this one out? Just trying to keep tabs on who's working on what. Thanks!

@sbdchd
Copy link
Contributor Author

sbdchd commented Jan 23, 2023

Yeah I can do that one as well

@charliermarsh
Copy link
Member

Gonna close this for now, can always do that final rule if anyone wants to grab it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers plugin Implementing a known but unsupported plugin
Projects
None yet
Development

No branches or pull requests

3 participants