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

RUF009 false positive for descriptor fields #4451

Closed
nstarman opened this issue May 16, 2023 · 7 comments · Fixed by #5537
Closed

RUF009 false positive for descriptor fields #4451

nstarman opened this issue May 16, 2023 · 7 comments · Fixed by #5537
Labels
bug Something isn't working type-inference Requires more advanced type inference.

Comments

@nstarman
Copy link

Dataclasses permit descriptor fields (https://docs.python.org/3.10/library/dataclasses.html#descriptor-typed-fields) but RUF009 is complaining about function calls at class construction. This is similar to #4091, but I think sufficiently different to merit its own issue.

@charliermarsh charliermarsh added the bug Something isn't working label May 16, 2023
@JonathanPlasse
Copy link
Contributor

You can use flake8-bugbear.extend-immutable-calls to set your descriptor as immutable.
You can find this option in the rule documentation function-call-in-dataclass-default-argument

The documentation could be explicit on the possibility to mark a call as immutable.

@charliermarsh
Copy link
Member

That's almost certainly the "best we can do" for now, until we can reliably detect that a class is a descriptor across files.

@nstarman
Copy link
Author

Thanks @JonathanPlasse. That looks like it will work for now.

@charliermarsh, what about within a file? Many of my descriptor field classes are defined in the same file as the dataclass.

@JonathanPlasse
Copy link
Contributor

Ruff did not have the type checking necessary to do this now.

@charliermarsh
Copy link
Member

We might be able to do it based on references though, even without type-checking. In the linked example, if the file were src/foo.py, we could certainly detect that references within that file to IntConversionDescriptor are foo.IntConversionDescriptor.

@JonathanPlasse
Copy link
Contributor

@charliermarsh, should we add a tag for issues that could be improved with type checking ability?
It would facilitate finding what should be changed when we add the type checker.

@charliermarsh charliermarsh added the type-inference Requires more advanced type inference. label May 16, 2023
@charliermarsh
Copy link
Member

Yeah, we have an existing tag -- just added it.

charliermarsh added a commit that referenced this issue Jul 5, 2023
## Summary

Per the Python documentation, dataclasses are allowed to instantiate
descriptors, like so:

```python
class IntConversionDescriptor:
  def __init__(self, *, default):
    self._default = default

  def __set_name__(self, owner, name):
    self._name = "_" + name

  def __get__(self, obj, type):
    if obj is None:
      return self._default

    return getattr(obj, self._name, self._default)

  def __set__(self, obj, value):
    setattr(obj, self._name, int(value))

@DataClass
class InventoryItem:
  quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100)
```

Closes #4451.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working type-inference Requires more advanced type inference.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants