diff --git a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs index 8b5ba3700d..aaac685e29 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::str::raw_contents_range; +use ruff_python_ast::{all::DunderAllName, str::raw_contents_range}; use ruff_text_size::{Ranged, TextRange}; use ruff_python_semantic::{ @@ -93,7 +93,7 @@ pub(crate) fn definitions(checker: &mut Checker) { } // Compute visibility of all definitions. - let exports: Option> = { + let exports: Option> = { checker .semantic .global_scope() diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index cd3a4868d6..d2a288f7aa 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -31,8 +31,9 @@ use std::path::Path; use itertools::Itertools; use log::debug; use ruff_python_ast::{ - self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, Keyword, - MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp, + self as ast, all::DunderAllName, Comprehension, ElifElseClause, ExceptHandler, Expr, + ExprContext, Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, + Suite, UnaryOp, }; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -2098,25 +2099,22 @@ impl<'a> Checker<'a> { fn visit_exports(&mut self) { let snapshot = self.semantic.snapshot(); - let exports: Vec<(&str, TextRange)> = self + let exports: Vec = self .semantic .global_scope() .get_all("__all__") .map(|binding_id| &self.semantic.bindings[binding_id]) .filter_map(|binding| match &binding.kind { - BindingKind::Export(Export { names }) => { - Some(names.iter().map(|name| (*name, binding.range()))) - } + BindingKind::Export(Export { names }) => Some(names.iter().copied()), _ => None, }) .flatten() .collect(); - for (name, range) in exports { + for export in exports { + let (name, range) = (export.name(), export.range()); if let Some(binding_id) = self.semantic.global_scope().get(name) { // Mark anything referenced in `__all__` as used. - // TODO(charlie): `range` here should be the range of the name in `__all__`, not - // the range of `__all__` itself. self.semantic .add_global_reference(binding_id, ExprContext::Load, range); } else { @@ -2124,7 +2122,7 @@ impl<'a> Checker<'a> { if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { self.diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedLocalWithImportStarUsage { - name: (*name).to_string(), + name: name.to_string(), }, range, )); @@ -2134,7 +2132,7 @@ impl<'a> Checker<'a> { if !self.path.ends_with("__init__.py") { self.diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedExport { - name: (*name).to_string(), + name: name.to_string(), }, range, )); diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F405_F405.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F405_F405.py.snap index 3e3c43d3b9..5c73ac384d 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F405_F405.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F405_F405.py.snap @@ -8,12 +8,10 @@ F405.py:5:11: F405 `name` may be undefined, or defined from star imports | ^^^^ F405 | -F405.py:11:1: F405 `a` may be undefined, or defined from star imports +F405.py:11:12: F405 `a` may be undefined, or defined from star imports | 9 | print(name) 10 | 11 | __all__ = ['a'] - | ^^^^^^^ F405 + | ^^^ F405 | - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.py.snap index e4ced191b5..78dfd63657 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.py.snap @@ -1,12 +1,10 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F822_0.py:3:1: F822 Undefined name `b` in `__all__` +F822_0.py:3:17: F822 Undefined name `b` in `__all__` | 1 | a = 1 2 | 3 | __all__ = ["a", "b"] - | ^^^^^^^ F822 + | ^^^ F822 | - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.pyi.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.pyi.snap index 320ac6c37f..d74b6e8936 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.pyi.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_0.pyi.snap @@ -1,10 +1,10 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F822_0.pyi:4:1: F822 Undefined name `c` in `__all__` +F822_0.pyi:4:22: F822 Undefined name `c` in `__all__` | 2 | b: int # Considered a binding in a `.pyi` stub file, not in a `.py` runtime file 3 | 4 | __all__ = ["a", "b", "c"] # c is flagged as missing; b is not - | ^^^^^^^ F822 + | ^^^ F822 | diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_1.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_1.py.snap index 0e28a36803..d65deed9e7 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_1.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_1.py.snap @@ -1,12 +1,10 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F822_1.py:3:1: F822 Undefined name `b` in `__all__` +F822_1.py:3:22: F822 Undefined name `b` in `__all__` | 1 | a = 1 2 | 3 | __all__ = list(["a", "b"]) - | ^^^^^^^ F822 + | ^^^ F822 | - - diff --git a/crates/ruff_python_ast/src/all.rs b/crates/ruff_python_ast/src/all.rs index 2b35ee58c5..71348cdc10 100644 --- a/crates/ruff_python_ast/src/all.rs +++ b/crates/ruff_python_ast/src/all.rs @@ -1,4 +1,5 @@ use bitflags::bitflags; +use ruff_text_size::{Ranged, TextRange}; use crate::helpers::map_subscript; use crate::{self as ast, Expr, Stmt}; @@ -15,17 +16,47 @@ bitflags! { } } +/// Abstraction for a string inside an `__all__` definition, +/// e.g. `"foo"` in `__all__ = ["foo"]` +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DunderAllName<'a> { + /// The value of the string inside the `__all__` definition + name: &'a str, + + /// The range of the string inside the `__all__` definition + range: TextRange, +} + +impl DunderAllName<'_> { + pub fn name(&self) -> &str { + self.name + } +} + +impl Ranged for DunderAllName<'_> { + fn range(&self) -> TextRange { + self.range + } +} + /// Extract the names bound to a given __all__ assignment. /// /// Accepts a closure that determines whether a given name (e.g., `"list"`) is a Python builtin. -pub fn extract_all_names(stmt: &Stmt, is_builtin: F) -> (Vec<&str>, DunderAllFlags) +pub fn extract_all_names(stmt: &Stmt, is_builtin: F) -> (Vec, DunderAllFlags) where F: Fn(&str) -> bool, { - fn add_to_names<'a>(elts: &'a [Expr], names: &mut Vec<&'a str>, flags: &mut DunderAllFlags) { + fn add_to_names<'a>( + elts: &'a [Expr], + names: &mut Vec>, + flags: &mut DunderAllFlags, + ) { for elt in elts { - if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = elt { - names.push(value.to_str()); + if let Expr::StringLiteral(ast::ExprStringLiteral { value, range }) = elt { + names.push(DunderAllName { + name: value.to_str(), + range: *range, + }); } else { *flags |= DunderAllFlags::INVALID_OBJECT; } @@ -96,7 +127,7 @@ where (None, DunderAllFlags::INVALID_FORMAT) } - let mut names: Vec<&str> = vec![]; + let mut names: Vec = vec![]; let mut flags = DunderAllFlags::empty(); if let Some(value) = match stmt { diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index fff0d852bb..9b791653ff 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -4,6 +4,7 @@ use std::ops::{Deref, DerefMut}; use bitflags::bitflags; use ruff_index::{newtype_index, IndexSlice, IndexVec}; +use ruff_python_ast::all::DunderAllName; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::Stmt; use ruff_source_file::Locator; @@ -370,7 +371,7 @@ impl<'a> FromIterator> for Bindings<'a> { #[derive(Debug, Clone)] pub struct Export<'a> { /// The names of the bindings exported via `__all__`. - pub names: Box<[&'a str]>, + pub names: Box<[DunderAllName<'a>]>, } /// A binding for an `import`, keyed on the name to which the import is bound. diff --git a/crates/ruff_python_semantic/src/definition.rs b/crates/ruff_python_semantic/src/definition.rs index 19f7bb8904..6b8f1d5d86 100644 --- a/crates/ruff_python_semantic/src/definition.rs +++ b/crates/ruff_python_semantic/src/definition.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use std::ops::Deref; use ruff_index::{newtype_index, IndexSlice, IndexVec}; -use ruff_python_ast::{self as ast, Stmt}; +use ruff_python_ast::{self as ast, all::DunderAllName, Stmt}; use ruff_text_size::{Ranged, TextRange}; use crate::analyze::visibility::{ @@ -187,7 +187,7 @@ impl<'a> Definitions<'a> { } /// Resolve the visibility of each definition in the collection. - pub fn resolve(self, exports: Option<&[&str]>) -> ContextualizedDefinitions<'a> { + pub fn resolve(self, exports: Option<&[DunderAllName]>) -> ContextualizedDefinitions<'a> { let mut definitions: IndexVec> = IndexVec::with_capacity(self.len()); @@ -201,7 +201,9 @@ impl<'a> Definitions<'a> { MemberKind::Class(class) => { let parent = &definitions[member.parent]; if parent.visibility.is_private() - || exports.is_some_and(|exports| !exports.contains(&member.name())) + || exports.is_some_and(|exports| { + !exports.iter().any(|export| export.name() == member.name()) + }) { Visibility::Private } else { @@ -221,7 +223,9 @@ impl<'a> Definitions<'a> { MemberKind::Function(function) => { let parent = &definitions[member.parent]; if parent.visibility.is_private() - || exports.is_some_and(|exports| !exports.contains(&member.name())) + || exports.is_some_and(|exports| { + !exports.iter().any(|export| export.name() == member.name()) + }) { Visibility::Private } else {