Skip to content

Commit

Permalink
Move
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 26, 2024
1 parent 1fe4a5f commit 6276b05
Show file tree
Hide file tree
Showing 18 changed files with 330 additions and 276 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 13 additions & 8 deletions crates/ruff_linter/src/importer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ use anyhow::Result;
use libcst_native::{ImportAlias, Name as cstName, NameOrAttribute};

use ruff_diagnostics::Edit;
use ruff_python_ast::imports::{AnyImport, Import, ImportFrom};
use ruff_python_ast::{self as ast, ModModule, Stmt};
use ruff_python_codegen::Stylist;
use ruff_python_parser::{Parsed, Tokens};
use ruff_python_semantic::{ImportedName, SemanticModel};
use ruff_python_semantic::{
ImportedName, MemberNameImport, ModuleNameImport, NameImport, SemanticModel,
};
use ruff_python_trivia::textwrap::indent;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextSize};
Expand Down Expand Up @@ -71,7 +72,7 @@ impl<'a> Importer<'a> {
/// If there are no existing imports, the new import will be added at the top
/// of the file. Otherwise, it will be added after the most recent top-level
/// import statement.
pub(crate) fn add_import(&self, import: &AnyImport, at: TextSize) -> Edit {
pub(crate) fn add_import(&self, import: &NameImport, at: TextSize) -> Edit {
let required_import = import.to_string();
if let Some(stmt) = self.preceding_import(at) {
// Insert after the last top-level import.
Expand Down Expand Up @@ -359,8 +360,12 @@ impl<'a> Importer<'a> {
// Case 2a: No `functools` import is in scope; thus, we add `import functools`,
// and return `"functools.cache"` as the bound name.
if semantic.is_available(symbol.module) {
let import_edit =
self.add_import(&AnyImport::Import(Import::module(symbol.module)), at);
let import_edit = self.add_import(
&NameImport::Import(ModuleNameImport::module(
symbol.module.to_string(),
)),
at,
);
Ok((
import_edit,
format!(
Expand All @@ -378,9 +383,9 @@ impl<'a> Importer<'a> {
// `from functools import cache`, and return `"cache"` as the bound name.
if semantic.is_available(symbol.member) {
let import_edit = self.add_import(
&AnyImport::ImportFrom(ImportFrom::member(
symbol.module,
symbol.member,
&NameImport::ImportFrom(MemberNameImport::member(
symbol.module.to_string(),
symbol.member.to_string(),
)),
at,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::fmt;

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::imports::{AnyImport, ImportFrom};
use ruff_python_ast::Expr;
use ruff_python_semantic::{MemberNameImport, NameImport};
use ruff_text_size::{Ranged, TextSize};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -86,7 +86,10 @@ impl AlwaysFixableViolation for FutureRequiredTypeAnnotation {
/// FA102
pub(crate) fn future_required_type_annotation(checker: &mut Checker, expr: &Expr, reason: Reason) {
let mut diagnostic = Diagnostic::new(FutureRequiredTypeAnnotation { reason }, expr.range());
let required_import = AnyImport::ImportFrom(ImportFrom::member("__future__", "annotations"));
let required_import = NameImport::ImportFrom(MemberNameImport::member(
"__future__".to_string(),
"annotations".to_string(),
));
diagnostic.set_fix(Fix::unsafe_edit(
checker
.importer()
Expand Down
60 changes: 26 additions & 34 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,11 @@ mod tests {
use std::path::Path;

use anyhow::Result;
use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport};
use ruff_text_size::Ranged;
use rustc_hash::{FxHashMap, FxHashSet};
use test_case::test_case;

use ruff_text_size::Ranged;

use crate::assert_messages;
use crate::registry::Rule;
use crate::rules::isort::categorize::{ImportSection, KnownModules};
Expand Down Expand Up @@ -804,9 +804,12 @@ mod tests {
&LinterSettings {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter([
"from __future__ import annotations".to_string()
]),
required_imports: BTreeSet::from_iter([NameImport::ImportFrom(
MemberNameImport::member(
"__future__".to_string(),
"annotations".to_string(),
),
)]),
..super::settings::Settings::default()
},
..LinterSettings::for_rule(Rule::MissingRequiredImport)
Expand Down Expand Up @@ -834,9 +837,13 @@ mod tests {
&LinterSettings {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter([
"from __future__ import annotations as _annotations".to_string(),
]),
required_imports: BTreeSet::from_iter([NameImport::ImportFrom(
MemberNameImport::alias(
"__future__".to_string(),
"annotations".to_string(),
"_annotations".to_string(),
),
)]),
..super::settings::Settings::default()
},
..LinterSettings::for_rule(Rule::MissingRequiredImport)
Expand All @@ -858,8 +865,14 @@ mod tests {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter([
"from __future__ import annotations".to_string(),
"from __future__ import generator_stop".to_string(),
NameImport::ImportFrom(MemberNameImport::member(
"__future__".to_string(),
"annotations".to_string(),
)),
NameImport::ImportFrom(MemberNameImport::member(
"__future__".to_string(),
"generator_stop".to_string(),
)),
]),
..super::settings::Settings::default()
},
Expand All @@ -870,29 +883,6 @@ mod tests {
Ok(())
}

#[test_case(Path::new("docstring.py"))]
#[test_case(Path::new("docstring.pyi"))]
#[test_case(Path::new("docstring_only.py"))]
#[test_case(Path::new("empty.py"))]
fn combined_required_imports(path: &Path) -> Result<()> {
let snapshot = format!("combined_required_imports_{}", path.to_string_lossy());
let diagnostics = test_path(
Path::new("isort/required_imports").join(path).as_path(),
&LinterSettings {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter(["from __future__ import annotations, \
generator_stop"
.to_string()]),
..super::settings::Settings::default()
},
..LinterSettings::for_rule(Rule::MissingRequiredImport)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Path::new("docstring.py"))]
#[test_case(Path::new("docstring.pyi"))]
#[test_case(Path::new("docstring_only.py"))]
Expand All @@ -904,7 +894,9 @@ mod tests {
&LinterSettings {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter(["import os".to_string()]),
required_imports: BTreeSet::from_iter([NameImport::Import(
ModuleNameImport::module("os".to_string()),
)]),
..super::settings::Settings::default()
},
..LinterSettings::for_rule(Rule::MissingRequiredImport)
Expand Down
90 changes: 14 additions & 76 deletions crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use log::error;

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_docstring_stmt;
use ruff_python_ast::imports::{Alias, AnyImport, FutureImport, Import, ImportFrom};
use ruff_python_ast::{self as ast, ModModule, PySourceType, Stmt};
use ruff_python_codegen::Stylist;
use ruff_python_parser::{parse_module, Parsed};
use ruff_python_parser::Parsed;
use ruff_python_semantic::{FutureImport, NameImport};
use ruff_source_file::Locator;
use ruff_text_size::{TextRange, TextSize};

Expand Down Expand Up @@ -53,18 +51,19 @@ impl AlwaysFixableViolation for MissingRequiredImport {
}
}

/// Return `true` if the [`Stmt`] includes the given [`AnyImport`].
fn includes_import(stmt: &Stmt, target: &AnyImport) -> bool {
/// Return `true` if the [`Stmt`] includes the given [`AnyImportRef`].
fn includes_import(stmt: &Stmt, target: &NameImport) -> bool {
match target {
AnyImport::Import(target) => {
NameImport::Import(target) => {
let Stmt::Import(ast::StmtImport { names, range: _ }) = &stmt else {
return false;
};
names.iter().any(|alias| {
&alias.name == target.name.name && alias.asname.as_deref() == target.name.as_name
alias.name == target.name.name
&& alias.asname.as_deref() == target.name.as_name.as_deref()
})
}
AnyImport::ImportFrom(target) => {
NameImport::ImportFrom(target) => {
let Stmt::ImportFrom(ast::StmtImportFrom {
module,
names,
Expand All @@ -74,19 +73,19 @@ fn includes_import(stmt: &Stmt, target: &AnyImport) -> bool {
else {
return false;
};
module.as_deref() == target.module
module.as_deref() == target.module.as_deref()
&& *level == target.level
&& names.iter().any(|alias| {
&alias.name == target.name.name
&& alias.asname.as_deref() == target.name.as_name
alias.name == target.name.name
&& alias.asname.as_deref() == target.name.as_name.as_deref()
})
}
}
}

#[allow(clippy::too_many_arguments)]
fn add_required_import(
required_import: &AnyImport,
required_import: &NameImport,
parsed: &Parsed<ModModule>,
locator: &Locator,
stylist: &Stylist,
Expand Down Expand Up @@ -134,69 +133,8 @@ pub(crate) fn add_required_imports(
.isort
.required_imports
.iter()
.flat_map(|required_import| {
let Ok(body) = parse_module(required_import).map(Parsed::into_suite) else {
error!("Failed to parse required import: `{}`", required_import);
return vec![];
};
if body.is_empty() || body.len() > 1 {
error!(
"Expected require import to contain a single statement: `{}`",
required_import
);
return vec![];
}
let stmt = &body[0];
match stmt {
Stmt::ImportFrom(ast::StmtImportFrom {
module,
names,
level,
range: _,
}) => names
.iter()
.filter_map(|name| {
add_required_import(
&AnyImport::ImportFrom(ImportFrom {
module: module.as_deref(),
name: Alias {
name: name.name.as_str(),
as_name: name.asname.as_deref(),
},
level: *level,
}),
parsed,
locator,
stylist,
source_type,
)
})
.collect(),
Stmt::Import(ast::StmtImport { names, range: _ }) => names
.iter()
.filter_map(|name| {
add_required_import(
&AnyImport::Import(Import {
name: Alias {
name: name.name.as_str(),
as_name: name.asname.as_deref(),
},
}),
parsed,
locator,
stylist,
source_type,
)
})
.collect(),
_ => {
error!(
"Expected required import to be in import-from style: `{}`",
required_import
);
vec![]
}
}
.filter_map(|required_import| {
add_required_import(required_import, parsed, locator, stylist, source_type)
})
.collect()
}
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/isort/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ use rustc_hash::FxHashSet;
use serde::{Deserialize, Serialize};
use strum::IntoEnumIterator;

use ruff_macros::CacheKey;

use crate::display_settings;
use crate::rules::isort::categorize::KnownModules;
use crate::rules::isort::ImportType;
use ruff_macros::CacheKey;
use ruff_python_semantic::NameImport;

use super::categorize::ImportSection;

Expand Down Expand Up @@ -47,7 +47,7 @@ impl Display for RelativeImportsOrder {
#[derive(Debug, Clone, CacheKey)]
#[allow(clippy::struct_excessive_bools)]
pub struct Settings {
pub required_imports: BTreeSet<String>,
pub required_imports: BTreeSet<NameImport>,
pub combine_as_imports: bool,
pub force_single_line: bool,
pub force_sort_within_sections: bool,
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

0 comments on commit 6276b05

Please sign in to comment.