Skip to content

Commit

Permalink
Respect runtime-required decorators for function signatures (#10091)
Browse files Browse the repository at this point in the history
## Summary

The original implementation of this applied the runtime-required context
to definitions _within_ the function, but not the signature itself. (We
had test coverage; the snapshot was just correctly showing the wrong
outcome.)

Closes #10089.
  • Loading branch information
charliermarsh authored Feb 23, 2024
1 parent 6fe15e7 commit 946028e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 46 deletions.
22 changes: 22 additions & 0 deletions crates/ruff_linter/src/checkers/ast/annotation.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ruff_python_ast::StmtFunctionDef;
use ruff_python_semantic::{ScopeKind, SemanticModel};

use crate::rules::flake8_type_checking;
Expand Down Expand Up @@ -26,6 +27,8 @@ pub(super) enum AnnotationContext {
}

impl AnnotationContext {
/// Determine the [`AnnotationContext`] for an annotation based on the current scope of the
/// semantic model.
pub(super) fn from_model(semantic: &SemanticModel, settings: &LinterSettings) -> Self {
// If the annotation is in a class scope (e.g., an annotated assignment for a
// class field) or a function scope, and that class or function is marked as
Expand Down Expand Up @@ -71,4 +74,23 @@ impl AnnotationContext {

Self::TypingOnly
}

/// Determine the [`AnnotationContext`] to use for annotations in a function signature.
pub(super) fn from_function(
function_def: &StmtFunctionDef,
semantic: &SemanticModel,
settings: &LinterSettings,
) -> Self {
if flake8_type_checking::helpers::runtime_required_function(
function_def,
&settings.flake8_type_checking.runtime_required_decorators,
semantic,
) {
Self::RuntimeRequired
} else if semantic.future_annotations() {
Self::TypingOnly
} else {
Self::RuntimeEvaluated
}
}
}
65 changes: 46 additions & 19 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,8 @@ where

// Function annotations are always evaluated at runtime, unless future annotations
// are enabled.
let runtime_annotation = !self.semantic.future_annotations();
let annotation =
AnnotationContext::from_function(function_def, &self.semantic, self.settings);

// The first parameter may be a single dispatch.
let mut singledispatch =
Expand All @@ -608,10 +609,18 @@ where
if let Some(expr) = &parameter_with_default.parameter.annotation {
if singledispatch {
self.visit_runtime_required_annotation(expr);
} else if runtime_annotation {
self.visit_runtime_evaluated_annotation(expr);
} else {
self.visit_annotation(expr);
match annotation {
AnnotationContext::RuntimeRequired => {
self.visit_runtime_required_annotation(expr);
}
AnnotationContext::RuntimeEvaluated => {
self.visit_runtime_evaluated_annotation(expr);
}
AnnotationContext::TypingOnly => {
self.visit_annotation(expr);
}
}
};
}
if let Some(expr) = &parameter_with_default.default {
Expand All @@ -621,28 +630,46 @@ where
}
if let Some(arg) = &parameters.vararg {
if let Some(expr) = &arg.annotation {
if runtime_annotation {
self.visit_runtime_evaluated_annotation(expr);
} else {
self.visit_annotation(expr);
};
match annotation {
AnnotationContext::RuntimeRequired => {
self.visit_runtime_required_annotation(expr);
}
AnnotationContext::RuntimeEvaluated => {
self.visit_runtime_evaluated_annotation(expr);
}
AnnotationContext::TypingOnly => {
self.visit_annotation(expr);
}
}
}
}
if let Some(arg) = &parameters.kwarg {
if let Some(expr) = &arg.annotation {
if runtime_annotation {
self.visit_runtime_evaluated_annotation(expr);
} else {
self.visit_annotation(expr);
};
match annotation {
AnnotationContext::RuntimeRequired => {
self.visit_runtime_required_annotation(expr);
}
AnnotationContext::RuntimeEvaluated => {
self.visit_runtime_evaluated_annotation(expr);
}
AnnotationContext::TypingOnly => {
self.visit_annotation(expr);
}
}
}
}
for expr in returns {
if runtime_annotation {
self.visit_runtime_evaluated_annotation(expr);
} else {
self.visit_annotation(expr);
};
match annotation {
AnnotationContext::RuntimeRequired => {
self.visit_runtime_required_annotation(expr);
}
AnnotationContext::RuntimeEvaluated => {
self.visit_runtime_evaluated_annotation(expr);
}
AnnotationContext::TypingOnly => {
self.visit_annotation(expr);
}
}
}

let definition = docstrings::extraction::extract_definition(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,4 @@ runtime_evaluated_decorators_3.py:6:18: TCH003 [*] Move standard library import
13 16 |
14 17 | @attrs.define(auto_attribs=True)

runtime_evaluated_decorators_3.py:7:29: TCH003 [*] Move standard library import `collections.abc.Sequence` into a type-checking block
|
5 | from dataclasses import dataclass
6 | from uuid import UUID # TCH003
7 | from collections.abc import Sequence
| ^^^^^^^^ TCH003
8 | from pydantic import validate_call
|
= help: Move into type-checking block

Unsafe fix
4 4 | from array import array
5 5 | from dataclasses import dataclass
6 6 | from uuid import UUID # TCH003
7 |-from collections.abc import Sequence
8 7 | from pydantic import validate_call
9 8 |
10 9 | import attrs
11 10 | from attrs import frozen
11 |+from typing import TYPE_CHECKING
12 |+
13 |+if TYPE_CHECKING:
14 |+ from collections.abc import Sequence
12 15 |
13 16 |
14 17 | @attrs.define(auto_attribs=True)


0 comments on commit 946028e

Please sign in to comment.