[unconventional-import-alias] Fix infinite loop between ICN001 and 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 <Alex.Waygood@Gmail.com>
This commit is contained in:
Brent Westbrook 2025-01-16 10:45:24 -05:00 committed by GitHub
parent ca3b210f2e
commit e2da33a45c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 93 additions and 12 deletions

View file

@ -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::{
@ -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
@ -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,
@ -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)
@ -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)
@ -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;