From ac028cd9f832aa88486c3fab4cc845839b8d5594 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Wed, 15 Feb 2023 00:45:12 +0100 Subject: [PATCH] [`numpy`] deprecated type aliases (#2810) Closes https://github.com/charliermarsh/ruff/issues/2455 Used `NPY` as prefix code as agreed in the issue. --- README.md | 7 + clippy.toml | 8 +- .../resources/test/fixtures/numpy/NPY001.py | 20 +++ crates/ruff/src/checkers/ast.rs | 10 +- crates/ruff/src/codes.rs | 3 + crates/ruff/src/registry.rs | 5 + crates/ruff/src/rules/mod.rs | 1 + crates/ruff/src/rules/numpy/mod.rs | 26 ++++ .../numpy/rules/deprecated_type_alias.rs | 87 +++++++++++ crates/ruff/src/rules/numpy/rules/mod.rs | 3 + ...numpy-deprecated-type-alias_NPY001.py.snap | 138 ++++++++++++++++++ crates/ruff_dev/src/generate_docs.rs | 8 +- crates/ruff_macros/src/rule_namespace.rs | 7 +- docs/rules/deprecated-type-alias.md | 26 ++++ ruff.schema.json | 4 + 15 files changed, 344 insertions(+), 9 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/numpy/NPY001.py create mode 100644 crates/ruff/src/rules/numpy/mod.rs create mode 100644 crates/ruff/src/rules/numpy/rules/deprecated_type_alias.rs create mode 100644 crates/ruff/src/rules/numpy/rules/mod.rs create mode 100644 crates/ruff/src/rules/numpy/snapshots/ruff__rules__numpy__tests__numpy-deprecated-type-alias_NPY001.py.snap create mode 100644 docs/rules/deprecated-type-alias.md diff --git a/README.md b/README.md index 14578e6cd3..4ec9f77695 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,7 @@ This README is also available as [documentation](https://beta.ruff.rs/docs/). 1. [pygrep-hooks (PGH)](#pygrep-hooks-pgh) 1. [Pylint (PL)](#pylint-pl) 1. [tryceratops (TRY)](#tryceratops-try) + 1. [NumPy-specific rules (NPY)](#numpy-specific-rules-npy) 1. [Ruff-specific rules (RUF)](#ruff-specific-rules-ruf) 1. [Editor Integrations](#editor-integrations) 1. [FAQ](#faq) @@ -1487,6 +1488,12 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI | TRY301 | raise-within-try | Abstract `raise` to an inner function | | | TRY400 | error-instead-of-exception | Use `logging.exception` instead of `logging.error` | | +### NumPy-specific rules (NPY) + +| Code | Name | Message | Fix | +| ---- | ---- | ------- | --- | +| NPY001 | [numpy-deprecated-type-alias](https://beta.ruff.rs/docs/rules/numpy-deprecated-type-alias/) | Type alias `np.{type_name}` is deprecated, replace with builtin type | 🛠 | + ### Ruff-specific rules (RUF) | Code | Name | Message | Fix | diff --git a/clippy.toml b/clippy.toml index 05fb13a21d..1ac6782ac7 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,7 @@ -doc-valid-idents = ["StackOverflow", "CodeQL", "IPython", ".."] +doc-valid-idents = [ + "StackOverflow", + "CodeQL", + "IPython", + "NumPy", + "..", +] diff --git a/crates/ruff/resources/test/fixtures/numpy/NPY001.py b/crates/ruff/resources/test/fixtures/numpy/NPY001.py new file mode 100644 index 0000000000..14ca7fb95f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/numpy/NPY001.py @@ -0,0 +1,20 @@ +import numpy as npy +import numpy as np +import numpy + +# Error +npy.bool +npy.int + +if dtype == np.object: + ... + +result = result.select_dtypes([np.byte, np.ubyte, np.short, np.ushort, np.int, np.long]) + +pdf = pd.DataFrame( + data=[[1, 2, 3]], + columns=["a", "b", "c"], + dtype=numpy.object, +) + +_ = arr.astype(np.int) diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 5443d509ad..711fa273fc 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -38,8 +38,8 @@ use crate::rules::{ flake8_django, flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_self, flake8_simplify, flake8_tidy_imports, flake8_type_checking, - flake8_unused_arguments, flake8_use_pathlib, mccabe, pandas_vet, pep8_naming, pycodestyle, - pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops, + flake8_unused_arguments, flake8_use_pathlib, mccabe, numpy, pandas_vet, pep8_naming, + pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops, }; use crate::settings::types::PythonVersion; use crate::settings::{flags, Settings}; @@ -2145,6 +2145,9 @@ where if self.settings.rules.enabled(&Rule::TypingTextStrAlias) { pyupgrade::rules::typing_text_str_alias(self, expr); } + if self.settings.rules.enabled(&Rule::NumpyDeprecatedTypeAlias) { + numpy::rules::deprecated_type_alias(self, expr); + } // Ex) List[...] if !self.in_deferred_string_type_definition @@ -2211,6 +2214,9 @@ where if self.settings.rules.enabled(&Rule::TypingTextStrAlias) { pyupgrade::rules::typing_text_str_alias(self, expr); } + if self.settings.rules.enabled(&Rule::NumpyDeprecatedTypeAlias) { + numpy::rules::deprecated_type_alias(self, expr); + } if self.settings.rules.enabled(&Rule::RewriteMockImport) { pyupgrade::rules::rewrite_mock_attribute(self, expr); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 46431f07ba..e883a9aa32 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -587,6 +587,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { // flake8-self (Flake8Self, "001") => Rule::PrivateMemberAccess, + // numpy + (Numpy, "001") => Rule::NumpyDeprecatedTypeAlias, + // ruff (Ruff, "001") => Rule::AmbiguousUnicodeCharacterString, (Ruff, "002") => Rule::AmbiguousUnicodeCharacterDocstring, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 1e24f7a098..9679f17580 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -550,6 +550,8 @@ ruff_macros::register_rules!( rules::flake8_raise::rules::UnnecessaryParenOnRaiseException, // flake8-self rules::flake8_self::rules::PrivateMemberAccess, + // numpy + rules::numpy::rules::NumpyDeprecatedTypeAlias, // ruff rules::ruff::rules::AmbiguousUnicodeCharacterString, rules::ruff::rules::AmbiguousUnicodeCharacterDocstring, @@ -695,6 +697,9 @@ pub enum Linter { /// [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) #[prefix = "TRY"] Tryceratops, + /// NumPy-specific rules + #[prefix = "NPY"] + Numpy, /// Ruff-specific rules #[prefix = "RUF"] Ruff, diff --git a/crates/ruff/src/rules/mod.rs b/crates/ruff/src/rules/mod.rs index 0cced3250b..4c3330227e 100644 --- a/crates/ruff/src/rules/mod.rs +++ b/crates/ruff/src/rules/mod.rs @@ -33,6 +33,7 @@ pub mod flake8_unused_arguments; pub mod flake8_use_pathlib; pub mod isort; pub mod mccabe; +pub mod numpy; pub mod pandas_vet; pub mod pep8_naming; pub mod pycodestyle; diff --git a/crates/ruff/src/rules/numpy/mod.rs b/crates/ruff/src/rules/numpy/mod.rs new file mode 100644 index 0000000000..76da450fb8 --- /dev/null +++ b/crates/ruff/src/rules/numpy/mod.rs @@ -0,0 +1,26 @@ +//! NumPy-specific rules. +pub(crate) mod rules; + +#[cfg(test)] +mod tests { + use std::convert::AsRef; + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::registry::Rule; + use crate::test::test_path; + use crate::{assert_yaml_snapshot, settings}; + + #[test_case(Rule::NumpyDeprecatedTypeAlias, Path::new("NPY001.py"); "NPY001")] + fn rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("numpy").join(path).as_path(), + &settings::Settings::for_rule(rule_code), + )?; + assert_yaml_snapshot!(snapshot, diagnostics); + Ok(()) + } +} diff --git a/crates/ruff/src/rules/numpy/rules/deprecated_type_alias.rs b/crates/ruff/src/rules/numpy/rules/deprecated_type_alias.rs new file mode 100644 index 0000000000..28eb1fd1c0 --- /dev/null +++ b/crates/ruff/src/rules/numpy/rules/deprecated_type_alias.rs @@ -0,0 +1,87 @@ +use ruff_macros::{define_violation, derive_message_formats}; +use rustpython_parser::ast::Expr; + +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::fix::Fix; +use crate::registry::Diagnostic; +use crate::violation::AlwaysAutofixableViolation; + +define_violation!( + /// ## What it does + /// Checks for deprecated NumPy type aliases. + /// + /// ## Why is this bad? + /// NumPy's `np.int` has long been an alias of the builtin `int`. The same + /// goes for `np.float`, `np.bool`, and others. These aliases exist + /// primarily primarily for historic reasons, and have been a cause of + /// frequent confusion for newcomers. + /// + /// These aliases were been deprecated in 1.20, and removed in 1.24. + /// + /// ## Examples + /// ```python + /// import numpy as np + /// + /// np.bool + /// ``` + /// + /// Use instead: + /// ```python + /// bool + /// ``` + pub struct NumpyDeprecatedTypeAlias { + pub type_name: String, + } +); +impl AlwaysAutofixableViolation for NumpyDeprecatedTypeAlias { + #[derive_message_formats] + fn message(&self) -> String { + let NumpyDeprecatedTypeAlias { type_name } = self; + format!("Type alias `np.{type_name}` is deprecated, replace with builtin type") + } + + fn autofix_title(&self) -> String { + let NumpyDeprecatedTypeAlias { type_name } = self; + format!("Replace `np.{type_name}` with builtin type") + } +} + +/// NPY001 +pub fn deprecated_type_alias(checker: &mut Checker, expr: &Expr) { + if let Some(type_name) = checker.resolve_call_path(expr).and_then(|call_path| { + if call_path.as_slice() == ["numpy", "bool"] + || call_path.as_slice() == ["numpy", "int"] + || call_path.as_slice() == ["numpy", "float"] + || call_path.as_slice() == ["numpy", "complex"] + || call_path.as_slice() == ["numpy", "object"] + || call_path.as_slice() == ["numpy", "str"] + || call_path.as_slice() == ["numpy", "long"] + || call_path.as_slice() == ["numpy", "unicode"] + { + Some(call_path[1]) + } else { + None + } + }) { + let mut diagnostic = Diagnostic::new( + NumpyDeprecatedTypeAlias { + type_name: type_name.to_string(), + }, + Range::from_located(expr), + ); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.amend(Fix::replacement( + match type_name { + "unicode" => "str", + "long" => "int", + _ => type_name, + } + .to_string(), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.diagnostics.push(diagnostic); + } +} diff --git a/crates/ruff/src/rules/numpy/rules/mod.rs b/crates/ruff/src/rules/numpy/rules/mod.rs new file mode 100644 index 0000000000..b99bbe6bfd --- /dev/null +++ b/crates/ruff/src/rules/numpy/rules/mod.rs @@ -0,0 +1,3 @@ +pub use deprecated_type_alias::{deprecated_type_alias, NumpyDeprecatedTypeAlias}; + +mod deprecated_type_alias; diff --git a/crates/ruff/src/rules/numpy/snapshots/ruff__rules__numpy__tests__numpy-deprecated-type-alias_NPY001.py.snap b/crates/ruff/src/rules/numpy/snapshots/ruff__rules__numpy__tests__numpy-deprecated-type-alias_NPY001.py.snap new file mode 100644 index 0000000000..86361f6253 --- /dev/null +++ b/crates/ruff/src/rules/numpy/snapshots/ruff__rules__numpy__tests__numpy-deprecated-type-alias_NPY001.py.snap @@ -0,0 +1,138 @@ +--- +source: crates/ruff/src/rules/numpy/mod.rs +expression: diagnostics +--- +- kind: + NumpyDeprecatedTypeAlias: + type_name: bool + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 8 + fix: + content: + - bool + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 8 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: int + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 7 + fix: + content: + - int + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 7 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: object + location: + row: 9 + column: 12 + end_location: + row: 9 + column: 21 + fix: + content: + - object + location: + row: 9 + column: 12 + end_location: + row: 9 + column: 21 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: int + location: + row: 12 + column: 71 + end_location: + row: 12 + column: 77 + fix: + content: + - int + location: + row: 12 + column: 71 + end_location: + row: 12 + column: 77 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: long + location: + row: 12 + column: 79 + end_location: + row: 12 + column: 86 + fix: + content: + - int + location: + row: 12 + column: 79 + end_location: + row: 12 + column: 86 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: object + location: + row: 17 + column: 10 + end_location: + row: 17 + column: 22 + fix: + content: + - object + location: + row: 17 + column: 10 + end_location: + row: 17 + column: 22 + parent: ~ +- kind: + NumpyDeprecatedTypeAlias: + type_name: int + location: + row: 20 + column: 15 + end_location: + row: 20 + column: 21 + fix: + content: + - int + location: + row: 20 + column: 15 + end_location: + row: 20 + column: 21 + parent: ~ + diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs index c75c9f67df..6a31eaa85a 100644 --- a/crates/ruff_dev/src/generate_docs.rs +++ b/crates/ruff_dev/src/generate_docs.rs @@ -27,9 +27,11 @@ pub fn main(args: &Args) -> Result<()> { output.push('\n'); let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap(); - output.push_str(&format!("Derived from the **{}** linter.", linter.name())); - output.push('\n'); - output.push('\n'); + if linter.url().is_some() { + output.push_str(&format!("Derived from the **{}** linter.", linter.name())); + output.push('\n'); + output.push('\n'); + } if let Some(autofix) = rule.autofixable() { output.push_str(match autofix.available { diff --git a/crates/ruff_macros/src/rule_namespace.rs b/crates/ruff_macros/src/rule_namespace.rs index ad55cc9f1f..1fc915efb0 100644 --- a/crates/ruff_macros/src/rule_namespace.rs +++ b/crates/ruff_macros/src/rule_namespace.rs @@ -15,8 +15,9 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result let mut parsed = Vec::new(); let mut common_prefix_match_arms = quote!(); - let mut name_match_arms = quote!(Self::Ruff => "Ruff-specific rules",); - let mut url_match_arms = quote!(Self::Ruff => None,); + let mut name_match_arms = + quote!(Self::Ruff => "Ruff-specific rules", Self::Numpy => "NumPy-specific rules", ); + let mut url_match_arms = quote!(Self::Ruff => None, Self::Numpy => None, ); let mut all_prefixes = HashSet::new(); @@ -58,7 +59,7 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result let variant_ident = variant.ident; - if variant_ident != "Ruff" { + if variant_ident != "Ruff" && variant_ident != "Numpy" { let (name, url) = parse_doc_attr(doc_attr)?; name_match_arms.extend(quote! {Self::#variant_ident => #name,}); url_match_arms.extend(quote! {Self::#variant_ident => Some(#url),}); diff --git a/docs/rules/deprecated-type-alias.md b/docs/rules/deprecated-type-alias.md new file mode 100644 index 0000000000..c5822bb84b --- /dev/null +++ b/docs/rules/deprecated-type-alias.md @@ -0,0 +1,26 @@ +# deprecated-type-alias (NPY001) + +Autofix is always available. + +## What it does +Checks for deprecated NumPy type aliases. + +## Why is this bad? +NumPy's `np.int` has long been an alias of the builtin `int`. The same +goes for `np.float`, `np.bool`, and others. These aliases exist +primarily primarily for historic reasons, and have been a cause of +frequent confusion for newcomers. + +These aliases were been deprecated in 1.20, and removed in 1.24. + +## Examples +```python +import numpy as np + +np.bool +``` + +Use instead: +```python +bool +``` \ No newline at end of file diff --git a/ruff.schema.json b/ruff.schema.json index 40959d62be..a81715c809 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1656,6 +1656,10 @@ "N816", "N817", "N818", + "NPY", + "NPY0", + "NPY00", + "NPY001", "PD", "PD0", "PD00",