From ce3d2b23cccb921a64d604b28701ea2dece6bb2a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 11 Dec 2023 11:41:03 -0500 Subject: [PATCH] Use boolean --- .../src/rules/flake8_type_checking/helpers.rs | 8 ++-- .../src/rules/flake8_type_checking/mod.rs | 3 +- .../runtime_import_in_type_checking_block.rs | 7 +-- .../rules/typing_only_runtime_import.rs | 6 +-- .../rules/flake8_type_checking/settings.rs | 24 +---------- crates/ruff_workspace/src/options.rs | 32 +++++--------- ruff.schema.json | 43 +++---------------- 7 files changed, 28 insertions(+), 95 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index f502d3519274e2..15f377fcea7a10 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -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}; @@ -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, @@ -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 { diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 754b7e8ae74ac2..0f97bc708a4464 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -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}; @@ -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) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index 6a766392f1403a..4254874b5111f6 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -145,18 +145,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); diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index b0184f4642d2b8..d0d04c797d6335 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -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()) }) { diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs b/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs index 5380d16babb581..26730af895d086 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs @@ -1,7 +1,6 @@ //! Settings for the `flake8-type-checking` plugin. use ruff_macros::CacheKey; -use serde::{Deserialize, Serialize}; #[derive(Debug, CacheKey)] pub struct Settings { @@ -9,7 +8,7 @@ pub struct Settings { pub exempt_modules: Vec, pub runtime_evaluated_base_classes: Vec, pub runtime_evaluated_decorators: Vec, - pub annotation_strategy: AnnotationStrategy, + pub quote_annotations: bool, } impl Default for Settings { @@ -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, -} diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 8c9d9aec638b6f..a527e64d4decb4 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -12,7 +12,6 @@ use ruff_linter::rules::flake8_pytest_style::settings::SettingsError; use ruff_linter::rules::flake8_pytest_style::types; use ruff_linter::rules::flake8_quotes::settings::Quote; use ruff_linter::rules::flake8_tidy_imports::settings::{ApiBan, Strictness}; -use ruff_linter::rules::flake8_type_checking::settings::AnnotationStrategy; use ruff_linter::rules::isort::settings::RelativeImportsOrder; use ruff_linter::rules::isort::{ImportSection, ImportType}; use ruff_linter::rules::pydocstyle::settings::Convention; @@ -1642,8 +1641,8 @@ pub struct Flake8TypeCheckingOptions { )] pub runtime_evaluated_decorators: Option>, - /// The strategy to use when analyzing type annotations that, by default, - /// Python would evaluate at runtime. + /// Whether to add quotes around type annotations, if doing so would allow + /// the corresponding import to be moved into a type-checking block. /// /// For example, in the following, Python requires that `Sequence` be /// available at runtime, despite the fact that it's only used in a type @@ -1658,31 +1657,22 @@ pub struct Flake8TypeCheckingOptions { /// In other words, moving `from collections.abc import Sequence` into an /// `if TYPE_CHECKING:` block above would cause a runtime error. /// - /// By default, Ruff will respect such runtime evaluations (`"preserve"`), - /// but supports two alternative strategies for handling them: - /// - /// - `"quote"`: Add quotes around the annotation (e.g., `"Sequence[int]"`), to - /// prevent Python from evaluating it at runtime. Adding quotes around - /// `Sequence` above will allow Ruff to move the import into an - /// `if TYPE_CHECKING:` block. - /// - `"future"`: Add a `from __future__ import annotations` import at the - /// top of the file, which will cause Python to treat all annotations as - /// strings, rather than evaluating them at runtime. - /// - /// Setting `annotation-strategy` to `"quote"` or `"future"` will allow - /// Ruff to move more imports into type-checking blocks, but may cause - /// issues with other tools that expect annotations to be evaluated at - /// runtime. + /// By default, Ruff will respect such runtime evaluations. However, + /// setting `annotation-strategy` to `"quote"` will instruct Ruff to + /// add quotes around the annotation (e.g., `"Sequence[int]"`), to + /// prevent Python from evaluating it at runtime. Adding quotes around + /// `Sequence` above will allow Ruff to move the import into an + /// `if TYPE_CHECKING:` block. #[option( default = "[]", value_type = r#""preserve" | "quote" | "future""#, example = r#" # Add quotes around type annotations, if doing so would allow # an import to be moved into a type-checking block. - annotation-strategy = "quote" + quote-annotations = true "# )] - pub annotation_strategy: Option, + pub quote_annotations: Option, } impl Flake8TypeCheckingOptions { @@ -1694,7 +1684,7 @@ impl Flake8TypeCheckingOptions { .unwrap_or_else(|| vec!["typing".to_string()]), runtime_evaluated_base_classes: self.runtime_evaluated_base_classes.unwrap_or_default(), runtime_evaluated_decorators: self.runtime_evaluated_decorators.unwrap_or_default(), - annotation_strategy: self.annotation_strategy.unwrap_or_default(), + quote_annotations: self.quote_annotations.unwrap_or_default(), } } } diff --git a/ruff.schema.json b/ruff.schema.json index 7892d0d011551b..f281aa3d5092e1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -699,31 +699,6 @@ }, "additionalProperties": false, "definitions": { - "AnnotationStrategy": { - "oneOf": [ - { - "description": "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.", - "type": "string", - "enum": [ - "preserve" - ] - }, - { - "description": "Quote runtime-evaluated annotations, if doing so would enable the corresponding import to be moved into an `if TYPE_CHECKING:` block.", - "type": "string", - "enum": [ - "quote" - ] - }, - { - "description": "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.", - "type": "string", - "enum": [ - "future" - ] - } - ] - }, "ApiBan": { "type": "object", "required": [ @@ -1209,17 +1184,6 @@ "Flake8TypeCheckingOptions": { "type": "object", "properties": { - "annotation-strategy": { - "description": "The strategy to use when analyzing type annotations that, by default, Python would evaluate at runtime.\n\nFor example, in the following, Python requires that `Sequence` be available at runtime, despite the fact that it's only used in a type annotation: ```python from collections.abc import Sequence\n\ndef func(value: Sequence[int]) -> None: ... ```\n\nIn other words, moving `from collections.abc import Sequence` into an `if TYPE_CHECKING:` block above would cause a runtime error.\n\nBy default, Ruff will respect such runtime evaluations (`\"preserve\"`), but supports two alternative strategies for handling them:\n\n- `\"quote\"`: Add quotes around the annotation (e.g., `\"Sequence[int]\"`), to prevent Python from evaluating it at runtime. Adding quotes around `Sequence` above will allow Ruff to move the import into an `if TYPE_CHECKING:` block. - `\"future\"`: Add a `from __future__ import annotations` import at the top of the file, which will cause Python to treat all annotations as strings, rather than evaluating them at runtime.\n\nSetting `annotation-strategy` to `\"quote\"` or `\"future\"` will allow Ruff to move more imports into type-checking blocks, but may cause issues with other tools that expect annotations to be evaluated at runtime.", - "anyOf": [ - { - "$ref": "#/definitions/AnnotationStrategy" - }, - { - "type": "null" - } - ] - }, "exempt-modules": { "description": "Exempt certain modules from needing to be moved into type-checking blocks.", "type": [ @@ -1230,6 +1194,13 @@ "type": "string" } }, + "quote-annotations": { + "description": "Whether to add quotes around type annotations, if doing so would allow the corresponding import to be moved into a type-checking block.\n\nFor example, in the following, Python requires that `Sequence` be available at runtime, despite the fact that it's only used in a type annotation: ```python from collections.abc import Sequence\n\ndef func(value: Sequence[int]) -> None: ... ```\n\nIn other words, moving `from collections.abc import Sequence` into an `if TYPE_CHECKING:` block above would cause a runtime error.\n\nBy default, Ruff will respect such runtime evaluations. However, setting `annotation-strategy` to `\"quote\"` will instruct Ruff to add quotes around the annotation (e.g., `\"Sequence[int]\"`), to prevent Python from evaluating it at runtime. Adding quotes around `Sequence` above will allow Ruff to move the import into an `if TYPE_CHECKING:` block.", + "type": [ + "boolean", + "null" + ] + }, "runtime-evaluated-base-classes": { "description": "Exempt classes that list any of the enumerated classes as a base class from needing to be moved into type-checking blocks.\n\nCommon examples include Pydantic's `pydantic.BaseModel` and SQLAlchemy's `sqlalchemy.orm.DeclarativeBase`, but can also support user-defined classes that inherit from those base classes. For example, if you define a common `DeclarativeBase` subclass that's used throughout your project (e.g., `class Base(DeclarativeBase) ...` in `base.py`), you can add it to this list (`runtime-evaluated-base-classes = [\"base.Base\"]`) to exempt models from being moved into type-checking blocks.", "type": [