Skip to content

Commit

Permalink
Move reference-resolution into Context
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed May 18, 2023
1 parent 8702b5a commit 728c881
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 95 deletions.
172 changes: 77 additions & 95 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use ruff_python_semantic::binding::{
Binding, BindingId, BindingKind, Exceptions, ExecutionContext, Export, FromImportation,
Importation, StarImportation, SubmoduleImportation,
};
use ruff_python_semantic::context::{Context, ContextFlags};
use ruff_python_semantic::context::{Context, ContextFlags, ResolvedReference};
use ruff_python_semantic::definition::{ContextualizedDefinition, Module, ModuleKind};
use ruff_python_semantic::node::NodeId;
use ruff_python_semantic::scope::{ClassDef, FunctionDef, Lambda, Scope, ScopeId, ScopeKind};
Expand Down Expand Up @@ -4732,40 +4732,20 @@ impl<'a> Checker<'a> {
let Expr::Name(ast::ExprName { id, .. } )= expr else {
return;
};
let id = id.as_str();

let mut first_iter = true;
let mut import_starred = false;

for scope in self.ctx.scopes.ancestors(self.ctx.scope_id) {
if scope.kind.is_class() {
if id == "__class__" {
return;
} else if !first_iter {
continue;
}
}

if let Some(index) = scope.get(id) {
// Mark the binding as used.
let context = self.ctx.execution_context();
self.ctx.bindings[*index].mark_used(self.ctx.scope_id, expr.range(), context);

if !self.ctx.in_deferred_type_definition()
&& self.ctx.bindings[*index].kind.is_annotation()
{
continue;
}

match self.ctx.resolve_reference(id, expr.range()) {
ResolvedReference::Resolved(scope_id, binding_id) => {
// If the name of the sub-importation is the same as an alias of another
// importation and the alias is used, that sub-importation should be
// marked as used too.
//
// This handles code like:
// import pyarrow as pa
// import pyarrow.csv
// print(pa.csv.read_csv("test.csv"))
match &self.ctx.bindings[*index].kind {
// For example, mark `pa` as used in:
//
// ```python
// import pyarrow as pa
// import pyarrow.csv
// print(pa.csv.read_csv("test.csv"))
// ```
match &self.ctx.bindings[binding_id].kind {
BindingKind::Importation(Importation { name, full_name })
| BindingKind::SubmoduleImportation(SubmoduleImportation { name, full_name }) =>
{
Expand All @@ -4776,8 +4756,9 @@ impl<'a> Checker<'a> {
.unwrap_or_default();
if has_alias {
// Mark the sub-importation as used.
if let Some(index) = scope.get(full_name) {
self.ctx.bindings[*index].mark_used(
if let Some(binding_id) = self.ctx.scopes[scope_id].get(full_name) {
let context = self.ctx.execution_context();
self.ctx.bindings[*binding_id].mark_used(
self.ctx.scope_id,
expr.range(),
context,
Expand All @@ -4793,8 +4774,11 @@ impl<'a> Checker<'a> {
.unwrap_or_default();
if has_alias {
// Mark the sub-importation as used.
if let Some(index) = scope.get(full_name.as_str()) {
self.ctx.bindings[*index].mark_used(
if let Some(binding_id) =
self.ctx.scopes[scope_id].get(full_name.as_str())
{
let context = self.ctx.execution_context();
self.ctx.bindings[*binding_id].mark_used(
self.ctx.scope_id,
expr.range(),
context,
Expand All @@ -4804,72 +4788,70 @@ impl<'a> Checker<'a> {
}
_ => {}
}

return;
}

first_iter = false;
import_starred = import_starred || scope.uses_star_imports();
}

if import_starred {
// F405
if self
.settings
.rules
.enabled(Rule::UndefinedLocalWithImportStarUsage)
{
let sources: Vec<String> = self
.ctx
.scopes
.iter()
.flat_map(Scope::star_imports)
.map(|StarImportation { level, module }| {
helpers::format_import_from(*level, *module)
})
.sorted()
.dedup()
.collect();
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithImportStarUsage {
name: id.to_string(),
sources,
},
expr.range(),
));
ResolvedReference::ImplicitGlobal => {
// Nothing to do.
}
return;
}

if self.settings.rules.enabled(Rule::UndefinedName) {
// Allow __path__.
if self.path.ends_with("__init__.py") && id == "__path__" {
return;
ResolvedReference::StarImport => {
// F405
if self
.settings
.rules
.enabled(Rule::UndefinedLocalWithImportStarUsage)
{
let sources: Vec<String> = self
.ctx
.scopes
.iter()
.flat_map(Scope::star_imports)
.map(|StarImportation { level, module }| {
helpers::format_import_from(*level, *module)
})
.sorted()
.dedup()
.collect();
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithImportStarUsage {
name: id.to_string(),
sources,
},
expr.range(),
));
}
}
ResolvedReference::NotFound => {
// F821
if self.settings.rules.enabled(Rule::UndefinedName) {
// Allow __path__.
if self.path.ends_with("__init__.py") && id == "__path__" {
return;
}

// Allow "__module__" and "__qualname__" in class scopes.
if (id == "__module__" || id == "__qualname__")
&& matches!(self.ctx.scope().kind, ScopeKind::Class(..))
{
return;
}
// Allow "__module__" and "__qualname__" in class scopes.
if (id == "__module__" || id == "__qualname__")
&& matches!(self.ctx.scope().kind, ScopeKind::Class(..))
{
return;
}

// Avoid flagging if NameError is handled.
if self
.ctx
.handled_exceptions
.iter()
.any(|handler_names| handler_names.contains(Exceptions::NAME_ERROR))
{
return;
}
// Avoid flagging if `NameError` is handled.
if self
.ctx
.handled_exceptions
.iter()
.any(|handler_names| handler_names.contains(Exceptions::NAME_ERROR))
{
return;
}

self.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedName {
name: id.to_string(),
},
expr.range(),
));
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedName {
name: id.to_string(),
},
expr.range(),
));
}
}
}
}

Expand Down
64 changes: 64 additions & 0 deletions crates/ruff_python_semantic/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::Path;

use bitflags::bitflags;
use nohash_hasher::{BuildNoHashHasher, IntMap};
use ruff_text_size::TextRange;
use rustpython_parser::ast::{Expr, Stmt};
use smallvec::smallvec;

Expand Down Expand Up @@ -113,6 +114,55 @@ impl<'a> Context<'a> {
.map_or(false, |binding| binding.kind.is_builtin())
}

/// Resolve a reference to the given symbol.
pub fn resolve_reference(&mut self, symbol: &str, range: TextRange) -> ResolvedReference {
let mut first_iter = true;
let mut import_starred = false;
for scope_id in self.scopes.ancestor_ids(self.scope_id) {
let scope = &self.scopes[scope_id];
if scope.kind.is_class() {
if symbol == "__class__" {
return ResolvedReference::ImplicitGlobal;
} else if !first_iter {
continue;
}
}

if let Some(binding_id) = scope.get(symbol) {
// Mark the binding as used.
let context = self.execution_context();
self.bindings[*binding_id].mark_used(self.scope_id, range, context);

// But if it's a type annotation, don't treat it as resolved, unless we're in a
// forward reference. For example, given:
//
// ```python
// name: str
// print(name)
// ```
//
// The `name` in `print(name)` should be treated as unresolved, but the `name` in
// `name: str` should be treated as used.
if !self.in_deferred_type_definition()
&& self.bindings[*binding_id].kind.is_annotation()
{
continue;
}

return ResolvedReference::Resolved(scope_id, *binding_id);
}

first_iter = false;
import_starred = import_starred || scope.uses_star_imports();
}

if import_starred {
ResolvedReference::StarImport
} else {
ResolvedReference::NotFound
}
}

/// Resolves the [`Expr`] to a fully-qualified symbol-name, if `value` resolves to an imported
/// or builtin symbol.
///
Expand Down Expand Up @@ -708,3 +758,17 @@ pub struct Snapshot {
definition_id: DefinitionId,
flags: ContextFlags,
}

#[derive(Debug)]
pub enum ResolvedReference {
/// The reference is resolved to a specific binding.
Resolved(ScopeId, BindingId),
/// The reference is resolved to a context-specific, implicit global (e.g., `__class__` within
/// a class scope).
ImplicitGlobal,
/// The reference is unresolved, but at least one of the containing scopes contains a star
/// import.
StarImport,
/// The reference is definitively unresolved.
NotFound,
}

0 comments on commit 728c881

Please sign in to comment.