Skip to content

Commit

Permalink
Use boolean
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Dec 11, 2023
1 parent b19e86e commit bf5749a
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ def baz() -> pd.DataFrame | int:
...



def f():
from pandas import DataFrame

def baz() -> DataFrame():
...


def f():
from typing import Literal

from pandas import DataFrame

def baz() -> DataFrame[Literal["int"]]:
...


def f():
from typing import TYPE_CHECKING

Expand Down
18 changes: 15 additions & 3 deletions crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::rules::flake8_type_checking::settings::Settings;
use anyhow::Result;
use rustc_hash::FxHashSet;

use ruff_diagnostics::Edit;
use ruff_python_ast::call_path::from_qualified_name;
use ruff_python_ast::helpers::{map_callable, map_subscript};
Expand All @@ -8,7 +9,8 @@ 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;

use crate::rules::flake8_type_checking::settings::Settings;

pub(crate) fn is_valid_runtime_import(
binding: &Binding,
Expand All @@ -30,7 +32,7 @@ pub(crate) fn is_valid_runtime_import(
|| reference.in_typing_only_annotation()
|| reference.in_complex_string_type_definition()
|| reference.in_simple_string_type_definition()
|| (settings.annotation_strategy.is_quote()
|| (settings.quote_annotations
&& reference.in_runtime_evaluated_annotation()))
})
} else {
Expand Down Expand Up @@ -202,6 +204,8 @@ pub(crate) fn is_singledispatch_implementation(
/// - 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']]"`.
///
/// In general, when expanding a component of a call chain, we want to quote the entire call chain.
pub(crate) fn quote_annotation(
node_id: NodeId,
semantic: &SemanticModel,
Expand All @@ -227,6 +231,14 @@ pub(crate) fn quote_annotation(
return quote_annotation(parent_id, semantic, locator, stylist);
}
}
Expr::Call(parent) => {
if expr == parent.func.as_ref() {
// If we're quoting the function of a call, we need to quote the entire
// expression. For example, when quoting `DataFrame` in `DataFrame()`, we
// should generate `"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
Expand Down
3 changes: 1 addition & 2 deletions crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ 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 Down Expand Up @@ -63,7 +62,7 @@ mod tests {
Path::new("flake8_type_checking").join(path).as_path(),
&settings::LinterSettings {
flake8_type_checking: super::settings::Settings {
annotation_strategy: AnnotationStrategy::Quote,
quote_annotations: true,
..Default::default()
},
..settings::LinterSettings::for_rule(rule_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ use crate::rules::flake8_type_checking::imports::ImportBinding;
/// The type-checking block is not executed at runtime, so the import will not
/// be available at runtime.
///
/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`,
/// If [`flake8-type-checking.quote-annotations`] is set to `true`,
/// annotations will be wrapped in quotes if doing so would enable the
/// corresponding import to remain in the type-checking block.
///
///
/// ## Example
/// ```python
/// from typing import TYPE_CHECKING
Expand All @@ -49,7 +48,7 @@ use crate::rules::flake8_type_checking::imports::ImportBinding;
/// ```
///
/// ## Options
/// - `flake8-type-checking.annotation-strategy`
/// - `flake8-type-checking.quote-annotations`
///
/// ## References
/// - [PEP 535](https://peps.python.org/pep-0563/#runtime-annotation-resolution-and-type-checking)
Expand Down Expand Up @@ -145,18 +144,13 @@ pub(crate) fn runtime_import_in_type_checking_block(
} else {
// Determine whether the member should be fixed by moving the import out of the
// type-checking block, or by quoting its references.
if checker
.settings
.flake8_type_checking
.annotation_strategy
.is_quote()
if checker.settings.flake8_type_checking.quote_annotations
&& binding.references().all(|reference_id| {
let reference = checker.semantic().reference(reference_id);
reference.context().is_typing()
|| reference.in_runtime_evaluated_annotation()
})
{
println!("quote");
quotes_by_statement.entry(node_id).or_default().push(import);
} else {
moves_by_statement.entry(node_id).or_default().push(import);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::rules::isort::{categorize, ImportSection, ImportType};
/// instead be imported conditionally under an `if TYPE_CHECKING:` block to
/// minimize runtime overhead.
///
/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`,
/// If [`flake8-type-checking.quote-annotations`] is set to `true`,
/// annotations will be wrapped in quotes if doing so would enable the
/// corresponding import to be moved into an `if TYPE_CHECKING:` block.
///
Expand Down Expand Up @@ -62,7 +62,7 @@ use crate::rules::isort::{categorize, ImportSection, ImportType};
/// ```
///
/// ## Options
/// - `flake8-type-checking.annotation-strategy`
/// - `flake8-type-checking.quote-annotations`
/// - `flake8-type-checking.runtime-evaluated-base-classes`
/// - `flake8-type-checking.runtime-evaluated-decorators`
///
Expand Down Expand Up @@ -99,7 +99,7 @@ impl Violation for TypingOnlyFirstPartyImport {
/// instead be imported conditionally under an `if TYPE_CHECKING:` block to
/// minimize runtime overhead.
///
/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`,
/// If [`flake8-type-checking.quote-annotations`] is set to `true`,
/// annotations will be wrapped in quotes if doing so would enable the
/// corresponding import to be moved into an `if TYPE_CHECKING:` block.
///
Expand Down Expand Up @@ -135,7 +135,7 @@ impl Violation for TypingOnlyFirstPartyImport {
/// ```
///
/// ## Options
/// - `flake8-type-checking.annotation-strategy`
/// - `flake8-type-checking.quote-annotations`
/// - `flake8-type-checking.runtime-evaluated-base-classes`
/// - `flake8-type-checking.runtime-evaluated-decorators`
///
Expand Down Expand Up @@ -172,7 +172,7 @@ impl Violation for TypingOnlyThirdPartyImport {
/// instead be imported conditionally under an `if TYPE_CHECKING:` block to
/// minimize runtime overhead.
///
/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`,
/// If [`flake8-type-checking.quote-annotations`] is set to `true`,
/// annotations will be wrapped in quotes if doing so would enable the
/// corresponding import to be moved into an `if TYPE_CHECKING:` block.
///
Expand Down Expand Up @@ -208,7 +208,7 @@ impl Violation for TypingOnlyThirdPartyImport {
/// ```
///
/// ## Options
/// - `flake8-type-checking.annotation-strategy`
/// - `flake8-type-checking.quote-annotations`
/// - `flake8-type-checking.runtime-evaluated-base-classes`
/// - `flake8-type-checking.runtime-evaluated-decorators`
///
Expand Down Expand Up @@ -281,11 +281,7 @@ pub(crate) fn typing_only_runtime_import(
|| reference.in_typing_only_annotation()
|| reference.in_complex_string_type_definition()
|| reference.in_simple_string_type_definition()
|| (checker
.settings
.flake8_type_checking
.annotation_strategy
.is_quote()
|| (checker.settings.flake8_type_checking.quote_annotations
&& reference.in_runtime_evaluated_annotation())
})
{
Expand Down
24 changes: 2 additions & 22 deletions crates/ruff_linter/src/rules/flake8_type_checking/settings.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
//! Settings for the `flake8-type-checking` plugin.
use ruff_macros::CacheKey;
use serde::{Deserialize, Serialize};

#[derive(Debug, CacheKey)]
pub struct Settings {
pub strict: bool,
pub exempt_modules: Vec<String>,
pub runtime_evaluated_base_classes: Vec<String>,
pub runtime_evaluated_decorators: Vec<String>,
pub annotation_strategy: AnnotationStrategy,
pub quote_annotations: bool,
}

impl Default for Settings {
Expand All @@ -19,26 +18,7 @@ impl Default for Settings {
exempt_modules: vec!["typing".to_string(), "typing_extensions".to_string()],
runtime_evaluated_base_classes: vec![],
runtime_evaluated_decorators: vec![],
annotation_strategy: AnnotationStrategy::Preserve,
quote_annotations: false,
}
}
}

#[derive(
Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, CacheKey, is_macro::Is,
)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum AnnotationStrategy {
/// Avoid changing the semantics of runtime-evaluated annotations (e.g., by quoting them, or
/// inserting `from __future__ import annotations`). Imports will be classified as typing-only
/// or runtime-required based exclusively on the existing type annotations.
#[default]
Preserve,
/// Quote runtime-evaluated annotations, if doing so would enable the corresponding import to
/// be moved into an `if TYPE_CHECKING:` block.
Quote,
/// Insert `from __future__ import annotations` at the top of the file, if doing so would enable
/// an import to be moved into an `if TYPE_CHECKING:` block.
Future,
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
quote.py:47:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is not available at runtime.
quote.py:64:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is not available at runtime.
|
46 | if TYPE_CHECKING:
47 | from pandas import DataFrame
63 | if TYPE_CHECKING:
64 | from pandas import DataFrame
| ^^^^^^^^^ TCH004
48 |
49 | def func(value: DataFrame):
65 |
66 | def func(value: DataFrame):
|
= help: Quote references

Unsafe fix
46 46 | if TYPE_CHECKING:
47 47 | from pandas import DataFrame
48 48 |
49 |- def func(value: DataFrame):
49 |+ def func(value: "DataFrame"):
50 50 | ...
63 63 | if TYPE_CHECKING:
64 64 | from pandas import DataFrame
65 65 |
66 |- def func(value: DataFrame):
66 |+ def func(value: "DataFrame"):
67 67 | ...


Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,45 @@ quote.py:37:22: TCH002 [*] Move third-party import `pandas` into a type-checking
41 44 |
42 45 |

quote.py:45:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block
|
44 | def f():
45 | from pandas import DataFrame
| ^^^^^^^^^ TCH002
46 |
47 | def baz() -> DataFrame():
|
= help: Move into type-checking block

Unsafe fix
1 |+from typing import TYPE_CHECKING
2 |+
3 |+if TYPE_CHECKING:
4 |+ from pandas import DataFrame
1 5 | def f():
2 6 | from pandas import DataFrame
3 7 |
--------------------------------------------------------------------------------
42 46 |
43 47 |
44 48 | def f():
45 |- from pandas import DataFrame
46 49 |
47 |- def baz() -> DataFrame():
50 |+ def baz() -> "DataFrame()":
48 51 | ...
49 52 |
50 53 |

quote.py:54:24: TCH002 Move third-party import `pandas.DataFrame` into a type-checking block
|
52 | from typing import Literal
53 |
54 | from pandas import DataFrame
| ^^^^^^^^^ TCH002
55 |
56 | def baz() -> DataFrame[Literal["int"]]:
|
= help: Move into type-checking block


Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
---
source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
---
quote.py:47:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting.
quote.py:64:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting.
|
46 | if TYPE_CHECKING:
47 | from pandas import DataFrame
63 | if TYPE_CHECKING:
64 | from pandas import DataFrame
| ^^^^^^^^^ TCH004
48 |
49 | def func(value: DataFrame):
65 |
66 | def func(value: DataFrame):
|
= help: Move out of type-checking block

Expand All @@ -17,13 +17,13 @@ quote.py:47:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking b
2 3 | from pandas import DataFrame
3 4 |
--------------------------------------------------------------------------------
44 45 | from typing import TYPE_CHECKING
45 46 |
46 47 | if TYPE_CHECKING:
47 |- from pandas import DataFrame
48 |+ pass
48 49 |
49 50 | def func(value: DataFrame):
50 51 | ...
61 62 | from typing import TYPE_CHECKING
62 63 |
63 64 | if TYPE_CHECKING:
64 |- from pandas import DataFrame
65 |+ pass
65 66 |
66 67 | def func(value: DataFrame):
67 68 | ...


Loading

0 comments on commit bf5749a

Please sign in to comment.