astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.6k stars 1.09k forks source link

RET504 "false positive" when using variables for typing #10732

Closed autinerd closed 7 months ago

autinerd commented 7 months ago

Hello together,

When introducing the RET rules to Home Assistant I currently have this case (simplified):

def fun(a: dict[str, Any]) -> list[dict[str, Any]:
    services: list[dict[str, Any]]
    if "services" in a:
        services = a["services"]
        return services
    ... # services is further assigned and used

where RET504 is flagging, but when fixed mypy is complaining because of the Any type. The current options are 1. use cast, 2. declare the type again or 3. add a # noqa.

It would be cool that when a type declaration of the variable is found, that this assignment followed by a return would not get flagged.

diceroll123 commented 7 months ago

You may want to have a be a TypedDict, something like

from typing import TypedDict, NotRequired, Any

class A_Type(TypedDict):
    services: NotRequired[list[dict[str, Any]]]

def fun(a: A_Type) -> list[dict[str, Any]]:
    services: list[dict[str, Any]]
    if "services" in a:
        return a["services"]
    # services being assigned other ways
    services = []
    return services
autinerd commented 7 months ago

In my case it is too versatile to have a TypedDict. https://github.com/home-assistant/core/pull/114528/files

https://github.com/home-assistant/core/pull/114528/files#diff-da81d27b5f83f07ead4bcbb0f29527511a2d7985acbe1cae45636e28202857ccR63-R74

hass.data is a dict which contains all kinds of different stuff for each integration/platform etc. in Home Assistant. There is currently work done to improve the typing topic, but until then we need these type annotations.

charliermarsh commented 7 months ago

I'm curious if you could do:

services: list[dict[str, Any]] = a["services"]
return services

That might be easier to "detect" / allow.

autinerd commented 7 months ago

I'm curious if you could do:

services: list[dict[str, Any]] = a["services"]
return services

That might be easier to "detect" / allow.

This is already working without issue, but then you have to annotate the variable multiple times.

charliermarsh commented 7 months ago

Oh, hah, I didn't realize that. I think we can support the variant from your first post.