From c5722d8a4d4ed588868c7de4a7a4d5ae03fff328 Mon Sep 17 00:00:00 2001 From: Martin Lehoux Date: Fri, 18 Nov 2022 18:10:47 +0100 Subject: [PATCH] Implement U013: Unnecessary TypedDict syntactic form (#716) --- README.md | 1 + resources/test/fixtures/U013.py | 29 +++ src/check_ast.rs | 5 + src/checks.rs | 9 + src/checks_gen.rs | 12 +- src/flake8_bugbear/mod.rs | 1 - .../plugins/getattr_with_constant.rs | 2 +- .../plugins/setattr_with_constant.rs | 2 +- src/linter.rs | 1 + .../constants.rs => python/identifiers.rs} | 0 src/python/mod.rs | 1 + .../convert_typed_dict_functional_to_class.rs | 232 ++++++++++++++++++ src/pyupgrade/plugins/mod.rs | 2 + .../ruff__linter__tests__U013_U013.py.snap | 158 ++++++++++++ 14 files changed, 451 insertions(+), 4 deletions(-) create mode 100644 resources/test/fixtures/U013.py rename src/{flake8_bugbear/constants.rs => python/identifiers.rs} (100%) create mode 100644 src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs create mode 100644 src/snapshots/ruff__linter__tests__U013_U013.py.snap diff --git a/README.md b/README.md index 663aa420ad..70e2410bc7 100644 --- a/README.md +++ b/README.md @@ -451,6 +451,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | U010 | UnnecessaryFutureImport | Unnecessary `__future__` import `...` for target Python version | 🛠 | | U011 | UnnecessaryLRUCacheParams | Unnecessary parameters to `functools.lru_cache` | 🛠 | | U012 | UnnecessaryEncodeUTF8 | Unnecessary call to `encode` as UTF-8 | 🛠 | +| U013 | ConvertTypedDictFunctionalToClass | Convert `TypedDict` functional syntax to class syntax | 🛠 | ### pep8-naming diff --git a/resources/test/fixtures/U013.py b/resources/test/fixtures/U013.py new file mode 100644 index 0000000000..b73b14a258 --- /dev/null +++ b/resources/test/fixtures/U013.py @@ -0,0 +1,29 @@ +from typing import TypedDict, NotRequired, Literal + +# dict literal +MyType1 = TypedDict('MyType1', {'a': int, 'b': str}) + +# dict call +MyType2 = TypedDict('MyType2', dict(a=int, b=str)) + +# kwargs +MyType3 = TypedDict('MyType3', a=int, b=str) + +# Empty TypedDict +MyType4 = TypedDict('MyType4') + +# Literal values +MyType5 = TypedDict('MyType5', {'a': 'hello'}) +MyType6 = TypedDict('MyType6', a='hello') + +# NotRequired +MyType7 = TypedDict('MyType7', {'a': NotRequired[dict]}) + +# total +MyType8 = TypedDict('MyType8', {'x': int, 'y': int}, total=False) + +# invalid identifiers +MyType9 = TypedDict('MyType9', {'in': int, 'x-y': int}) + +# using Literal type +MyType10 = TypedDict('MyType10', {'key': Literal['value']}) diff --git a/src/check_ast.rs b/src/check_ast.rs index 1a93893495..8928165837 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -945,6 +945,11 @@ where self.add_check(check); } } + if self.settings.enabled.contains(&CheckCode::U013) { + pyupgrade::plugins::convert_typed_dict_functional_to_class( + self, stmt, targets, value, + ); + } } StmtKind::AnnAssign { target, value, .. } => { if self.settings.enabled.contains(&CheckCode::E731) { diff --git a/src/checks.rs b/src/checks.rs index 4271cc0c50..45b756f735 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -167,6 +167,7 @@ pub enum CheckCode { U010, U011, U012, + U013, // pydocstyle D100, D101, @@ -481,6 +482,7 @@ pub enum CheckKind { UnnecessaryFutureImport(Vec), UnnecessaryLRUCacheParams, UnnecessaryEncodeUTF8, + ConvertTypedDictFunctionalToClass, // pydocstyle BlankLineAfterLastSection(String), BlankLineAfterSection(String), @@ -747,6 +749,7 @@ impl CheckCode { CheckCode::U010 => CheckKind::UnnecessaryFutureImport(vec!["...".to_string()]), CheckCode::U011 => CheckKind::UnnecessaryLRUCacheParams, CheckCode::U012 => CheckKind::UnnecessaryEncodeUTF8, + CheckCode::U013 => CheckKind::ConvertTypedDictFunctionalToClass, // pydocstyle CheckCode::D100 => CheckKind::PublicModule, CheckCode::D101 => CheckKind::PublicClass, @@ -972,6 +975,7 @@ impl CheckCode { CheckCode::U010 => CheckCategory::Pyupgrade, CheckCode::U011 => CheckCategory::Pyupgrade, CheckCode::U012 => CheckCategory::Pyupgrade, + CheckCode::U013 => CheckCategory::Pyupgrade, CheckCode::D100 => CheckCategory::Pydocstyle, CheckCode::D101 => CheckCategory::Pydocstyle, CheckCode::D102 => CheckCategory::Pydocstyle, @@ -1189,6 +1193,7 @@ impl CheckKind { CheckKind::UnnecessaryFutureImport(_) => &CheckCode::U010, CheckKind::UnnecessaryLRUCacheParams => &CheckCode::U011, CheckKind::UnnecessaryEncodeUTF8 => &CheckCode::U012, + CheckKind::ConvertTypedDictFunctionalToClass => &CheckCode::U013, // pydocstyle CheckKind::BlankLineAfterLastSection(_) => &CheckCode::D413, CheckKind::BlankLineAfterSection(_) => &CheckCode::D410, @@ -1733,6 +1738,9 @@ impl CheckKind { "Unnecessary parameters to `functools.lru_cache`".to_string() } CheckKind::UnnecessaryEncodeUTF8 => "Unnecessary call to `encode` as UTF-8".to_string(), + CheckKind::ConvertTypedDictFunctionalToClass => { + "Convert `TypedDict` functional syntax to class syntax".to_string() + } // pydocstyle CheckKind::FitsOnOneLine => "One-line docstring should fit on one line".to_string(), CheckKind::BlankLineAfterSummary => { @@ -2018,6 +2026,7 @@ impl CheckKind { | CheckKind::UnnecessaryCollectionCall(_) | CheckKind::UnnecessaryComprehension(_) | CheckKind::UnnecessaryEncodeUTF8 + | CheckKind::ConvertTypedDictFunctionalToClass | CheckKind::UnnecessaryFutureImport(_) | CheckKind::UnnecessaryGeneratorDict | CheckKind::UnnecessaryGeneratorList diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 135992b939..c175eb7efc 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -286,6 +286,7 @@ pub enum CheckCodePrefix { U010, U011, U012, + U013, W, W2, W29, @@ -1089,6 +1090,7 @@ impl CheckCodePrefix { CheckCode::U010, CheckCode::U011, CheckCode::U012, + CheckCode::U013, ], CheckCodePrefix::U0 => vec![ CheckCode::U001, @@ -1102,6 +1104,7 @@ impl CheckCodePrefix { CheckCode::U010, CheckCode::U011, CheckCode::U012, + CheckCode::U013, ], CheckCodePrefix::U00 => vec![ CheckCode::U001, @@ -1121,10 +1124,16 @@ impl CheckCodePrefix { CheckCodePrefix::U007 => vec![CheckCode::U007], CheckCodePrefix::U008 => vec![CheckCode::U008], CheckCodePrefix::U009 => vec![CheckCode::U009], - CheckCodePrefix::U01 => vec![CheckCode::U010, CheckCode::U011, CheckCode::U012], + CheckCodePrefix::U01 => vec![ + CheckCode::U010, + CheckCode::U011, + CheckCode::U012, + CheckCode::U013, + ], CheckCodePrefix::U010 => vec![CheckCode::U010], CheckCodePrefix::U011 => vec![CheckCode::U011], CheckCodePrefix::U012 => vec![CheckCode::U012], + CheckCodePrefix::U013 => vec![CheckCode::U013], CheckCodePrefix::W => vec![CheckCode::W292, CheckCode::W605], CheckCodePrefix::W2 => vec![CheckCode::W292], CheckCodePrefix::W29 => vec![CheckCode::W292], @@ -1456,6 +1465,7 @@ impl CheckCodePrefix { CheckCodePrefix::U010 => PrefixSpecificity::Explicit, CheckCodePrefix::U011 => PrefixSpecificity::Explicit, CheckCodePrefix::U012 => PrefixSpecificity::Explicit, + CheckCodePrefix::U013 => PrefixSpecificity::Explicit, CheckCodePrefix::W => PrefixSpecificity::Category, CheckCodePrefix::W2 => PrefixSpecificity::Hundreds, CheckCodePrefix::W29 => PrefixSpecificity::Tens, diff --git a/src/flake8_bugbear/mod.rs b/src/flake8_bugbear/mod.rs index 407ecd7513..a1763da717 100644 --- a/src/flake8_bugbear/mod.rs +++ b/src/flake8_bugbear/mod.rs @@ -1,4 +1,3 @@ -mod constants; pub mod plugins; pub mod settings; diff --git a/src/flake8_bugbear/plugins/getattr_with_constant.rs b/src/flake8_bugbear/plugins/getattr_with_constant.rs index 0ed1c0c3c4..e5fce9accf 100644 --- a/src/flake8_bugbear/plugins/getattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/getattr_with_constant.rs @@ -5,7 +5,7 @@ use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; use crate::code_gen::SourceGenerator; -use crate::flake8_bugbear::constants::IDENTIFIER_REGEX; +use crate::python::identifiers::IDENTIFIER_REGEX; use crate::python::keyword::KWLIST; fn attribute(value: &Expr, attr: &str) -> Expr { diff --git a/src/flake8_bugbear/plugins/setattr_with_constant.rs b/src/flake8_bugbear/plugins/setattr_with_constant.rs index 479014ece6..782c2f7b2c 100644 --- a/src/flake8_bugbear/plugins/setattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/setattr_with_constant.rs @@ -3,7 +3,7 @@ use rustpython_ast::{Constant, Expr, ExprKind}; use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; -use crate::flake8_bugbear::constants::IDENTIFIER_REGEX; +use crate::python::identifiers::IDENTIFIER_REGEX; use crate::python::keyword::KWLIST; /// B010 diff --git a/src/linter.rs b/src/linter.rs index 1634c8f511..7a5a173ec3 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -501,6 +501,7 @@ mod tests { #[test_case(CheckCode::U011, Path::new("U011_0.py"); "U011_0")] #[test_case(CheckCode::U011, Path::new("U011_1.py"); "U011_1")] #[test_case(CheckCode::U012, Path::new("U012.py"); "U012")] + #[test_case(CheckCode::U013, Path::new("U013.py"); "U013")] #[test_case(CheckCode::W292, Path::new("W292_0.py"); "W292_0")] #[test_case(CheckCode::W292, Path::new("W292_1.py"); "W292_1")] #[test_case(CheckCode::W292, Path::new("W292_2.py"); "W292_2")] diff --git a/src/flake8_bugbear/constants.rs b/src/python/identifiers.rs similarity index 100% rename from src/flake8_bugbear/constants.rs rename to src/python/identifiers.rs diff --git a/src/python/mod.rs b/src/python/mod.rs index f609cb82c7..8659ae3c4f 100644 --- a/src/python/mod.rs +++ b/src/python/mod.rs @@ -1,5 +1,6 @@ pub mod builtins; pub mod future; +pub mod identifiers; pub mod keyword; pub mod string; pub mod sys; diff --git a/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs b/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs new file mode 100644 index 0000000000..276699e01b --- /dev/null +++ b/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs @@ -0,0 +1,232 @@ +use anyhow::{anyhow, bail, Result}; +use log::error; +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Keyword, KeywordData, Stmt, StmtKind}; + +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; +use crate::code_gen::SourceGenerator; +use crate::python::identifiers::IDENTIFIER_REGEX; +use crate::python::keyword::KWLIST; + +// Returns (class_name, args, keywords) +fn match_typed_dict_assign<'a>( + targets: &'a [Expr], + value: &'a Expr, +) -> Option<(&'a str, &'a [Expr], &'a [Keyword])> { + if let Some(target) = targets.get(0) { + if let ExprKind::Name { id: class_name, .. } = &target.node { + if let ExprKind::Call { + func, + args, + keywords, + } = &value.node + { + if let ExprKind::Name { id: func_name, .. } = &func.node { + if func_name == "TypedDict" { + return Some((class_name, args, keywords)); + } + } + } + } + } + None +} + +fn create_property_assignment_stmt(property: &String, type_ann: &ExprKind) -> Stmt { + Stmt::new( + Default::default(), + Default::default(), + StmtKind::AnnAssign { + target: Box::new(Expr::new( + Default::default(), + Default::default(), + ExprKind::Name { + id: property.to_string(), + ctx: ExprContext::Load, + }, + )), + annotation: Box::new(Expr::new( + Default::default(), + Default::default(), + type_ann.clone(), + )), + value: None, + simple: 1, + }, + ) +} + +fn create_pass_stmt() -> Stmt { + Stmt::new(Default::default(), Default::default(), StmtKind::Pass) +} + +fn create_classdef_stmt( + class_name: &str, + body: Vec, + total_keyword: Option, +) -> Stmt { + let keywords = match total_keyword { + Some(keyword) => vec![Keyword::new( + Default::default(), + Default::default(), + keyword, + )], + None => vec![], + }; + Stmt::new( + Default::default(), + Default::default(), + StmtKind::ClassDef { + name: class_name.to_string(), + bases: vec![Expr::new( + Default::default(), + Default::default(), + ExprKind::Name { + id: "TypedDict".to_string(), + ctx: ExprContext::Load, + }, + )], + keywords, + body, + decorator_list: vec![], + }, + ) +} + +fn get_properties_from_dict_literal(keys: &[Expr], values: &[Expr]) -> Result> { + keys.iter() + .zip(values.iter()) + .map(|(key, value)| match &key.node { + ExprKind::Constant { + value: Constant::Str(property), + .. + } => { + if IDENTIFIER_REGEX.is_match(property) && !KWLIST.contains(&property.as_str()) { + Ok(create_property_assignment_stmt(property, &value.node)) + } else { + bail!("Invalid property name: {}", property) + } + } + _ => bail!("key is not a Str"), + }) + .collect() +} + +fn get_properties_from_dict_call(func: &Expr, keywords: &[Keyword]) -> Result> { + match &func.node { + ExprKind::Name { id: func_name, .. } => match func_name.as_str() { + "dict" => keywords + .iter() + .map(|keyword| { + let property = &keyword.node.arg.clone().ok_or_else(|| anyhow!("no arg"))?; + Ok(create_property_assignment_stmt( + property, + &keyword.node.value.node, + )) + }) + .collect(), + _ => bail!("func is not dict"), + }, + _ => bail!("func is not a Name"), + } +} + +// Deprecated in Python 3.11, Removed in Python 3.13? +fn get_properties_from_keywords(keywords: &[Keyword]) -> Result> { + keywords + .iter() + .map(|keyword| { + let property = &keyword.node.arg.clone().ok_or_else(|| anyhow!("no arg"))?; + Ok(create_property_assignment_stmt( + property, + &keyword.node.value.node, + )) + }) + .collect() +} + +// The only way to have total keyword is to use the arg version +// (`TypedDict('name', {'a': int}, total=True)`) +fn get_total_from_only_keyword(keywords: &[Keyword]) -> Option { + match keywords.get(0) { + Some(keyword) => match &keyword.node.arg { + Some(arg) => match arg.as_str() { + "total" => Some(keyword.node.clone()), + _ => None, + }, + None => None, + }, + None => None, + } +} + +fn get_properties_and_total( + args: &[Expr], + keywords: &[Keyword], +) -> Result<(Vec, Option)> { + if let Some(dict) = args.get(1) { + let total = get_total_from_only_keyword(keywords); + match &dict.node { + ExprKind::Dict { keys, values } => { + Ok((get_properties_from_dict_literal(keys, values)?, total)) + } + ExprKind::Call { func, keywords, .. } => { + Ok((get_properties_from_dict_call(func, keywords)?, total)) + } + _ => Ok((vec![create_pass_stmt()], total)), + } + } else if !keywords.is_empty() { + Ok((get_properties_from_keywords(keywords)?, None)) + } else { + Ok((vec![create_pass_stmt()], None)) + } +} + +fn try_to_fix( + stmt: &Stmt, + class_name: &str, + body: Vec, + total_keyword: Option, +) -> Result { + // We don't have to manage an hybrid case because it's not possible to have a + // dict and keywords Illegal: `MyType = TypedDict('MyType', {'a': int, 'b': + // str}, a=int, b=str)` + let classdef_stmt = create_classdef_stmt(class_name, body, total_keyword); + let mut generator = SourceGenerator::new(); + generator.unparse_stmt(&classdef_stmt)?; + let content = generator.generate()?; + Ok(Fix::replacement( + content, + stmt.location, + stmt.end_location.unwrap(), + )) +} + +/// U013 +pub fn convert_typed_dict_functional_to_class( + checker: &mut Checker, + stmt: &Stmt, + targets: &[Expr], + value: &Expr, +) { + if let Some((class_name, args, keywords)) = match_typed_dict_assign(targets, value) { + match get_properties_and_total(args, keywords) { + Err(err) => error!("Failed to parse TypedDict: {}", err), + Ok((body, total_keyword)) => { + let mut check = Check::new( + CheckKind::ConvertTypedDictFunctionalToClass, + Range::from_located(stmt), + ); + if checker.patch() { + match try_to_fix(stmt, class_name, body, total_keyword) { + Ok(fix) => check.amend(fix), + Err(err) => error!("Failed to convert TypedDict: {}", err), + }; + } + checker.add_check(check); + } + } + } +} diff --git a/src/pyupgrade/plugins/mod.rs b/src/pyupgrade/plugins/mod.rs index a22e0744a4..6e363ba85d 100644 --- a/src/pyupgrade/plugins/mod.rs +++ b/src/pyupgrade/plugins/mod.rs @@ -1,3 +1,4 @@ +pub use convert_typed_dict_functional_to_class::convert_typed_dict_functional_to_class; pub use deprecated_unittest_alias::deprecated_unittest_alias; pub use super_call_with_parameters::super_call_with_parameters; pub use type_of_primitive::type_of_primitive; @@ -9,6 +10,7 @@ pub use use_pep604_annotation::use_pep604_annotation; pub use useless_metaclass_type::useless_metaclass_type; pub use useless_object_inheritance::useless_object_inheritance; +mod convert_typed_dict_functional_to_class; mod deprecated_unittest_alias; mod super_call_with_parameters; mod type_of_primitive; diff --git a/src/snapshots/ruff__linter__tests__U013_U013.py.snap b/src/snapshots/ruff__linter__tests__U013_U013.py.snap new file mode 100644 index 0000000000..ee60463df2 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__U013_U013.py.snap @@ -0,0 +1,158 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertTypedDictFunctionalToClass + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 52 + fix: + patch: + content: "\nclass MyType1(TypedDict):\n a: int\n b: str" + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 52 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 50 + fix: + patch: + content: "\nclass MyType2(TypedDict):\n a: int\n b: str" + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 50 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 44 + fix: + patch: + content: "\nclass MyType3(TypedDict):\n a: int\n b: str" + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 44 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 30 + fix: + patch: + content: "\nclass MyType4(TypedDict):\n pass" + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 30 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 16 + column: 0 + end_location: + row: 16 + column: 46 + fix: + patch: + content: "\nclass MyType5(TypedDict):\n a: 'hello'" + location: + row: 16 + column: 0 + end_location: + row: 16 + column: 46 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 41 + fix: + patch: + content: "\nclass MyType6(TypedDict):\n a: 'hello'" + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 41 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 20 + column: 0 + end_location: + row: 20 + column: 56 + fix: + patch: + content: "\nclass MyType7(TypedDict):\n a: NotRequired[dict]" + location: + row: 20 + column: 0 + end_location: + row: 20 + column: 56 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 23 + column: 0 + end_location: + row: 23 + column: 65 + fix: + patch: + content: "\nclass MyType8(TypedDict, total=False):\n x: int\n y: int" + location: + row: 23 + column: 0 + end_location: + row: 23 + column: 65 + applied: false +- kind: ConvertTypedDictFunctionalToClass + location: + row: 29 + column: 0 + end_location: + row: 29 + column: 59 + fix: + patch: + content: "\nclass MyType10(TypedDict):\n key: Literal['value']" + location: + row: 29 + column: 0 + end_location: + row: 29 + column: 59 + applied: false +