diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.py new file mode 100644 index 0000000000000..198c02a3d643f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.py @@ -0,0 +1,21 @@ +from typing import Final, Literal + +x: Final[Literal[True]] = True # PYI064 +y: Final[Literal[None]] = None # PYI064 +z: Final[Literal[ # PYI064 + "this is a really long literal, that won't be rendered in the issue text" +]] = "this is a really long literal, that won't be rendered in the issue text" + +# This should be fixable, and marked as safe +w1: Final[Literal[123]] # PYI064 + +# This should not be fixable +w2: Final[Literal[123]] = "random value" # PYI064 + +n1: Final[Literal[True, False]] = True # No issue here +n2: Literal[True] = True # No issue here + +PlatformName = Literal["linux", "macos", "windows"] +PLATFORMS: Final[set[PlatformName]] = {"linux", "macos", "windows"} # No issue here + +foo: Final[{1, 2, 3}] = {1, 2, 3} # No issue here diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.pyi new file mode 100644 index 0000000000000..feb8f7926bca1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.pyi @@ -0,0 +1,19 @@ +from typing import Final, Literal + +x: Final[Literal[True]] # PYI064 +y: Final[Literal[None]] = None # PYI064 +z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 + +# This should be fixable, and marked as safe +w1: Final[Literal[123]] # PYI064 + +# This should not be fixable +w2: Final[Literal[123]] = "random value" # PYI064 + +n1: Final[Literal[True, False]] # No issue here +n2: Literal[True] # No issue here + +PlatformName = Literal["linux", "macos", "windows"] +PLATFORMS: Final[set[PlatformName]] = {"linux", "macos", "windows"} # No issue here + +foo: Final[{1, 2, 3}] = {1, 2, 3} # No issue here diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 375ce1aafab06..57671fb771d94 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1633,6 +1633,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::TSuffixedTypeAlias) { flake8_pyi::rules::t_suffixed_type_alias(checker, target); } + } else if checker + .semantic + .match_typing_expr(helpers::map_subscript(annotation), "Final") + { + if checker.enabled(Rule::RedundantFinalLiteral) { + flake8_pyi::rules::redundant_final_literal(checker, assign_stmt); + } } } Stmt::TypeAlias(ast::StmtTypeAlias { name, .. }) => { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 3d490cc961683..56e18af8214f3 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -811,6 +811,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "058") => (RuleGroup::Stable, rules::flake8_pyi::rules::GeneratorReturnFromIterMethod), (Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass), (Flake8Pyi, "062") => (RuleGroup::Preview, rules::flake8_pyi::rules::DuplicateLiteralMember), + (Flake8Pyi, "064") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantFinalLiteral), // flake8-pytest-style (Flake8PytestStyle, "001") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle), diff --git a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs index e69606adfbd5c..f53bd1e7668a0 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs @@ -63,6 +63,8 @@ mod tests { #[test_case(Rule::PatchVersionComparison, Path::new("PYI004.pyi"))] #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.py"))] #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.pyi"))] + #[test_case(Rule::RedundantFinalLiteral, Path::new("PYI064.py"))] + #[test_case(Rule::RedundantFinalLiteral, Path::new("PYI064.pyi"))] #[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.py"))] #[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.pyi"))] #[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs index 18b50b8e78408..9dc5c4456d381 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs @@ -21,6 +21,7 @@ pub(crate) use pass_in_class_body::*; pub(crate) use pass_statement_stub_body::*; pub(crate) use prefix_type_params::*; pub(crate) use quoted_annotation_in_stub::*; +pub(crate) use redundant_final_literal::*; pub(crate) use redundant_literal_union::*; pub(crate) use redundant_numeric_union::*; pub(crate) use simple_defaults::*; @@ -61,6 +62,7 @@ mod pass_in_class_body; mod pass_statement_stub_body; mod prefix_type_params; mod quoted_annotation_in_stub; +mod redundant_final_literal; mod redundant_literal_union; mod redundant_numeric_union; mod simple_defaults; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_final_literal.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_final_literal.rs new file mode 100644 index 0000000000000..8f93ba0d5d968 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_final_literal.rs @@ -0,0 +1,143 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, comparable::ComparableExpr}; +use ruff_source_file::Locator; +use ruff_text_size::{Ranged, TextSize}; + +use crate::checkers::ast::Checker; +use crate::fix::snippet::SourceCodeSnippet; + +/// ## What it does +/// Checks for redundant `Final[Literal[...]]` annotations. +/// +/// ## Why is this bad? +/// A `Final[Literal[...]]` annotation can be replaced with `Final`; the literal +/// use is unnecessary. +/// +/// ## Example +/// +/// ```python +/// x: Final[Literal[42]] +/// y: Final[Literal[42]] = 42 +/// ``` +/// +/// Use instead: +/// ```python +/// x: Final = 42 +/// y: Final = 42 +/// ``` +#[violation] +pub struct RedundantFinalLiteral { + literal: SourceCodeSnippet, +} + +impl Violation for RedundantFinalLiteral { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let RedundantFinalLiteral { literal } = self; + format!( + "`Final[Literal[{literal}]]` can be replaced with a bare `Final`", + literal = literal.truncated_display() + ) + } + + fn fix_title(&self) -> Option { + Some("Replace with `Final`".to_string()) + } +} + +/// PYI064 +pub(crate) fn redundant_final_literal(checker: &mut Checker, ann_assign: &ast::StmtAnnAssign) { + if !checker.semantic().seen_typing() { + return; + } + + let ast::StmtAnnAssign { + value: assign_value, + annotation, + .. + } = ann_assign; + + let ast::Expr::Subscript(annotation) = &**annotation else { + return; + }; + + // Ensure it is `Final[Literal[...]]`. + let ast::Expr::Subscript(ast::ExprSubscript { + value, + slice: literal, + .. + }) = &*annotation.slice + else { + return; + }; + if !checker.semantic().match_typing_expr(value, "Literal") { + return; + } + + // Discards tuples like `Literal[1, 2, 3]` and complex literals like `Literal[{1, 2}]`. + if !matches!( + &**literal, + ast::Expr::StringLiteral(_) + | ast::Expr::BytesLiteral(_) + | ast::Expr::NumberLiteral(_) + | ast::Expr::BooleanLiteral(_) + | ast::Expr::NoneLiteral(_) + | ast::Expr::EllipsisLiteral(_) + ) { + return; + } + + let mut diagnostic = Diagnostic::new( + RedundantFinalLiteral { + literal: SourceCodeSnippet::from_str(checker.locator().slice(literal.range())), + }, + ann_assign.range(), + ); + + // The literal value and the assignment value being different doesn't make sense, so we skip + // fixing in that case. + if let Some(assign_value) = assign_value.as_ref() { + if ComparableExpr::from(assign_value) == ComparableExpr::from(literal) { + diagnostic.set_fix(generate_fix(annotation, None, checker.locator())); + } + } else { + diagnostic.set_fix(generate_fix(annotation, Some(literal), checker.locator())); + } + + checker.diagnostics.push(diagnostic); +} + +/// Generate a fix to convert a `Final[Literal[...]]` annotation to a `Final` annotation. +fn generate_fix( + annotation: &ast::ExprSubscript, + literal: Option<&ast::Expr>, + locator: &Locator, +) -> Fix { + // Remove the `Literal[...]` part from `Final[Literal[...]]`. + let deletion = Edit::range_deletion( + annotation + .slice + .range() + .sub_start(TextSize::new(1)) + .add_end(TextSize::new(1)), + ); + + // If a literal was provided, insert an assignment. + // + // For example, change `x: Final[Literal[42]]` to `x: Final = 42`. + if let Some(literal) = literal { + let assignment = Edit::insertion( + format!( + " = {literal_source}", + literal_source = locator.slice(literal) + ), + annotation.end(), + ); + Fix::safe_edits(deletion, std::iter::once(assignment)) + } else { + Fix::safe_edit(deletion) + } +} diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.py.snap new file mode 100644 index 0000000000000..4e441d5012a1f --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.py.snap @@ -0,0 +1,97 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +--- +PYI064.py:3:1: PYI064 [*] `Final[Literal[True]]` can be replaced with a bare `Final` + | +1 | from typing import Final, Literal +2 | +3 | x: Final[Literal[True]] = True # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +4 | y: Final[Literal[None]] = None # PYI064 +5 | z: Final[Literal[ # PYI064 + | + = help: Replace with `Final` + +ℹ Safe fix +1 1 | from typing import Final, Literal +2 2 | +3 |-x: Final[Literal[True]] = True # PYI064 + 3 |+x: Final = True # PYI064 +4 4 | y: Final[Literal[None]] = None # PYI064 +5 5 | z: Final[Literal[ # PYI064 +6 6 | "this is a really long literal, that won't be rendered in the issue text" + +PYI064.py:4:1: PYI064 [*] `Final[Literal[None]]` can be replaced with a bare `Final` + | +3 | x: Final[Literal[True]] = True # PYI064 +4 | y: Final[Literal[None]] = None # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +5 | z: Final[Literal[ # PYI064 +6 | "this is a really long literal, that won't be rendered in the issue text" + | + = help: Replace with `Final` + +ℹ Safe fix +1 1 | from typing import Final, Literal +2 2 | +3 3 | x: Final[Literal[True]] = True # PYI064 +4 |-y: Final[Literal[None]] = None # PYI064 + 4 |+y: Final = None # PYI064 +5 5 | z: Final[Literal[ # PYI064 +6 6 | "this is a really long literal, that won't be rendered in the issue text" +7 7 | ]] = "this is a really long literal, that won't be rendered in the issue text" + +PYI064.py:5:1: PYI064 [*] `Final[Literal[...]]` can be replaced with a bare `Final` + | +3 | x: Final[Literal[True]] = True # PYI064 +4 | y: Final[Literal[None]] = None # PYI064 +5 | / z: Final[Literal[ # PYI064 +6 | | "this is a really long literal, that won't be rendered in the issue text" +7 | | ]] = "this is a really long literal, that won't be rendered in the issue text" + | |______________________________________________________________________________^ PYI064 +8 | +9 | # This should be fixable, and marked as safe + | + = help: Replace with `Final` + +ℹ Safe fix +2 2 | +3 3 | x: Final[Literal[True]] = True # PYI064 +4 4 | y: Final[Literal[None]] = None # PYI064 +5 |-z: Final[Literal[ # PYI064 +6 |- "this is a really long literal, that won't be rendered in the issue text" +7 |-]] = "this is a really long literal, that won't be rendered in the issue text" + 5 |+z: Final = "this is a really long literal, that won't be rendered in the issue text" +8 6 | +9 7 | # This should be fixable, and marked as safe +10 8 | w1: Final[Literal[123]] # PYI064 + +PYI064.py:10:1: PYI064 [*] `Final[Literal[123]]` can be replaced with a bare `Final` + | + 9 | # This should be fixable, and marked as safe +10 | w1: Final[Literal[123]] # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +11 | +12 | # This should not be fixable + | + = help: Replace with `Final` + +ℹ Safe fix +7 7 | ]] = "this is a really long literal, that won't be rendered in the issue text" +8 8 | +9 9 | # This should be fixable, and marked as safe +10 |-w1: Final[Literal[123]] # PYI064 + 10 |+w1: Final = 123 # PYI064 +11 11 | +12 12 | # This should not be fixable +13 13 | w2: Final[Literal[123]] = "random value" # PYI064 + +PYI064.py:13:1: PYI064 `Final[Literal[123]]` can be replaced with a bare `Final` + | +12 | # This should not be fixable +13 | w2: Final[Literal[123]] = "random value" # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +14 | +15 | n1: Final[Literal[True, False]] = True # No issue here + | + = help: Replace with `Final` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.pyi.snap new file mode 100644 index 0000000000000..2c25d8adc5f36 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI064_PYI064.pyi.snap @@ -0,0 +1,92 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +--- +PYI064.pyi:3:1: PYI064 [*] `Final[Literal[True]]` can be replaced with a bare `Final` + | +1 | from typing import Final, Literal +2 | +3 | x: Final[Literal[True]] # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +4 | y: Final[Literal[None]] = None # PYI064 +5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 + | + = help: Replace with `Final` + +ℹ Safe fix +1 1 | from typing import Final, Literal +2 2 | +3 |-x: Final[Literal[True]] # PYI064 + 3 |+x: Final = True # PYI064 +4 4 | y: Final[Literal[None]] = None # PYI064 +5 5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 +6 6 | + +PYI064.pyi:4:1: PYI064 [*] `Final[Literal[None]]` can be replaced with a bare `Final` + | +3 | x: Final[Literal[True]] # PYI064 +4 | y: Final[Literal[None]] = None # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 + | + = help: Replace with `Final` + +ℹ Safe fix +1 1 | from typing import Final, Literal +2 2 | +3 3 | x: Final[Literal[True]] # PYI064 +4 |-y: Final[Literal[None]] = None # PYI064 + 4 |+y: Final = None # PYI064 +5 5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 +6 6 | +7 7 | # This should be fixable, and marked as safe + +PYI064.pyi:5:1: PYI064 [*] `Final[Literal[...]]` can be replaced with a bare `Final` + | +3 | x: Final[Literal[True]] # PYI064 +4 | y: Final[Literal[None]] = None # PYI064 +5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +6 | +7 | # This should be fixable, and marked as safe + | + = help: Replace with `Final` + +ℹ Safe fix +2 2 | +3 3 | x: Final[Literal[True]] # PYI064 +4 4 | y: Final[Literal[None]] = None # PYI064 +5 |-z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 + 5 |+z: Final = "this is a really long literal, that won't be rendered in the issue text" # PYI064 +6 6 | +7 7 | # This should be fixable, and marked as safe +8 8 | w1: Final[Literal[123]] # PYI064 + +PYI064.pyi:8:1: PYI064 [*] `Final[Literal[123]]` can be replaced with a bare `Final` + | + 7 | # This should be fixable, and marked as safe + 8 | w1: Final[Literal[123]] # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI064 + 9 | +10 | # This should not be fixable + | + = help: Replace with `Final` + +ℹ Safe fix +5 5 | z: Final[Literal["this is a really long literal, that won't be rendered in the issue text"]] # PYI064 +6 6 | +7 7 | # This should be fixable, and marked as safe +8 |-w1: Final[Literal[123]] # PYI064 + 8 |+w1: Final = 123 # PYI064 +9 9 | +10 10 | # This should not be fixable +11 11 | w2: Final[Literal[123]] = "random value" # PYI064 + +PYI064.pyi:11:1: PYI064 `Final[Literal[123]]` can be replaced with a bare `Final` + | +10 | # This should not be fixable +11 | w2: Final[Literal[123]] = "random value" # PYI064 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI064 +12 | +13 | n1: Final[Literal[True, False]] # No issue here + | + = help: Replace with `Final` diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 208010a993a95..86e9d8c61b8b7 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -657,15 +657,14 @@ pub fn map_callable(decorator: &Expr) -> &Expr { } } -/// Given an [`Expr`] that can be callable or not (like a decorator, which could -/// be used with or without explicit call syntax), return the underlying -/// callable. +/// Given an [`Expr`] that can be a [`ExprSubscript`][ast::ExprSubscript] or not +/// (like an annotation that may be generic or not), return the underlying expr. pub fn map_subscript(expr: &Expr) -> &Expr { if let Expr::Subscript(ast::ExprSubscript { value, .. }) = expr { - // Ex) `Iterable[T]` + // Ex) `Iterable[T]` => return `Iterable` value } else { - // Ex) `Iterable` + // Ex) `Iterable` => return `Iterable` expr } } diff --git a/ruff.schema.json b/ruff.schema.json index 2428468dde04b..2a409bc1332b2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3580,6 +3580,7 @@ "PYI059", "PYI06", "PYI062", + "PYI064", "Q", "Q0", "Q00",