Respect `# noqa` directives on `__all__` openers (#10798)

## Summary

Historically, given:

```python
__all__ = [  # noqa: F822
    "Bernoulli",
    "Beta",
    "Binomial",
]
```

The F822 violations would be attached to the `__all__`, so this `# noqa`
would be enforced for _all_ definitions in the list. This changed in
https://github.com/astral-sh/ruff/pull/10525 for the better, in that we
now use the range of each string. But these `# noqa` directives stopped
working.

This PR sets the `__all__` as a parent range in the diagnostic, so that
these directives are respected once again.

Closes https://github.com/astral-sh/ruff/issues/10795.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2024-04-06 10:51:17 -04:00 committed by GitHub
parent 83db62bcda
commit 7fb5f47efe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 102 additions and 33 deletions

View File

@ -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) {

View File

@ -0,0 +1,13 @@
"""Respect `# noqa` directives on `__all__` definitions."""
__all__ = [ # noqa: F822
"Bernoulli",
"Beta",
"Binomial",
]
__all__ += [
"ContinuousBernoulli", # noqa: F822
"ExponentialFamily",
]

View File

@ -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<DunderAllName> = self
let definitions: Vec<DunderAllDefinition> = 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()),
);
}
}
}
}

View File

@ -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"))]

View File

@ -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 | ]
|

View File

@ -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<DunderAllName<'a>>,
}
impl<'a> DunderAllDefinition<'a> {
/// Initialize a new [`DunderAllDefinition`] instance.
pub fn new(range: TextRange, names: Vec<DunderAllName<'a>>) -> 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.