[flake8-import-conventions] Syntax check aliases supplied in configuration for unconventional-import-alias (ICN001)
#14477
Conversation
|
Two notes:
|
|
| @@ -42,6 +43,7 @@ serde = { workspace = true } | |||
| shellexpand = { workspace = true } | |||
| strum = { workspace = true } | |||
| toml = { workspace = true } | |||
| serde_json.workspace = true | |||
There was a problem hiding this comment.
This should be a dev-dependency if it is only used in tests
MichaReiser
added
bug
| #[test] | ||
| fn flake8_import_conventions_validate_aliases() { | ||
| let json_options = r#"{"aliases": {"a.b":"a.b"}}"#; | ||
| let result: Result<Flake8ImportConventionsOptions, _> = serde_json::from_str(json_options); | ||
| assert!(result.unwrap_err().to_string().contains("Module must be valid identifier separated by single periods and alias must be valid identifier")); | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm leaning towards making this a ruff cli integration test. It then also demonstrates how the error message is displayed to the users.
See
ruff/crates/ruff/tests/lint.rs
Line 23 in c847cad
| if module.is_empty() | ||
| || module.split('.').any(|part| !is_identifier(part)) | ||
| || !is_identifier(alias) | ||
| { | ||
| return Err(de::Error::custom( | ||
| "Module must be valid identifier separated by single periods and alias must be valid identifier", | ||
| )); | ||
| } |
There was a problem hiding this comment.
I suggest that we use two dedicated errors: One for alias and one for module.
|
Let's try to get this into ruff 0.8 if we can — while it is a bugfix, it might still be breaking for some users who are currently "using the rule incorrectly" |
AlexWaygood
requested review from
carljm,
AlexWaygood and
sharkdp
as code owners
|
Sorry — I optimistically tried to change the target branch, but it messed up the diff, so I swiftly reverted that back 😅 It's probably fine if this goes straight into |
This PR introduces a validation step to the deserialization of the configuration for unconventional-import-alias (ICN001). We verify that the import module and alias supplied have valid syntax in order to avoid a panic if these aliases are added in during a fix.
Closes #14439