diff --git a/crates/ruff_diagnostics/src/diagnostic.rs b/crates/ruff_diagnostics/src/diagnostic.rs index 006df346365fd..84da5f3904607 100644 --- a/crates/ruff_diagnostics/src/diagnostic.rs +++ b/crates/ruff_diagnostics/src/diagnostic.rs @@ -71,6 +71,14 @@ impl Diagnostic { } } + /// Consumes `self` and returns a new `Diagnostic` with the given parent node. + #[inline] + #[must_use] + pub fn with_parent(mut self, parent: TextSize) -> Self { + self.set_parent(parent); + self + } + /// Set the location of the diagnostic's parent node. #[inline] pub fn set_parent(&mut self, parent: TextSize) { diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py new file mode 100644 index 0000000000000..682b69597223e --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py @@ -0,0 +1,13 @@ +"""Respect `# noqa` directives on `__all__` definitions.""" + +__all__ = [ # noqa: F822 + "Bernoulli", + "Beta", + "Binomial", +] + + +__all__ += [ + "ContinuousBernoulli", # noqa: F822 + "ExponentialFamily", +] diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 6af274cb8f331..990fa0abefe33 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -31,15 +31,14 @@ use std::path::Path; use itertools::Itertools; use log::debug; use ruff_python_ast::{ - self as ast, all::DunderAllName, Comprehension, ElifElseClause, ExceptHandler, Expr, - ExprContext, FStringElement, Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, - Pattern, Stmt, Suite, UnaryOp, + self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, FStringElement, + Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp, }; use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_diagnostics::{Diagnostic, IsolationLevel}; use ruff_notebook::{CellOffsets, NotebookIndex}; -use ruff_python_ast::all::{extract_all_names, DunderAllFlags}; +use ruff_python_ast::all::{extract_all_names, DunderAllDefinition, DunderAllFlags}; use ruff_python_ast::helpers::{ collect_import_from_member, extract_handled_exceptions, is_docstring_stmt, to_module_path, }; @@ -2109,45 +2108,54 @@ impl<'a> Checker<'a> { fn visit_exports(&mut self) { let snapshot = self.semantic.snapshot(); - let exports: Vec = self + let definitions: 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().copied()), + BindingKind::Export(Export { names }) => { + Some(DunderAllDefinition::new(binding.range(), names.to_vec())) + } _ => None, }) - .flatten() .collect(); - for export in exports { - let (name, range) = (export.name(), export.range()); - if let Some(binding_id) = self.semantic.global_scope().get(name) { - self.semantic.flags |= SemanticModelFlags::DUNDER_ALL_DEFINITION; - // Mark anything referenced in `__all__` as used. - self.semantic - .add_global_reference(binding_id, ExprContext::Load, range); - self.semantic.flags -= SemanticModelFlags::DUNDER_ALL_DEFINITION; - } else { - if self.semantic.global_scope().uses_star_imports() { - if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedLocalWithImportStarUsage { - name: name.to_string(), - }, - range, - )); - } + for definition in definitions { + for export in definition.names() { + let (name, range) = (export.name(), export.range()); + if let Some(binding_id) = self.semantic.global_scope().get(name) { + self.semantic.flags |= SemanticModelFlags::DUNDER_ALL_DEFINITION; + // Mark anything referenced in `__all__` as used. + self.semantic + .add_global_reference(binding_id, ExprContext::Load, range); + self.semantic.flags -= SemanticModelFlags::DUNDER_ALL_DEFINITION; } else { - if self.enabled(Rule::UndefinedExport) { - if !self.path.ends_with("__init__.py") { - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedExport { - name: name.to_string(), - }, - range, - )); + if self.semantic.global_scope().uses_star_imports() { + if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { + self.diagnostics.push( + Diagnostic::new( + pyflakes::rules::UndefinedLocalWithImportStarUsage { + name: name.to_string(), + }, + range, + ) + .with_parent(definition.start()), + ); + } + } else { + if self.enabled(Rule::UndefinedExport) { + if !self.path.ends_with("__init__.py") { + self.diagnostics.push( + Diagnostic::new( + pyflakes::rules::UndefinedExport { + name: name.to_string(), + }, + range, + ) + .with_parent(definition.start()), + ); + } } } } diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 503690624cac2..6f5673b1ecc39 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -162,6 +162,7 @@ mod tests { #[test_case(Rule::UndefinedExport, Path::new("F822_0.pyi"))] #[test_case(Rule::UndefinedExport, Path::new("F822_1.py"))] #[test_case(Rule::UndefinedExport, Path::new("F822_2.py"))] + #[test_case(Rule::UndefinedExport, Path::new("F822_3.py"))] #[test_case(Rule::UndefinedLocal, Path::new("F823.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_0.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_1.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap new file mode 100644 index 0000000000000..5d572ea096297 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F822_3.py:12:5: F822 Undefined name `ExponentialFamily` in `__all__` + | +10 | __all__ += [ +11 | "ContinuousBernoulli", # noqa: F822 +12 | "ExponentialFamily", + | ^^^^^^^^^^^^^^^^^^^ F822 +13 | ] + | diff --git a/crates/ruff_python_ast/src/all.rs b/crates/ruff_python_ast/src/all.rs index 71348cdc105eb..4a5899ce172e6 100644 --- a/crates/ruff_python_ast/src/all.rs +++ b/crates/ruff_python_ast/src/all.rs @@ -39,6 +39,34 @@ impl Ranged for DunderAllName<'_> { } } +/// Abstraction for a collection of names inside an `__all__` definition, +/// e.g. `["foo", "bar"]` in `__all__ = ["foo", "bar"]` +#[derive(Debug, Clone)] +pub struct DunderAllDefinition<'a> { + /// The range of the `__all__` identifier. + range: TextRange, + /// The names inside the `__all__` definition. + names: Vec>, +} + +impl<'a> DunderAllDefinition<'a> { + /// Initialize a new [`DunderAllDefinition`] instance. + pub fn new(range: TextRange, names: Vec>) -> Self { + Self { range, names } + } + + /// The names inside the `__all__` definition. + pub fn names(&self) -> &[DunderAllName<'a>] { + &self.names + } +} + +impl Ranged for DunderAllDefinition<'_> { + 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.