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

Missing check and fix for pep 484 implicit optional #1983

Closed
rytilahti opened this issue Jan 18, 2023 · 3 comments · Fixed by #4831
Closed

Missing check and fix for pep 484 implicit optional #1983

rytilahti opened this issue Jan 18, 2023 · 3 comments · Fixed by #4831
Assignees
Labels
fixes Related to suggested fixes for violations typing Related to type annotations

Comments

@rytilahti
Copy link

rytilahti commented Jan 18, 2023

Recent mypy versions have started to warn about implicit optionals on function signatures, which has been prohibited by pep 484 since python/peps#689.
It would be great if ruff would spot and fix this automatically for you, considering how easy it is to forget to do this while coding.

Minimal code example:

def implicit_optional(optional: str = None) -> None:
    pass

mypy output:

test.py:1: error: Incompatible default for argument "optional" (default has type "None", argument has type "str")  [assignment]
test.py:1: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test.py:1: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase

The fix for python3.10 and newer would be modifying the definition to be:

def implicit_optional(optional: str | None = None) -> None:
    pass

On the implementation side, I think this would belong to flake8-annotations but as it isn't checking for this, I'm wondering what would be the correct place to implement this?

For older Python versions the fix would require either from __future__ import annotations or from typing import Optional and using optional: Optional[str], both of which might be too invasive for automated fixing.

@charliermarsh
Copy link
Member

Yeah I'd love to do this. I want to add a couple autofixes for typing-related stuff and this is a good candidate.

@charliermarsh charliermarsh added fixes Related to suggested fixes for violations typing Related to type annotations labels Jan 18, 2023
@dhruvmanila
Copy link
Member

I think this is a good rule to add along with the auto-fix ability. Should this be added under the RUF or ANN category? I'm leaning towards RUF. I can take this on.

@dhruvmanila
Copy link
Member

I've started working on this

@dhruvmanila dhruvmanila self-assigned this Jun 10, 2023
charliermarsh added a commit that referenced this issue Jun 12, 2023
## Summary

Add rule to disallow implicit optional with autofix.

Currently, I've added it under `RUF` category.

### Limitation

Type aliases could result in false positive:

```python
from typing import Optional

StrOptional = Optional[str]


def foo(arg: StrOptional = None):
	pass
```

## Test Plan

`cargo test`

resolves: #1983

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
konstin pushed a commit that referenced this issue Jun 13, 2023
## Summary

Add rule to disallow implicit optional with autofix.

Currently, I've added it under `RUF` category.

### Limitation

Type aliases could result in false positive:

```python
from typing import Optional

StrOptional = Optional[str]


def foo(arg: StrOptional = None):
	pass
```

## Test Plan

`cargo test`

resolves: #1983

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations typing Related to type annotations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants