mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-18 17:40:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

refactor: Use QualifiedName for Imported::call_path (#10214)

Browse source 
## Summary

When you try to remove an internal representation leaking into another
type and end up rewriting a simple version of `smallvec`.

The goal of this PR is to replace the `Box<[&'a str]>` with
`Box<QualifiedName>` to avoid that the internal `QualifiedName`
representation leaks (and it gives us a nicer API too). However, doing
this when `QualifiedName` uses `SmallVec` internally gives us all sort
of funny lifetime errors. I was lost but @BurntSushi came to rescue me.
He figured out that `smallvec` has a variance problem which is already
tracked in https://github.com/servo/rust-smallvec/issues/146

To fix the variants problem, I could use the smallvec-2-alpha-4 or
implement our own smallvec. I went with implementing our own small vec
for this specific problem. It obviously isn't as sophisticated as
smallvec (only uses safe code), e.g. it doesn't perform any size
optimizations, but it does its job.

Other changes:

* Removed `Imported::qualified_name` (the version that returns a
`String`). This can be replaced by calling `ToString` on the qualified
name.
* Renamed `Imported::call_path` to `qualified_name` and changed its
return type to `&QualifiedName`.
* Renamed `QualifiedName::imported` to `user_defined` which is the more
common term when talking about builtins vs the rest/user defined
functions.


## Test plan

`cargo test`
This commit is contained in:
Micha Reiser 2024-03-06 09:55:59 +01:00 • committed by GitHub
parent 4c05c258de
commit 8ea5b08700
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 793 additions and 338 deletions
Download patch file Download diff file Expand all files Collapse all files

2
crates/ruff_python_semantic/src/analyze/typing.rs
View file

@ -63,7 +63,7 @@ pub fn match_annotated_subscript<'a>(
}
for module in typing_modules {
let module_qualified_name = QualifiedName::imported(module);
let module_qualified_name = QualifiedName::user_defined(module);
if qualified_name.starts_with(&module_qualified_name) {
if let Some(member) = qualified_name.segments().last() {
if is_literal_member(member) {

47
crates/ruff_python_semantic/src/binding.rs
View file

@ -4,7 +4,7 @@ use std::ops::{Deref, DerefMut};
use bitflags::bitflags;
use ruff_index::{newtype_index, IndexSlice, IndexVec};
use ruff_python_ast::name::format_qualified_name_segments;
use ruff_python_ast::name::QualifiedName;
use ruff_python_ast::Stmt;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};
@ -362,7 +362,7 @@ pub struct Import<'a> {
/// The full name of the module being imported.
/// Ex) Given `import foo`, `qualified_name` would be "foo".
/// Ex) Given `import foo as bar`, `qualified_name` would be "foo".
pub qualified_name: Box<[&'a str]>,
pub qualified_name: Box<QualifiedName<'a>>,
}
/// A binding for a member imported from a module, keyed on the name to which the member is bound.
@ -373,7 +373,7 @@ pub struct FromImport<'a> {
/// The full name of the member being imported.
/// Ex) Given `from foo import bar`, `qualified_name` would be "foo.bar".
/// Ex) Given `from foo import bar as baz`, `qualified_name` would be "foo.bar".
pub qualified_name: Box<[&'a str]>,
pub qualified_name: Box<QualifiedName<'a>>,
}
/// A binding for a submodule imported from a module, keyed on the name of the parent module.
@ -382,7 +382,7 @@ pub struct FromImport<'a> {
pub struct SubmoduleImport<'a> {
/// The full name of the submodule being imported.
/// Ex) Given `import foo.bar`, `qualified_name` would be "foo.bar".
pub qualified_name: Box<[&'a str]>,
pub qualified_name: Box<QualifiedName<'a>>,
}
#[derive(Debug, Clone, is_macro::Is)]
@ -549,7 +549,7 @@ bitflags! {
/// A trait for imported symbols.
pub trait Imported<'a> {
/// Returns the call path to the imported symbol.
fn call_path(&self) -> &[&'a str];
fn qualified_name(&self) -> &QualifiedName<'a>;
/// Returns the module name of the imported symbol.
fn module_name(&self) -> &[&'a str];
@ -557,63 +557,56 @@ pub trait Imported<'a> {
/// Returns the member name of the imported symbol. For a straight import, this is equivalent
/// to the qualified name; for a `from` import, this is the name of the imported symbol.
fn member_name(&self) -> Cow<'a, str>;
/// Returns the fully-qualified name of the imported symbol.
fn qualified_name(&self) -> String {
let mut output = String::new();
format_qualified_name_segments(self.call_path(), &mut output).unwrap();
output
}
}
impl<'a> Imported<'a> for Import<'a> {
/// For example, given `import foo`, returns `["foo"]`.
fn call_path(&self) -> &[&'a str] {
self.qualified_name.as_ref()
fn qualified_name(&self) -> &QualifiedName<'a> {
&self.qualified_name
}
/// For example, given `import foo`, returns `["foo"]`.
fn module_name(&self) -> &[&'a str] {
&self.qualified_name[..1]
&self.qualified_name.segments()[..1]
}
/// For example, given `import foo`, returns `"foo"`.
fn member_name(&self) -> Cow<'a, str> {
Cow::Owned(self.qualified_name())
Cow::Owned(self.qualified_name().to_string())
}
}
impl<'a> Imported<'a> for SubmoduleImport<'a> {
/// For example, given `import foo.bar`, returns `["foo", "bar"]`.
fn call_path(&self) -> &[&'a str] {
self.qualified_name.as_ref()
fn qualified_name(&self) -> &QualifiedName<'a> {
&self.qualified_name
}
/// For example, given `import foo.bar`, returns `["foo"]`.
fn module_name(&self) -> &[&'a str] {
&self.qualified_name[..1]
&self.qualified_name.segments()[..1]
}
/// For example, given `import foo.bar`, returns `"foo.bar"`.
fn member_name(&self) -> Cow<'a, str> {
Cow::Owned(self.qualified_name())
Cow::Owned(self.qualified_name().to_string())
}
}
impl<'a> Imported<'a> for FromImport<'a> {
/// For example, given `from foo import bar`, returns `["foo", "bar"]`.
fn call_path(&self) -> &[&'a str] {
fn qualified_name(&self) -> &QualifiedName<'a> {
&self.qualified_name
}
/// For example, given `from foo import bar`, returns `["foo"]`.
fn module_name(&self) -> &[&'a str] {
&self.qualified_name[..self.qualified_name.len() - 1]
&self.qualified_name.segments()[..self.qualified_name.segments().len() - 1]
}
/// For example, given `from foo import bar`, returns `"bar"`.
fn member_name(&self) -> Cow<'a, str> {
Cow::Borrowed(self.qualified_name[self.qualified_name.len() - 1])
Cow::Borrowed(self.qualified_name.segments()[self.qualified_name.segments().len() - 1])
}
}
@ -626,11 +619,11 @@ pub enum AnyImport<'a, 'ast> {
}
impl<'a, 'ast> Imported<'ast> for AnyImport<'a, 'ast> {
fn call_path(&self) -> &[&'ast str] {
fn qualified_name(&self) -> &QualifiedName<'ast> {
match self {
Self::Import(import) => import.call_path(),
Self::SubmoduleImport(import) => import.call_path(),
Self::FromImport(import) => import.call_path(),
Self::Import(import) => import.qualified_name(),
Self::SubmoduleImport(import) => import.qualified_name(),
Self::FromImport(import) => import.qualified_name(),
}
}

38
crates/ruff_python_semantic/src/model.rs
View file

@ -4,7 +4,7 @@ use bitflags::bitflags;
use rustc_hash::FxHashMap;
use ruff_python_ast::helpers::from_relative_import;
use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder, UnqualifiedName};
use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
use ruff_python_ast::{self as ast, Expr, Operator, Stmt};
use ruff_python_stdlib::path::is_python_stub_file;
use ruff_text_size::{Ranged, TextRange, TextSize};
@ -194,10 +194,7 @@ impl<'a> SemanticModel<'a> {
if self.typing_modules.iter().any(|module| {
let module = QualifiedName::from_dotted_name(module);
let mut builder = QualifiedNameBuilder::from_qualified_name(module);
builder.push(target);
let target_path = builder.build();
qualified_name == &target_path
qualified_name == &module.append_member(target)
}) {
return true;
}
@ -617,8 +614,8 @@ impl<'a> SemanticModel<'a> {
}
// Grab, e.g., `pyarrow` from `import pyarrow as pa`.
let call_path = import.call_path();
let segment = call_path.last()?;
let call_path = import.qualified_name();
let segment = call_path.segments().last()?;
if *segment == symbol {
return None;
}
@ -692,8 +689,12 @@ impl<'a> SemanticModel<'a> {
BindingKind::Import(Import { qualified_name }) => {
let unqualified_name = UnqualifiedName::from_expr(value)?;
let (_, tail) = unqualified_name.segments().split_first()?;
let resolved: QualifiedName =
qualified_name.iter().chain(tail.iter()).copied().collect();
let resolved: QualifiedName = qualified_name
.segments()
.iter()
.chain(tail.iter())
.copied()
.collect();
Some(resolved)
}
BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => {
@ -702,6 +703,7 @@ impl<'a> SemanticModel<'a> {
Some(
qualified_name
.segments()
.iter()
.take(1)
.chain(tail.iter())
@ -714,19 +716,25 @@ impl<'a> SemanticModel<'a> {
let (_, tail) = value_name.segments().split_first()?;
let resolved: QualifiedName = if qualified_name
.segments()
.first()
.map_or(false, |segment| *segment == ".")
{
from_relative_import(self.module_path?, qualified_name, tail)?
from_relative_import(self.module_path?, qualified_name.segments(), tail)?
} else {
qualified_name.iter().chain(tail.iter()).copied().collect()
qualified_name
.segments()
.iter()
.chain(tail.iter())
.copied()
.collect()
};
Some(resolved)
}
BindingKind::Builtin => {
if value.is_name_expr() {
// Ex) `dict`
Some(QualifiedName::from_slice(&["", head.id.as_str()]))
Some(QualifiedName::builtin(head.id.as_str()))
} else {
// Ex) `dict.__dict__`
let value_name = UnqualifiedName::from_expr(value)?;
@ -780,7 +788,7 @@ impl<'a> SemanticModel<'a> {
// `import sys` -> `sys.exit`
// `import sys as sys2` -> `sys2.exit`
BindingKind::Import(Import { qualified_name }) => {
if qualified_name.as_ref() == module_path.as_slice() {
if qualified_name.segments() == module_path.as_slice() {
if let Some(source) = binding.source {
// Verify that `sys` isn't bound in an inner scope.
if self
@ -803,7 +811,7 @@ impl<'a> SemanticModel<'a> {
// `from os.path import join as join2` -> `join2`
BindingKind::FromImport(FromImport { qualified_name }) => {
if let Some((target_member, target_module)) =
qualified_name.split_last()
qualified_name.segments().split_last()
{
if target_module == module_path.as_slice()
&& target_member == &member
@ -831,7 +839,7 @@ impl<'a> SemanticModel<'a> {
// Ex) Given `module="os.path"` and `object="join"`:
// `import os.path ` -> `os.path.join`
BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => {
if qualified_name.starts_with(&module_path) {
if qualified_name.segments().starts_with(&module_path) {
if let Some(source) = binding.source {
// Verify that `os` isn't bound in an inner scope.
if self