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

TCH003 false-positive when using dataclasses.KW_ONLY #12859

Closed
flying-sheep opened this issue Aug 13, 2024 · 4 comments · Fixed by #12863
Closed

TCH003 false-positive when using dataclasses.KW_ONLY #12859

flying-sheep opened this issue Aug 13, 2024 · 4 comments · Fixed by #12863
Assignees
Labels
bug Something isn't working

Comments

@flying-sheep
Copy link
Contributor

flying-sheep commented Aug 13, 2024

Seen in Ruff 0.5.7

In the following code, from __future__ import annotations causes the KW_ONLY to become a string.
The dataclasses module can deal with that, but only if the object isn’t in a if TYPE_CHECKING block.
Ruff supports this for things like ClassVar¹, i.e. if a ClassVar is used in a dataclass, Ruff knows the from typing import ClassVar needs to come out of the if TYPE_CHECKING block. However the same doesn’t seem to be true here.

from __future__ import annotations

from dataclasses import KW_ONLY, dataclass

@dataclass
class Test:
    a: int
    _: KW_ONLY
    b: str

¹This is probably the place that should handle KW_ONLY

/// Returns `true` if an annotation will be inspected at runtime by the `dataclasses` module.
///
/// Specifically, detects whether an annotation is to either `dataclasses.InitVar` or
/// `typing.ClassVar` within a `@dataclass` class definition.
///
/// See: <https://docs.python.org/3/library/dataclasses.html#init-only-variables>
pub(crate) fn is_dataclass_meta_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool {
if !semantic.seen_module(Modules::DATACLASSES) {
return false;
}
// Determine whether the assignment is in a `@dataclass` class definition.
if let ScopeKind::Class(class_def) = semantic.current_scope().kind {
if class_def.decorator_list.iter().any(|decorator| {
semantic
.resolve_qualified_name(map_callable(&decorator.expression))
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["dataclasses", "dataclass"])
})
}) {
// Determine whether the annotation is `typing.ClassVar` or `dataclasses.InitVar`.
return semantic
.resolve_qualified_name(map_subscript(annotation))
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["dataclasses", "InitVar"])
|| semantic.match_typing_qualified_name(&qualified_name, "ClassVar")
});
}
}
false
}

@charliermarsh
Copy link
Member

Wow TIL, I’ve never seen that feature before.

@charliermarsh charliermarsh added the bug Something isn't working label Aug 13, 2024
@charliermarsh charliermarsh self-assigned this Aug 13, 2024
@charliermarsh
Copy link
Member

Ah, this requires:

[tool.ruff.flake8-type-checking]
strict = true

So probably somewhat rare (but should be fixed).

@flying-sheep
Copy link
Contributor Author

Thank you!

@flying-sheep
Copy link
Contributor Author

Btw.: I always do

[tool.ruff.lint.flake8-type-checking]
exempt-modules = []
strict = true

because that way I can use new typing features without having to pay attention that they are imported from typing_extensions or in a if TYPE_CHECKING block for backwards compatibility. They just always are in there.

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.

2 participants