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

Unsafe fix for ANN2XX is too unsafe #8879

Closed
tylerlaprade opened this issue Nov 28, 2023 · 4 comments · Fixed by #8881
Closed

Unsafe fix for ANN2XX is too unsafe #8879

tylerlaprade opened this issue Nov 28, 2023 · 4 comments · Fixed by #8881
Assignees
Labels
bug Something isn't working

Comments

@tylerlaprade
Copy link
Contributor

def do_thing(thing: Thing | None):
    if not thing:
        return None
    return { 'foo': 1 }

The (unsafe) autofix adds -> None as the return type. I know this is under "unsafe" fixes, but this should be easy enough to avoid. https://github.com/JelleZijlstra/autotyping handles this case, so maybe that logic can be reused.

@zanieb
Copy link
Member

zanieb commented Nov 28, 2023

Thing = dict


def do_thing(thing: Thing | None):
    if not thing:
        return None
    return {"foo": 1}
❯ ruff check example.py --select ANN
example.py:4:5: ANN201 Missing return type annotation for public function `do_thing`
Found 1 error.
❯ ruff version
ruff 0.1.6 (f460f9c5c 2023-11-17)

I don't get a suggested fix for this example. Could you provide more reproduction details?

@charliermarsh
Copy link
Member

I can reproduce this with preview enabled -- I think it's just a bug.

@charliermarsh charliermarsh added the bug Something isn't working label Nov 28, 2023
@charliermarsh charliermarsh self-assigned this Nov 28, 2023
@charliermarsh
Copy link
Member

Huh, I can reproduce this in the playground but not locally.

@charliermarsh
Copy link
Member

Oh, you need to have the target Python version set to >= 3.10. Okay. I'm on it.

charliermarsh added a commit that referenced this issue Nov 28, 2023
## Summary

Given `Union[Dict, None]` (in our internal representation), we were
filtering out `Dict` since we treat it as un-representable (i.e., we
can't convert it to an expression), returning just `None` as the type
annotation. We should require that all members of the union are
representable.

Closes #8879.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants