Skip to content

Commit

Permalink
Properly support runtime annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Dec 11, 2023
1 parent f8ee1df commit b19e86e
Show file tree
Hide file tree
Showing 14 changed files with 531 additions and 271 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,13 @@ def f():

def baz() -> pd.DataFrame | int:
...


def f():
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from pandas import DataFrame

def func(value: DataFrame):
...
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
flake8_type_checking::helpers::is_valid_runtime_import(
binding,
&checker.semantic,
&checker.settings.flake8_type_checking,
)
})
.collect()
Expand Down
76 changes: 72 additions & 4 deletions crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
use crate::rules::flake8_type_checking::settings::Settings;
use anyhow::Result;
use ruff_diagnostics::Edit;
use ruff_python_ast::call_path::from_qualified_name;
use ruff_python_ast::helpers::{map_callable, map_subscript};
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::{Binding, BindingId, BindingKind, SemanticModel};
use ruff_python_codegen::Stylist;
use ruff_python_semantic::{Binding, BindingId, BindingKind, NodeId, SemanticModel};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
use rustc_hash::FxHashSet;

pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticModel) -> bool {
pub(crate) fn is_valid_runtime_import(
binding: &Binding,
semantic: &SemanticModel,
settings: &Settings,
) -> bool {
if matches!(
binding.kind,
BindingKind::Import(..) | BindingKind::FromImport(..) | BindingKind::SubmoduleImport(..)
Expand All @@ -18,9 +28,10 @@ pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticMode
// willing to quote runtime-evaluated, but not runtime-required annotations.
!(reference.in_type_checking_block()
|| reference.in_typing_only_annotation()
|| reference.in_runtime_evaluated_annotation()
|| reference.in_complex_string_type_definition()
|| reference.in_simple_string_type_definition())
|| reference.in_simple_string_type_definition()
|| (settings.annotation_strategy.is_quote()
&& reference.in_runtime_evaluated_annotation()))
})
} else {
false
Expand Down Expand Up @@ -183,3 +194,60 @@ pub(crate) fn is_singledispatch_implementation(
is_singledispatch_interface(function_def, semantic)
})
}

/// Wrap a type annotation in quotes.
///
/// This requires more than just wrapping the reference itself in quotes. For example:
/// - When quoting `Series` in `Series[pd.Timestamp]`, we want `"Series[pd.Timestamp]"`.
/// - When quoting `kubernetes` in `kubernetes.SecurityContext`, we want `"kubernetes.SecurityContext"`.
/// - When quoting `Series` in `Series["pd.Timestamp"]`, we want `"Series[pd.Timestamp]"`.
/// - When quoting `Series` in `Series[Literal["pd.Timestamp"]]`, we want `"Series[Literal['pd.Timestamp']]"`.
pub(crate) fn quote_annotation(
node_id: NodeId,
semantic: &SemanticModel,
locator: &Locator,
stylist: &Stylist,
) -> Result<Edit> {
let expr = semantic.expression(node_id);
if let Some(parent_id) = semantic.parent_expression_id(node_id) {
match semantic.expression(parent_id) {
Expr::Subscript(parent) => {
if expr == parent.value.as_ref() {
// If we're quoting the value of a subscript, we need to quote the entire
// expression. For example, when quoting `DataFrame` in `DataFrame[int]`, we
// should generate `"DataFrame[int]"`.
return quote_annotation(parent_id, semantic, locator, stylist);
}
}
Expr::Attribute(parent) => {
if expr == parent.value.as_ref() {
// If we're quoting the value of an attribute, we need to quote the entire
// expression. For example, when quoting `DataFrame` in `pd.DataFrame`, we
// should generate `"pd.DataFrame"`.
return quote_annotation(parent_id, semantic, locator, stylist);
}
}
Expr::BinOp(parent) => {
if parent.op.is_bit_or() {
// If we're quoting the left or right side of a binary operation, we need to
// quote the entire expression. For example, when quoting `DataFrame` in
// `DataFrame | Series`, we should generate `"DataFrame | Series"`.
return quote_annotation(parent_id, semantic, locator, stylist);
}
}
_ => {}
}
}

let annotation = locator.slice(expr);

// If the annotation already contains a quote, avoid attempting to re-quote it.
if annotation.contains('\'') || annotation.contains('"') {
return Err(anyhow::anyhow!("Annotation already contains a quote"));
}

// If we're quoting a name, we need to quote the entire expression.
let quote = stylist.quote();
let annotation = format!("{quote}{annotation}{quote}");
Ok(Edit::range_replacement(annotation, expr.range()))
}
22 changes: 22 additions & 0 deletions crates/ruff_linter/src/rules/flake8_type_checking/imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use ruff_python_semantic::{AnyImport, Binding, ResolvedReferenceId};
use ruff_text_size::{Ranged, TextRange};

/// An import with its surrounding context.
pub(crate) struct ImportBinding<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
pub(crate) import: AnyImport<'a>,
/// The binding for the imported symbol.
pub(crate) binding: &'a Binding<'a>,
/// The first reference to the imported symbol.
pub(crate) reference_id: ResolvedReferenceId,
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
pub(crate) range: TextRange,
/// The range of the import's parent statement.
pub(crate) parent_range: Option<TextRange>,
}

impl Ranged for ImportBinding<'_> {
fn range(&self) -> TextRange {
self.range
}
}
21 changes: 21 additions & 0 deletions crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Rules from [flake8-type-checking](https://pypi.org/project/flake8-type-checking/).
pub(crate) mod helpers;
mod imports;
pub(crate) mod rules;
pub mod settings;

Expand All @@ -12,6 +13,7 @@ mod tests {
use test_case::test_case;

use crate::registry::{Linter, Rule};
use crate::rules::flake8_type_checking::settings::AnnotationStrategy;
use crate::test::{test_path, test_snippet};
use crate::{assert_messages, settings};

Expand All @@ -33,6 +35,7 @@ mod tests {
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_7.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"))]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))]
#[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))]
#[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TCH003.py"))]
#[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))]
Expand All @@ -52,6 +55,24 @@ mod tests {
Ok(())
}

#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))]
fn quote(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("quote_{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("flake8_type_checking").join(path).as_path(),
&settings::LinterSettings {
flake8_type_checking: super::settings::Settings {
annotation_strategy: AnnotationStrategy::Quote,
..Default::default()
},
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"))]
fn strict(rule_code: Rule, path: &Path) -> Result<()> {
let diagnostics = test_path(
Expand Down
Loading

0 comments on commit b19e86e

Please sign in to comment.