Skip to content

Commit

Permalink
Recursively visit deferred AST nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 16, 2024
1 parent 5c424f0 commit ed1d8d0
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Test: ensure that we visit type definitions and lambdas recursively."""

from __future__ import annotations

from collections import Counter
Expand Down
11 changes: 11 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F401_22.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"""Test: ensure that we visit type definitions and lambdas recursively."""

from __future__ import annotations

import re
import os
from typing import cast

cast(lambda: re.match, 1)

cast(lambda: cast(lambda: os.path, 1), 1)
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::rules::{flake8_bugbear, flake8_simplify, perflint, pyupgrade, refurb}

/// Run lint rules over all deferred for-loops in the [`SemanticModel`].
pub(crate) fn deferred_for_loops(checker: &mut Checker) {
while !checker.deferred.for_loops.is_empty() {
let for_loops = std::mem::take(&mut checker.deferred.for_loops);
while !checker.analyze.for_loops.is_empty() {
let for_loops = std::mem::take(&mut checker.analyze.for_loops);
for snapshot in for_loops {
checker.semantic.restore(snapshot);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::rules::{flake8_pie, pylint, refurb};

/// Run lint rules over all deferred lambdas in the [`SemanticModel`].
pub(crate) fn deferred_lambdas(checker: &mut Checker) {
while !checker.deferred.lambdas.is_empty() {
let lambdas = std::mem::take(&mut checker.deferred.lambdas);
while !checker.analyze.lambdas.is_empty() {
let lambdas = std::mem::take(&mut checker.analyze.lambdas);
for snapshot in lambdas {
checker.semantic.restore(snapshot);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
};

let mut diagnostics: Vec<Diagnostic> = vec![];
for scope_id in checker.deferred.scopes.iter().rev().copied() {
for scope_id in checker.analyze.scopes.iter().rev().copied() {
let scope = &checker.semantic.scopes[scope_id];

if checker.enabled(Rule::UndefinedLocal) {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::UnusedLoopControlVariable,
Rule::YieldInForLoop,
]) {
checker.deferred.for_loops.push(checker.semantic.snapshot());
checker.analyze.for_loops.push(checker.semantic.snapshot());
}
if checker.enabled(Rule::LoopVariableOverridesIterator) {
flake8_bugbear::rules::loop_variable_overrides_iterator(checker, target, iter);
Expand Down
28 changes: 23 additions & 5 deletions crates/ruff_linter/src/checkers/ast/deferred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,34 @@ use ruff_python_ast::Expr;
use ruff_python_semantic::{ScopeId, Snapshot};
use ruff_text_size::TextRange;

/// A collection of AST nodes that are deferred for later analysis.
/// Used to, e.g., store functions, whose bodies shouldn't be analyzed until all
/// module-level definitions have been analyzed.
/// A collection of AST nodes that are deferred for later visitation. Used to, e.g., store
/// functions, whose bodies shouldn't be visited until all module-level definitions have been
/// visited.
#[derive(Debug, Default)]
pub(crate) struct Deferred<'a> {
pub(crate) scopes: Vec<ScopeId>,
pub(crate) struct Visit<'a> {
pub(crate) string_type_definitions: Vec<(TextRange, &'a str, Snapshot)>,
pub(crate) future_type_definitions: Vec<(&'a Expr, Snapshot)>,
pub(crate) type_param_definitions: Vec<(&'a Expr, Snapshot)>,
pub(crate) functions: Vec<Snapshot>,
pub(crate) lambdas: Vec<Snapshot>,
}

impl Visit<'_> {
/// Returns `true` if there are no deferred nodes.
pub(crate) fn is_empty(&self) -> bool {
self.string_type_definitions.is_empty()
&& self.future_type_definitions.is_empty()
&& self.type_param_definitions.is_empty()
&& self.functions.is_empty()
&& self.lambdas.is_empty()
}
}

/// A collection of AST nodes to be analyzed after the AST traversal. Used to, e.g., store
/// all `for` loops, so that they can be analyzed after the entire AST has been visited.
#[derive(Debug, Default)]
pub(crate) struct Analyze {
pub(crate) scopes: Vec<ScopeId>,
pub(crate) lambdas: Vec<Snapshot>,
pub(crate) for_loops: Vec<Snapshot>,
}
83 changes: 46 additions & 37 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind};
use ruff_python_semantic::analyze::{imports, typing, visibility};
use ruff_python_semantic::{
BindingFlags, BindingId, BindingKind, Exceptions, Export, FromImport, Globals, Import, Module,
ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, Snapshot,
StarImport, SubmoduleImport,
ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, StarImport,
SubmoduleImport,
};
use ruff_python_stdlib::builtins::{IPYTHON_BUILTINS, MAGIC_GLOBALS, PYTHON_BUILTINS};
use ruff_source_file::{Locator, OneIndexed, SourceRow};

use crate::checkers::ast::annotation::AnnotationContext;
use crate::checkers::ast::deferred::Deferred;
use crate::checkers::ast::deferred::{Analyze, Visit};
use crate::docstrings::extraction::ExtractionTarget;
use crate::importer::Importer;
use crate::noqa::NoqaMapping;
Expand Down Expand Up @@ -105,8 +105,10 @@ pub(crate) struct Checker<'a> {
importer: Importer<'a>,
/// The [`SemanticModel`], built up over the course of the AST traversal.
semantic: SemanticModel<'a>,
/// A set of deferred nodes to be processed after the current traversal (e.g., function bodies).
deferred: Deferred<'a>,
/// A set of deferred nodes to be visited after the current traversal (e.g., function bodies).
visit: Visit<'a>,
/// A set of deferred nodes to be analyzed after the AST traversal (e.g., `for` loops).
analyze: Analyze,
/// The cumulative set of diagnostics computed across all lint rules.
pub(crate) diagnostics: Vec<Diagnostic>,
/// The list of names already seen by flake8-bugbear diagnostics, to avoid duplicate violations..
Expand Down Expand Up @@ -145,7 +147,8 @@ impl<'a> Checker<'a> {
indexer,
importer,
semantic: SemanticModel::new(&settings.typing_modules, path, module),
deferred: Deferred::default(),
visit: Visit::default(),
analyze: Analyze::default(),
diagnostics: Vec::default(),
flake8_bugbear_seen: Vec::default(),
cell_offsets,
Expand Down Expand Up @@ -596,7 +599,7 @@ where
self.semantic.push_scope(ScopeKind::Function(function_def));
self.semantic.flags -= SemanticModelFlags::EXCEPTION_HANDLER;

self.deferred.functions.push(self.semantic.snapshot());
self.visit.functions.push(self.semantic.snapshot());

// Extract any global bindings from the function body.
if let Some(globals) = Globals::from_body(body) {
Expand Down Expand Up @@ -652,7 +655,7 @@ where
if let Some(type_params) = type_params {
self.visit_type_params(type_params);
}
self.deferred
self.visit
.type_param_definitions
.push((value, self.semantic.snapshot()));
self.semantic.pop_scope();
Expand Down Expand Up @@ -785,7 +788,7 @@ where
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => {
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.analyze.scopes.push(scope_id);
self.semantic.pop_scope(); // Function scope
self.semantic.pop_definition();
self.semantic.pop_scope(); // Type parameter scope
Expand All @@ -798,7 +801,7 @@ where
}
Stmt::ClassDef(ast::StmtClassDef { name, .. }) => {
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.analyze.scopes.push(scope_id);
self.semantic.pop_scope(); // Class scope
self.semantic.pop_definition();
self.semantic.pop_scope(); // Type parameter scope
Expand Down Expand Up @@ -835,13 +838,13 @@ where
&& self.semantic.future_annotations()
{
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
self.deferred.string_type_definitions.push((
self.visit.string_type_definitions.push((
expr.range(),
value.to_str(),
self.semantic.snapshot(),
));
} else {
self.deferred
self.visit
.future_type_definitions
.push((expr, self.semantic.snapshot()));
}
Expand Down Expand Up @@ -945,7 +948,8 @@ where
}

self.semantic.push_scope(ScopeKind::Lambda(lambda));
self.deferred.lambdas.push(self.semantic.snapshot());
self.visit.lambdas.push(self.semantic.snapshot());
self.analyze.lambdas.push(self.semantic.snapshot());
}
Expr::IfExp(ast::ExprIfExp {
test,
Expand Down Expand Up @@ -1252,7 +1256,7 @@ where
}
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
if self.semantic.in_type_definition() && !self.semantic.in_typing_literal() {
self.deferred.string_type_definitions.push((
self.visit.string_type_definitions.push((
expr.range(),
value.to_str(),
self.semantic.snapshot(),
Expand All @@ -1273,7 +1277,7 @@ where
| Expr::ListComp(_)
| Expr::DictComp(_)
| Expr::SetComp(_) => {
self.deferred.scopes.push(self.semantic.scope_id);
self.analyze.scopes.push(self.semantic.scope_id);
self.semantic.pop_scope();
}
_ => {}
Expand Down Expand Up @@ -1447,7 +1451,7 @@ where
bound: Some(bound), ..
}) = type_param
{
self.deferred
self.visit
.type_param_definitions
.push((bound, self.semantic.snapshot()));
}
Expand Down Expand Up @@ -1809,8 +1813,8 @@ impl<'a> Checker<'a> {

fn visit_deferred_future_type_definitions(&mut self) {
let snapshot = self.semantic.snapshot();
while !self.deferred.future_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.deferred.future_type_definitions);
while !self.visit.future_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.visit.future_type_definitions);
for (expr, snapshot) in type_definitions {
self.semantic.restore(snapshot);

Expand All @@ -1824,8 +1828,8 @@ impl<'a> Checker<'a> {

fn visit_deferred_type_param_definitions(&mut self) {
let snapshot = self.semantic.snapshot();
while !self.deferred.type_param_definitions.is_empty() {
let type_params = std::mem::take(&mut self.deferred.type_param_definitions);
while !self.visit.type_param_definitions.is_empty() {
let type_params = std::mem::take(&mut self.visit.type_param_definitions);
for (type_param, snapshot) in type_params {
self.semantic.restore(snapshot);

Expand All @@ -1839,8 +1843,8 @@ impl<'a> Checker<'a> {

fn visit_deferred_string_type_definitions(&mut self, allocator: &'a typed_arena::Arena<Expr>) {
let snapshot = self.semantic.snapshot();
while !self.deferred.string_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.deferred.string_type_definitions);
while !self.visit.string_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.visit.string_type_definitions);
for (range, value, snapshot) in type_definitions {
if let Ok((expr, kind)) =
parse_type_annotation(value, range, self.locator.contents())
Expand Down Expand Up @@ -1887,8 +1891,8 @@ impl<'a> Checker<'a> {

fn visit_deferred_functions(&mut self) {
let snapshot = self.semantic.snapshot();
while !self.deferred.functions.is_empty() {
let deferred_functions = std::mem::take(&mut self.deferred.functions);
while !self.visit.functions.is_empty() {
let deferred_functions = std::mem::take(&mut self.visit.functions);
for snapshot in deferred_functions {
self.semantic.restore(snapshot);

Expand All @@ -1906,11 +1910,12 @@ impl<'a> Checker<'a> {
self.semantic.restore(snapshot);
}

/// Visit all deferred lambdas. Returns a list of snapshots, such that the caller can restore
/// the semantic model to the state it was in before visiting the deferred lambdas.
fn visit_deferred_lambdas(&mut self) {
let snapshot = self.semantic.snapshot();
let mut deferred: Vec<Snapshot> = Vec::with_capacity(self.deferred.lambdas.len());
while !self.deferred.lambdas.is_empty() {
let lambdas = std::mem::take(&mut self.deferred.lambdas);
while !self.visit.lambdas.is_empty() {
let lambdas = std::mem::take(&mut self.visit.lambdas);
for snapshot in lambdas {
self.semantic.restore(snapshot);

Expand All @@ -1927,15 +1932,23 @@ impl<'a> Checker<'a> {
self.visit_parameters(parameters);
}
self.visit_expr(body);

deferred.push(snapshot);
}
}
// Reset the deferred lambdas, so we can analyze them later on.
self.deferred.lambdas = deferred;
self.semantic.restore(snapshot);
}

/// Recursively visit all deferred AST nodes, including lambdas, functions, and type
/// annotations.
fn visit_deferred(&mut self, allocator: &'a typed_arena::Arena<Expr>) {
while !self.visit.is_empty() {
self.visit_deferred_functions();
self.visit_deferred_type_param_definitions();
self.visit_deferred_lambdas();
self.visit_deferred_future_type_definitions();
self.visit_deferred_string_type_definitions(&allocator);
}
}

/// Run any lint rules that operate over the module exports (i.e., members of `__all__`).
fn visit_exports(&mut self) {
let snapshot = self.semantic.snapshot();
Expand Down Expand Up @@ -2043,12 +2056,8 @@ pub(crate) fn check_ast(
// Visit any deferred syntax nodes. Take care to visit in order, such that we avoid adding
// new deferred nodes after visiting nodes of that kind. For example, visiting a deferred
// function can add a deferred lambda, but the opposite is not true.
checker.visit_deferred_functions();
checker.visit_deferred_type_param_definitions();
checker.visit_deferred_lambdas();
checker.visit_deferred_future_type_definitions();
let allocator = typed_arena::Arena::new();
checker.visit_deferred_string_type_definitions(&allocator);
checker.visit_deferred(&allocator);
checker.visit_exports();

// Check docstrings, bindings, and unresolved references.
Expand All @@ -2060,7 +2069,7 @@ pub(crate) fn check_ast(

// Reset the scope to module-level, and check all consumed scopes.
checker.semantic.scope_id = ScopeId::global();
checker.deferred.scopes.push(ScopeId::global());
checker.analyze.scopes.push(ScopeId::global());
analyze::deferred_scopes(&mut checker);

checker.diagnostics
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ mod tests {
#[test_case(Rule::UnusedImport, Path::new("F401_19.py"))]
#[test_case(Rule::UnusedImport, Path::new("F401_20.py"))]
#[test_case(Rule::UnusedImport, Path::new("F401_21.py"))]
#[test_case(Rule::UnusedImport, Path::new("F401_22.py"))]
#[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"))]
#[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.ipynb"))]
#[test_case(Rule::UndefinedLocalWithImportStar, Path::new("F403.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

0 comments on commit ed1d8d0

Please sign in to comment.