Skip to content

Commit

Permalink
[unconventional-import-alias] Fix infinite loop between ICN001 and …
Browse files Browse the repository at this point in the history
…I002 (`ICN001`) (#15480)

## Summary

This fixes the infinite loop reported in #14389 by raising an error to
the user about conflicting ICN001 (`unconventional-import-alias`) and
I002 (`missing-required-import`) configuration options.

## Test Plan

Added a CLI integration test reproducing the old behavior and then
confirming the fix.

Closes #14389

---------

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
ntBre and AlexWaygood authored Jan 16, 2025
1 parent ca3b210 commit e2da33a
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 12 deletions.
14 changes: 14 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2160,3 +2160,17 @@ fn flake8_import_convention_invalid_aliases_config_module_name() -> Result<()> {
"#);});
Ok(())
}

#[test]
fn flake8_import_convention_unused_aliased_import() {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.arg("--config")
.arg(r#"lint.isort.required-imports = ["import pandas"]"#)
.args(["--select", "I002,ICN001,F401"])
.args(["--stdin-filename", "test.py"])
.arg("--unsafe-fixes")
.arg("--fix")
.arg("-")
.pass_stdin("1"));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
source: crates/ruff/tests/lint.rs
info:
program: ruff
args:
- check
- "--no-cache"
- "--output-format"
- concise
- "--config"
- "lint.isort.required-imports = [\"import pandas\"]"
- "--select"
- "I002,ICN001,F401"
- "--stdin-filename"
- test.py
- "--unsafe-fixes"
- "--fix"
- "-"
stdin: "1"
snapshot_kind: text
---
success: false
exit_code: 2
----- stdout -----

----- stderr -----
ruff failed
Cause: Required import specified in `lint.isort.required-imports` (I002) conflicts with the required import alias specified in either `lint.flake8-import-conventions.aliases` or `lint.flake8-import-conventions.extend-aliases` (ICN001):
- `pandas` -> `pd`

Help: Remove the required import or alias from your configuration.
2 changes: 1 addition & 1 deletion crates/ruff_python_semantic/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl NameImport {
}

/// Returns the [`QualifiedName`] of the imported name (e.g., given `from foo import bar as baz`, returns `["foo", "bar"]`).
fn qualified_name(&self) -> QualifiedName {
pub fn qualified_name(&self) -> QualifiedName {
match self {
NameImport::Import(import) => QualifiedName::user_defined(&import.name.name),
NameImport::ImportFrom(import_from) => collect_import_from_member(
Expand Down
58 changes: 47 additions & 11 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use ruff_linter::line_width::{IndentWidth, LineLength};
use ruff_linter::registry::RuleNamespace;
use ruff_linter::registry::{Rule, RuleSet, INCOMPATIBLE_CODES};
use ruff_linter::rule_selector::{PreviewOptions, Specificity};
use ruff_linter::rules::pycodestyle;
use ruff_linter::rules::{flake8_import_conventions, isort, pycodestyle};
use ruff_linter::settings::fix_safety_table::FixSafetyTable;
use ruff_linter::settings::rule_table::RuleTable;
use ruff_linter::settings::types::{
Expand Down Expand Up @@ -232,6 +232,21 @@ impl Configuration {

let line_length = self.line_length.unwrap_or_default();

let rules = lint.as_rule_table(lint_preview)?;

// LinterSettings validation
let isort = lint
.isort
.map(IsortOptions::try_into_settings)
.transpose()?
.unwrap_or_default();
let flake8_import_conventions = lint
.flake8_import_conventions
.map(Flake8ImportConventionsOptions::into_settings)
.unwrap_or_default();

conflicting_import_settings(&isort, &flake8_import_conventions)?;

Ok(Settings {
cache_dir: self
.cache_dir
Expand Down Expand Up @@ -259,7 +274,7 @@ impl Configuration {

#[allow(deprecated)]
linter: LinterSettings {
rules: lint.as_rule_table(lint_preview)?,
rules,
exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?,
extension: self.extension.unwrap_or_default(),
preview: lint_preview,
Expand Down Expand Up @@ -341,10 +356,7 @@ impl Configuration {
.flake8_implicit_str_concat
.map(Flake8ImplicitStrConcatOptions::into_settings)
.unwrap_or_default(),
flake8_import_conventions: lint
.flake8_import_conventions
.map(Flake8ImportConventionsOptions::into_settings)
.unwrap_or_default(),
flake8_import_conventions,
flake8_pytest_style: lint
.flake8_pytest_style
.map(Flake8PytestStyleOptions::try_into_settings)
Expand Down Expand Up @@ -374,11 +386,7 @@ impl Configuration {
.flake8_gettext
.map(Flake8GetTextOptions::into_settings)
.unwrap_or_default(),
isort: lint
.isort
.map(IsortOptions::try_into_settings)
.transpose()?
.unwrap_or_default(),
isort,
mccabe: lint
.mccabe
.map(McCabeOptions::into_settings)
Expand Down Expand Up @@ -1553,6 +1561,34 @@ fn warn_about_deprecated_top_level_lint_options(
);
}

/// Detect conflicts between I002 (missing-required-import) and ICN001 (unconventional-import-alias)
fn conflicting_import_settings(
isort: &isort::settings::Settings,
flake8_import_conventions: &flake8_import_conventions::settings::Settings,
) -> Result<()> {
use std::fmt::Write;
let mut err_body = String::new();
for required_import in &isort.required_imports {
let name = required_import.qualified_name().to_string();
if let Some(alias) = flake8_import_conventions.aliases.get(&name) {
writeln!(err_body, " - `{name}` -> `{alias}`").unwrap();
}
}

if !err_body.is_empty() {
return Err(anyhow!(
"Required import specified in `lint.isort.required-imports` (I002) \
conflicts with the required import alias specified in either \
`lint.flake8-import-conventions.aliases` or \
`lint.flake8-import-conventions.extend-aliases` (ICN001):\
\n{err_body}\n\
Help: Remove the required import or alias from your configuration."
));
}

Ok(())
}

#[cfg(test)]
mod tests {
use std::str::FromStr;
Expand Down

0 comments on commit e2da33a

Please sign in to comment.