Skip to content

Commit

Permalink
Avoid allocations for isort module names
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 2, 2024
1 parent 1673bc4 commit dd402ec
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 30 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
};

let definitions = std::mem::take(&mut checker.semantic.definitions);
let mut overloaded_name: Option<String> = None;
let mut overloaded_name: Option<&str> = None;
for ContextualizedDefinition {
definition,
visibility,
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) {
checker.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithNestedImportStarUsage {
name: helpers::format_import_from(level, module),
name: helpers::format_import_from(level, module).to_string(),
},
stmt.range(),
));
Expand All @@ -886,7 +886,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::UndefinedLocalWithImportStar) {
checker.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithImportStar {
name: helpers::format_import_from(level, module),
name: helpers::format_import_from(level, module).to_string(),
},
stmt.range(),
));
Expand Down
7 changes: 5 additions & 2 deletions crates/ruff_linter/src/rules/flake8_annotations/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ use crate::importer::{ImportRequest, Importer};
use crate::settings::types::PythonVersion;

/// Return the name of the function, if it's overloaded.
pub(crate) fn overloaded_name(definition: &Definition, semantic: &SemanticModel) -> Option<String> {
pub(crate) fn overloaded_name<'a>(
definition: &'a Definition,
semantic: &SemanticModel,
) -> Option<&'a str> {
let function = definition.as_function_def()?;
if visibility::is_overload(&function.decorator_list, semantic) {
Some(function.name.to_string())
Some(function.name.as_str())
} else {
None
}
Expand Down
41 changes: 25 additions & 16 deletions crates/ruff_linter/src/rules/isort/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,38 @@ pub(crate) struct CommentSet<'a> {
pub(crate) inline: Vec<Cow<'a, str>>,
}

pub(crate) trait Importable {
fn module_name(&self) -> String;
fn module_base(&self) -> String;
}

impl Importable for AliasData<'_> {
fn module_name(&self) -> String {
self.name.to_string()
pub(crate) trait Importable<'a> {
fn module_name(&self) -> Cow<'a, str>;

fn module_base(&self) -> Cow<'a, str> {
match self.module_name() {
Cow::Borrowed(module_name) => Cow::Borrowed(
module_name
.split('.')
.next()
.expect("module to include at least one segment"),
),
Cow::Owned(module_name) => Cow::Owned(
module_name
.split('.')
.next()
.expect("module to include at least one segment")
.to_owned(),
),
}
}
}

fn module_base(&self) -> String {
self.module_name().split('.').next().unwrap().to_string()
impl<'a> Importable<'a> for AliasData<'a> {
fn module_name(&self) -> Cow<'a, str> {
Cow::Borrowed(self.name)
}
}

impl Importable for ImportFromData<'_> {
fn module_name(&self) -> String {
impl<'a> Importable<'a> for ImportFromData<'a> {
fn module_name(&self) -> Cow<'a, str> {
format_import_from(self.level, self.module)
}

fn module_base(&self) -> String {
self.module_name().split('.').next().unwrap().to_string()
}
}

#[derive(Debug, Default)]
Expand Down
23 changes: 14 additions & 9 deletions crates/ruff_python_ast/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,17 +715,22 @@ where
/// assert_eq!(format_import_from(1, None), ".".to_string());
/// assert_eq!(format_import_from(1, Some("foo")), ".foo".to_string());
/// ```
pub fn format_import_from(level: u32, module: Option<&str>) -> String {
let mut module_name = String::with_capacity(16);
if level > 0 {
for _ in 0..level {
module_name.push('.');
pub fn format_import_from(level: u32, module: Option<&str>) -> Cow<str> {
match (level, module) {
(0, Some(module)) => Cow::Borrowed(module),
(level, module) => {
let mut module_name = String::with_capacity(
(level as usize) + module.as_ref().map_or(0, |module| module.len()),
);
for _ in 0..level {
module_name.push('.');
}
if let Some(module) = module {
module_name.push_str(module);
}
Cow::Owned(module_name)
}
}
if let Some(module) = module {
module_name.push_str(module);
}
module_name
}

/// Format the member reference name for a relative import.
Expand Down

0 comments on commit dd402ec

Please sign in to comment.