Skip to content

Commit

Permalink
Use own SmallVec for QualifiedName to work-around lifetime issue
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Mar 4, 2024
1 parent b9f194e commit a852379
Show file tree
Hide file tree
Showing 22 changed files with 426 additions and 231 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::rules::{
use crate::settings::types::PythonVersion;

/// Run lint rules over a [`Stmt`] syntax node.
pub(crate) fn statement<'a>(stmt: &'a Stmt, checker: &mut Checker<'a>) {
pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
match stmt {
Stmt::Global(ast::StmtGlobal { names, range: _ }) => {
if checker.enabled(Rule::GlobalAtModuleLevel) {
Expand Down
16 changes: 11 additions & 5 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.semantic.add_module(module);

if alias.asname.is_none() && alias.name.contains('.') {
let qualified_name = QualifiedName::imported(&alias.name);
let qualified_name = QualifiedName::user_defined(&alias.name);
self.add_binding(
module,
alias.identifier(),
BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }),
BindingKind::SubmoduleImport(SubmoduleImport {
qualified_name: Box::new(qualified_name),
}),
BindingFlags::EXTERNAL,
);
} else {
Expand All @@ -440,11 +442,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
}

let name = alias.asname.as_ref().unwrap_or(&alias.name);
let qualified_name = QualifiedName::imported(&alias.name);
let qualified_name = QualifiedName::user_defined(&alias.name);
self.add_binding(
name,
alias.identifier(),
BindingKind::Import(Import { qualified_name }),
BindingKind::Import(Import {
qualified_name: Box::new(qualified_name),
}),
flags,
);
}
Expand Down Expand Up @@ -508,7 +512,9 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.add_binding(
name,
alias.identifier(),
BindingKind::FromImport(FromImport { qualified_name }),
BindingKind::FromImport(FromImport {
qualified_name: Box::new(qualified_name),
}),
flags,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ruff_python_ast::{Expr, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder};
use ruff_python_ast::name::QualifiedName;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -69,10 +69,7 @@ pub(crate) fn debugger_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
/// Checks for the presence of a debugger import.
pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> Option<Diagnostic> {
if let Some(module) = module {
let mut builder =
QualifiedNameBuilder::from_qualified_name(QualifiedName::imported(module));
builder.push(name);
let qualified_name = builder.build();
let qualified_name = QualifiedName::user_defined(module).append_member(name);

if is_debugger_call(&qualified_name) {
return Some(Diagnostic::new(
Expand All @@ -83,7 +80,7 @@ pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) ->
));
}
} else {
let qualified_name = QualifiedName::imported(name);
let qualified_name = QualifiedName::user_defined(name);

if is_debugger_import(&qualified_name) {
return Some(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(crate) fn unconventional_import_alias(
return None;
};

let qualified_name = import.qualified_name();
let qualified_name = import.qualified_name().to_string();

let Some(expected_alias) = conventions.get(qualified_name.as_str()) else {
return None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ pub(crate) fn unaliased_collections_abc_set_import(
let BindingKind::FromImport(import) = &binding.kind else {
return None;
};
if !matches!(import.call_path(), ["collections", "abc", "Set"]) {
if !matches!(
import.qualified_name().segments(),
["collections", "abc", "Set"]
) {
return None;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
qualified_name: import.qualified_name(),
qualified_name: import.qualified_name().to_string(),
strategy: Strategy::MoveImport,
},
range,
Expand Down Expand Up @@ -218,7 +218,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
qualified_name: import.qualified_name(),
qualified_name: import.qualified_name().to_string(),
strategy: Strategy::QuoteUsages,
},
range,
Expand All @@ -245,7 +245,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
qualified_name: import.qualified_name(),
qualified_name: import.qualified_name().to_string(),
strategy: Strategy::MoveImport,
},
range,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ pub(crate) fn typing_only_runtime_import(
let qualified_name = import.qualified_name();

if is_exempt(
qualified_name.as_str(),
&qualified_name.to_string(),
&checker
.settings
.flake8_type_checking
Expand All @@ -296,7 +296,7 @@ pub(crate) fn typing_only_runtime_import(

// Categorize the import, using coarse-grained categorization.
let import_type = match categorize(
qualified_name.as_str(),
&qualified_name.to_string(),
None,
&checker.settings.src,
checker.package(),
Expand Down Expand Up @@ -365,8 +365,10 @@ pub(crate) fn typing_only_runtime_import(
..
} in imports
{
let mut diagnostic =
Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
let mut diagnostic = Diagnostic::new(
diagnostic_for(import_type, import.qualified_name().to_string()),
range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
Expand All @@ -387,8 +389,10 @@ pub(crate) fn typing_only_runtime_import(
..
} in imports
{
let mut diagnostic =
Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
let mut diagnostic = Diagnostic::new(
diagnostic_for(import_type, import.qualified_name().to_string()),
range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/rules/pandas_vet/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti
| BindingKind::ComprehensionVar
| BindingKind::Global
| BindingKind::Nonlocal(_) => Resolution::RelevantLocal,
BindingKind::Import(import) if matches!(import.call_path(), ["pandas"]) => {
BindingKind::Import(import)
if matches!(import.qualified_name().segments(), ["pandas"]) =>
{
Resolution::PandasModule
}
_ => Resolution::IrrelevantBinding,
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: import.qualified_name(),
name: import.qualified_name().to_string(),
context: if in_except_handler {
Some(UnusedImportContext::ExceptHandler)
} else if in_init {
Expand Down Expand Up @@ -212,7 +212,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: import.qualified_name(),
name: import.qualified_name().to_string(),
context: None,
multiple: false,
},
Expand Down
17 changes: 10 additions & 7 deletions crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use itertools::Itertools;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::name::QualifiedName;
use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope};
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -113,7 +114,8 @@ pub(crate) fn import_private_name(
// Ignore public imports; require at least one private name.
// Ex) `from foo import bar`
let Some((index, private_name)) = import_info
.call_path
.qualified_name
.segments()
.iter()
.find_position(|name| name.starts_with('_'))
else {
Expand All @@ -132,7 +134,7 @@ pub(crate) fn import_private_name(

let name = (*private_name).to_string();
let module = if index > 0 {
Some(import_info.call_path[..index].join("."))
Some(import_info.qualified_name.segments()[..index].join("."))
} else {
None
};
Expand All @@ -152,21 +154,22 @@ fn is_typing(reference: &ResolvedReference) -> bool {
|| reference.in_runtime_evaluated_annotation()
}

#[allow(clippy::struct_field_names)]
struct ImportInfo<'a> {
module_name: &'a [&'a str],
member_name: Cow<'a, str>,
call_path: &'a [&'a str],
qualified_name: &'a QualifiedName<'a>,
}

impl<'a> From<&'a FromImport<'_>> for ImportInfo<'a> {
fn from(import: &'a FromImport) -> Self {
let module_name = import.module_name();
let member_name = import.member_name();
let call_path = import.call_path();
let qualified_name = import.qualified_name();
Self {
module_name,
member_name,
call_path,
qualified_name,
}
}
}
Expand All @@ -175,11 +178,11 @@ impl<'a> From<&'a Import<'_>> for ImportInfo<'a> {
fn from(import: &'a Import) -> Self {
let module_name = import.module_name();
let member_name = import.member_name();
let call_path = import.call_path();
let qualified_name = import.qualified_name();
Self {
module_name,
member_name,
call_path,
qualified_name,
}
}
}
8 changes: 4 additions & 4 deletions crates/ruff_linter/src/rules/refurb/rules/repeated_append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Violation for RepeatedAppend {
}

/// FURB113
pub(crate) fn repeated_append<'a, 'b>(checker: &'b mut Checker<'a>, stmt: &'a Stmt) {
pub(crate) fn repeated_append(checker: &mut Checker, stmt: &Stmt) {
let Some(appends) = match_consecutive_appends(stmt, checker.semantic()) else {
return;
};
Expand Down Expand Up @@ -171,9 +171,9 @@ impl Ranged for AppendGroup<'_> {
}

/// Match consecutive calls to `append` on list variables starting from the given statement.
fn match_consecutive_appends<'a, 'b>(
fn match_consecutive_appends<'a>(
stmt: &'a Stmt,
semantic: &'b SemanticModel<'a>,
semantic: &'a SemanticModel,
) -> Option<Vec<Append<'a>>> {
// Match the current statement, to see if it's an append.
let append = match_append(semantic, stmt)?;
Expand Down Expand Up @@ -267,7 +267,7 @@ fn get_or_add<'a, 'b>(
}

/// Matches that the given statement is a call to `append` on a list variable.
fn match_append<'a>(semantic: &'a SemanticModel<'a>, stmt: &'a Stmt) -> Option<Append<'a>> {
fn match_append<'a>(semantic: &'a SemanticModel, stmt: &'a Stmt) -> Option<Append<'a>> {
let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else {
return None;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ impl Violation for AsyncioDanglingTask {
}

/// RUF006
pub(crate) fn asyncio_dangling_task<'a>(
expr: &'a Expr,
semantic: &SemanticModel<'a>,
) -> Option<Diagnostic> {
pub(crate) fn asyncio_dangling_task(expr: &Expr, semantic: &SemanticModel) -> Option<Diagnostic> {
let Expr::Call(ast::ExprCall { func, .. }) = expr else {
return None;
};
Expand Down
12 changes: 6 additions & 6 deletions crates/ruff_linter/src/rules/ruff/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,25 +298,25 @@ mod tests {

#[test]
fn test_is_known_type() {
assert!(is_known_type(&QualifiedName::from_slice(&["", "int"]), 11));
assert!(is_known_type(&QualifiedName::builtin("int"), 11));
assert!(is_known_type(
&QualifiedName::from_slice(&["builtins", "int"]),
&QualifiedName::from_iter(["builtins", "int"]),
11
));
assert!(is_known_type(
&QualifiedName::from_slice(&["typing", "Optional"]),
&QualifiedName::from_iter(["typing", "Optional"]),
11
));
assert!(is_known_type(
&QualifiedName::from_slice(&["typing_extensions", "Literal"]),
&QualifiedName::from_iter(["typing_extensions", "Literal"]),
11
));
assert!(is_known_type(
&QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]),
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
11
));
assert!(!is_known_type(
&QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]),
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
8
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ impl Violation for ErrorInsteadOfException {
}

/// TRY400
pub(crate) fn error_instead_of_exception<'a>(
checker: &mut Checker<'a>,
handlers: &'a [ExceptHandler],
) {
pub(crate) fn error_instead_of_exception(checker: &mut Checker, handlers: &[ExceptHandler]) {
for handler in handlers {
let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { body, .. }) = handler;
let calls = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl Violation for VerboseLogMessage {
}

/// TRY401
pub(crate) fn verbose_log_message<'a>(checker: &mut Checker<'a>, handlers: &'a [ExceptHandler]) {
pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandler]) {
for handler in handlers {
let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { body, .. }) = handler;

Expand Down
1 change: 0 additions & 1 deletion crates/ruff_python_ast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ itertools = { workspace = true }
once_cell = { workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true, optional = true }
smallvec = { workspace = true }

[dev-dependencies]
insta = { workspace = true }
Expand Down
Loading

0 comments on commit a852379

Please sign in to comment.