From 2efa3fbb627cc6ebed71283195f59c4e2caddd9d Mon Sep 17 00:00:00 2001 From: Dylan <53534755+dylwil3@users.noreply.github.com> Date: Thu, 21 Nov 2024 09:54:49 -0600 Subject: [PATCH] [`flake8-import-conventions`] Syntax check aliases supplied in configuration for `unconventional-import-alias (ICN001)` (#14477) Co-authored-by: Micha Reiser Co-authored-by: Alex Waygood --- Cargo.lock | 1 + crates/ruff/tests/lint.rs | 72 ++++++++++++++++++++++++++++ crates/ruff_workspace/Cargo.toml | 1 + crates/ruff_workspace/src/options.rs | 67 ++++++++++++++++++++++++-- ruff.schema.json | 5 +- 5 files changed, 141 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f641455da69e..0ad4e8129fe2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3072,6 +3072,7 @@ dependencies = [ "ruff_python_ast", "ruff_python_formatter", "ruff_python_semantic", + "ruff_python_stdlib", "ruff_source_file", "rustc-hash 2.0.0", "schemars", diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 336e888eb6d89..8c7ae672d8002 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -1966,3 +1966,75 @@ fn nested_implicit_namespace_package() -> Result<()> { Ok(()) } + +#[test] +fn flake8_import_convention_invalid_aliases_config_alias_name() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +[lint.flake8-import-conventions.aliases] +"module.name" = "invalid.alias" +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + , @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to parse [TMP]/ruff.toml + Cause: TOML parse error at line 2, column 2 + | + 2 | [lint.flake8-import-conventions.aliases] + | ^^^^ + invalid value: string "invalid.alias", expected a Python identifier + "###);}); + Ok(()) +} + +#[test] +fn flake8_import_convention_invalid_aliases_config_module_name() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +[lint.flake8-import-conventions.aliases] +"module..invalid" = "alias" +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + , @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to parse [TMP]/ruff.toml + Cause: TOML parse error at line 2, column 2 + | + 2 | [lint.flake8-import-conventions.aliases] + | ^^^^ + invalid value: string "module..invalid", expected a sequence of Python identifiers delimited by periods + "###);}); + Ok(()) +} diff --git a/crates/ruff_workspace/Cargo.toml b/crates/ruff_workspace/Cargo.toml index df81cd034764a..d7d247752b926 100644 --- a/crates/ruff_workspace/Cargo.toml +++ b/crates/ruff_workspace/Cargo.toml @@ -21,6 +21,7 @@ ruff_macros = { workspace = true } ruff_python_ast = { workspace = true } ruff_python_formatter = { workspace = true, features = ["serde"] } ruff_python_semantic = { workspace = true, features = ["serde"] } +ruff_python_stdlib = {workspace = true} ruff_source_file = { workspace = true } anyhow = { workspace = true } diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index c91c256c2a397..7da7f9dd5e19a 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -1,6 +1,7 @@ use regex::Regex; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; -use serde::{Deserialize, Serialize}; +use serde::de::{self}; +use serde::{Deserialize, Deserializer, Serialize}; use std::collections::BTreeMap; use std::path::PathBuf; use strum::IntoEnumIterator; @@ -34,6 +35,7 @@ use ruff_macros::{CombineOptions, OptionsMetadata}; use ruff_python_ast::name::Name; use ruff_python_formatter::{DocstringCodeLineWidth, QuoteStyle}; use ruff_python_semantic::NameImports; +use ruff_python_stdlib::identifiers::is_identifier; #[derive(Clone, Debug, PartialEq, Eq, Default, OptionsMetadata, Serialize, Deserialize)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] @@ -1327,7 +1329,7 @@ pub struct Flake8ImportConventionsOptions { scipy = "sp" "# )] - pub aliases: Option>, + pub aliases: Option>, /// A mapping from module to conventional import alias. These aliases will /// be added to the [`aliases`](#lint_flake8-import-conventions_aliases) mapping. @@ -1370,10 +1372,67 @@ pub struct Flake8ImportConventionsOptions { pub banned_from: Option>, } +#[derive(Clone, Debug, PartialEq, Eq, Hash, Default, Serialize)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct ModuleName(String); + +impl ModuleName { + pub fn into_string(self) -> String { + self.0 + } +} + +impl<'de> Deserialize<'de> for ModuleName { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let name = String::deserialize(deserializer)?; + if name.is_empty() || name.split('.').any(|part| !is_identifier(part)) { + Err(de::Error::invalid_value( + de::Unexpected::Str(&name), + &"a sequence of Python identifiers delimited by periods", + )) + } else { + Ok(Self(name)) + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, Default, Serialize)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct Alias(String); + +impl Alias { + pub fn into_string(self) -> String { + self.0 + } +} + +impl<'de> Deserialize<'de> for Alias { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let name = String::deserialize(deserializer)?; + if is_identifier(&name) { + Ok(Self(name)) + } else { + Err(de::Error::invalid_value( + de::Unexpected::Str(&name), + &"a Python identifier", + )) + } + } +} + impl Flake8ImportConventionsOptions { pub fn into_settings(self) -> flake8_import_conventions::settings::Settings { - let mut aliases = match self.aliases { - Some(options_aliases) => options_aliases, + let mut aliases: FxHashMap = match self.aliases { + Some(options_aliases) => options_aliases + .into_iter() + .map(|(module, alias)| (module.into_string(), alias.into_string())) + .collect(), None => flake8_import_conventions::settings::default_aliases(), }; if let Some(extend_aliases) = self.extend_aliases { diff --git a/ruff.schema.json b/ruff.schema.json index 48a44c9f6bc7b..afc275e427ec9 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -745,6 +745,9 @@ }, "additionalProperties": false, "definitions": { + "Alias": { + "type": "string" + }, "AnalyzeOptions": { "description": "Configures Ruff's `analyze` command.", "type": "object", @@ -1134,7 +1137,7 @@ "null" ], "additionalProperties": { - "type": "string" + "$ref": "#/definitions/Alias" } }, "banned-aliases": {