mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 13:51:37 +00:00
Track ranges of names inside __all__ definitions (#10525)
This commit is contained in:
Alex Waygood
GitHub
parent
b74dd420fc
commit
a06ffeb54e
9 changed files with 65 additions and 37 deletions
|
|
@ -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<Vec<&str>> = {
|
||||
let exports: Option<Vec<DunderAllName>> = {
|
||||
checker
|
||||
.semantic
|
||||
.global_scope()
|
||||
|
|
|
|||
|
|
@ -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<DunderAllName> = 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,
|
||||
));
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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<F>(stmt: &Stmt, is_builtin: F) -> (Vec<&str>, DunderAllFlags)
|
||||
pub fn extract_all_names<F>(stmt: &Stmt, is_builtin: F) -> (Vec<DunderAllName>, 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<DunderAllName<'a>>,
|
||||
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<DunderAllName> = vec![];
|
||||
let mut flags = DunderAllFlags::empty();
|
||||
|
||||
if let Some(value) = match stmt {
|
||||
|
|
|
|||
|
|
@ -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<Binding<'a>> 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.
|
||||
|
|
|
|||
|
|
@ -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<DefinitionId, ContextualizedDefinition<'a>> =
|
||||
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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue