Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace parents statement stack with a Nodes abstraction #4233

Merged
merged 3 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions crates/ruff/src/checkers/ast/deferred.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use ruff_text_size::TextRange;
use rustpython_parser::ast::{Expr, Stmt};

use ruff_python_ast::types::RefEquality;
use ruff_python_semantic::analyze::visibility::{Visibility, VisibleScope};
use ruff_python_semantic::node::NodeId;
use ruff_python_semantic::scope::ScopeId;

use crate::checkers::ast::AnnotationContext;
use crate::docstrings::definition::Definition;

type Context<'a> = (ScopeId, Vec<RefEquality<'a, Stmt>>);
/// A snapshot of the current scope and statement, which will be restored when visiting any
/// deferred definitions.
type Context<'a> = (ScopeId, Option<NodeId>);
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved

/// 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
Expand Down
141 changes: 63 additions & 78 deletions crates/ruff/src/checkers/ast/mod.rs

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::registry::{Linter, Rule};
use std::fmt::Formatter;

use crate::registry::{Linter, Rule};

#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub struct NoqaCode(&'static str, &'static str);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,9 @@ pub fn unused_loop_control_variable(
let scope = checker.ctx.scope();
let binding = scope.bindings_for_name(name).find_map(|index| {
let binding = &checker.ctx.bindings[*index];
binding
.source
.as_ref()
.and_then(|source| (source == &RefEquality(stmt)).then_some(binding))
binding.source.and_then(|source| {
(RefEquality(source) == RefEquality(stmt)).then_some(binding)
})
});
if let Some(binding) = binding {
if binding.kind.is_loop_var() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn match_async_exit_stack(checker: &Checker) -> bool {
if attr != "enter_async_context" {
return false;
}
for parent in &checker.ctx.parents {
for parent in checker.ctx.parents() {
if let StmtKind::With { items, .. } = &parent.node {
for item in items {
if let ExprKind::Call { func, .. } = &item.context_expr.node {
Expand Down Expand Up @@ -68,7 +68,7 @@ fn match_exit_stack(checker: &Checker) -> bool {
if attr != "enter_context" {
return false;
}
for parent in &checker.ctx.parents {
for parent in checker.ctx.parents() {
if let StmtKind::With { items, .. } = &parent.node {
for item in items {
if let ExprKind::Call { func, .. } = &item.context_expr.node {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,7 @@ pub fn empty_type_checking_block<'a, 'b>(

// Delete the entire type-checking block.
if checker.patch(diagnostic.kind.rule()) {
let parent = checker
.ctx
.child_to_parent
.get(&RefEquality(stmt))
.map(Into::into);
let parent = checker.ctx.stmts.parent(stmt);
let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect();
match delete_stmt(
stmt,
Expand Down
14 changes: 3 additions & 11 deletions crates/ruff/src/rules/pyflakes/rules/unused_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,7 @@ fn remove_unused_variable(
))
} else {
// If (e.g.) assigning to a constant (`x = 1`), delete the entire statement.
let parent = checker
.ctx
.child_to_parent
.get(&RefEquality(stmt))
.map(Into::into);
let parent = checker.ctx.stmts.parent(stmt);
let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect();
match delete_stmt(
stmt,
Expand Down Expand Up @@ -259,11 +255,7 @@ fn remove_unused_variable(
))
} else {
// If assigning to a constant (`x = 1`), delete the entire statement.
let parent = checker
.ctx
.child_to_parent
.get(&RefEquality(stmt))
.map(Into::into);
let parent = checker.ctx.stmts.parent(stmt);
let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect();
match delete_stmt(
stmt,
Expand Down Expand Up @@ -336,7 +328,7 @@ pub fn unused_variable(checker: &mut Checker, scope: ScopeId) {
binding.range,
);
if checker.patch(diagnostic.kind.rule()) {
if let Some(stmt) = binding.source.as_ref().map(Into::into) {
if let Some(stmt) = binding.source {
if let Some((kind, fix)) = remove_unused_variable(stmt, binding.range, checker)
{
if matches!(kind, DeletionKind::Whole) {
Expand Down
4 changes: 1 addition & 3 deletions crates/ruff/src/rules/pylint/rules/global_statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ pub fn global_statement(checker: &mut Checker, name: &str) {
if binding.kind.is_global() {
let source: &Stmt = binding
.source
.as_ref()
.expect("`global` bindings should always have a `source`")
.into();
.expect("`global` bindings should always have a `source`");
let diagnostic = Diagnostic::new(
GlobalStatement {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ fn fix_py2_block(
let defined_by = checker.ctx.current_stmt();
let defined_in = checker.ctx.current_stmt_parent();
return match delete_stmt(
defined_by.into(),
defined_by,
if block.starter == Tok::If {
defined_in.map(Into::into)
defined_in
} else {
None
},
Expand All @@ -176,7 +176,7 @@ fn fix_py2_block(
checker.stylist,
) {
Ok(fix) => {
checker.deletions.insert(RefEquality(defined_by.into()));
checker.deletions.insert(RefEquality(defined_by));
Some(fix)
}
Err(err) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustpython_parser::ast::{ArgData, Expr, ExprKind, Stmt, StmtKind};
use rustpython_parser::ast::{ArgData, Expr, ExprKind, StmtKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
use ruff_macros::{derive_message_formats, violation};
Expand Down Expand Up @@ -39,14 +39,13 @@ pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Exp
return;
}
let scope = checker.ctx.scope();
let parents: Vec<&Stmt> = checker.ctx.parents.iter().map(Into::into).collect();

// Check: are we in a Function scope?
if !matches!(scope.kind, ScopeKind::Function { .. }) {
return;
}

let mut parents = parents.iter().rev();
let mut parents = checker.ctx.parents();

// For a `super` invocation to be unnecessary, the first argument needs to match
// the enclosing class, and the second argument needs to match the first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustpython_parser::ast::{Alias, AliasData, Located, Stmt};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::RefEquality;

use crate::autofix;
use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -113,16 +114,16 @@ pub fn unnecessary_builtin_import(
.collect();
match autofix::actions::remove_unused_imports(
unused_imports.iter().map(String::as_str),
defined_by.into(),
defined_in.map(Into::into),
defined_by,
defined_in,
&deleted,
checker.locator,
checker.indexer,
checker.stylist,
) {
Ok(fix) => {
if fix.is_deletion() || fix.content() == Some("pass") {
checker.deletions.insert(*defined_by);
checker.deletions.insert(RefEquality(defined_by));
}
diagnostic.set_fix(fix);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustpython_parser::ast::{Alias, AliasData, Located, Stmt};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::RefEquality;

use crate::autofix;
use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -93,16 +94,16 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo
.collect();
match autofix::actions::remove_unused_imports(
unused_imports.iter().map(String::as_str),
defined_by.into(),
defined_in.map(Into::into),
defined_by,
defined_in,
&deleted,
checker.locator,
checker.indexer,
checker.stylist,
) {
Ok(fix) => {
if fix.is_deletion() || fix.content() == Some("pass") {
checker.deletions.insert(*defined_by);
checker.deletions.insert(RefEquality(defined_by));
}
diagnostic.set_fix(fix);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustpython_parser::ast::{Expr, ExprKind, Stmt};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::RefEquality;

use crate::autofix::actions;
use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -53,16 +54,16 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr,
let defined_by = checker.ctx.current_stmt();
let defined_in = checker.ctx.current_stmt_parent();
match actions::delete_stmt(
defined_by.into(),
defined_in.map(Into::into),
defined_by,
defined_in,
&deleted,
checker.locator,
checker.indexer,
checker.stylist,
) {
Ok(fix) => {
if fix.is_deletion() || fix.content() == Some("pass") {
checker.deletions.insert(*defined_by);
checker.deletions.insert(RefEquality(defined_by));
}
diagnostic.set_fix(fix);
}
Expand Down
113 changes: 0 additions & 113 deletions crates/ruff_python_ast/src/branch_detection.rs

This file was deleted.

1 change: 0 additions & 1 deletion crates/ruff_python_ast/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub mod all;
pub mod branch_detection;
pub mod call_path;
pub mod cast;
pub mod comparable;
Expand Down
12 changes: 12 additions & 0 deletions crates/ruff_python_ast/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,15 @@ impl<'a> From<&RefEquality<'a, Expr>> for &'a Expr {
r.0
}
}

impl<'a> From<RefEquality<'a, Stmt>> for &'a Stmt {
fn from(r: RefEquality<'a, Stmt>) -> Self {
r.0
}
}

impl<'a> From<RefEquality<'a, Expr>> for &'a Expr {
fn from(r: RefEquality<'a, Expr>) -> Self {
r.0
}
}
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
Loading