From 7be26dafe974b2858ff6a58869709e1317e52f59 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Sep 2022 13:54:11 -0400 Subject: [PATCH] Try to annotate lifetimes --- resources/test/fixtures/F821.py | 93 +++++++++++----------- src/ast_ops.rs | 2 + src/check_ast.rs | 132 ++++++++++++++++++++++---------- src/linter.rs | 13 +++- src/visitor.rs | 94 +++++++++++++---------- 5 files changed, 202 insertions(+), 132 deletions(-) diff --git a/resources/test/fixtures/F821.py b/resources/test/fixtures/F821.py index a50b65869e..0b47c98897 100644 --- a/resources/test/fixtures/F821.py +++ b/resources/test/fixtures/F821.py @@ -1,51 +1,50 @@ -def get_name(): - return self.name - - -def get_name(): - return (self.name,) - - -def get_name(): - del self.name - - -def get_name(self): - return self.name - - -x = list() - - -def randdec(maxprec, maxexp): - return numeric_string(maxprec, maxexp) - - -def ternary_optarg(prec, exp_range, itr): - for _ in range(100): - a = randdec(prec, 2 * exp_range) - b = randdec(prec, 2 * exp_range) - c = randdec(prec, 2 * exp_range) - yield a, b, c, None - yield a, b, c, None, None - - -class Foo: - CLASS_VAR = 1 - REFERENCES_CLASS_VAR = {"CLASS_VAR": CLASS_VAR} - ANNOTATED_CLASS_VAR: int = 2 - +# def get_name(): +# return self.name +# +# +# def get_name(): +# return (self.name,) +# +# +# def get_name(): +# del self.name +# +# +# def get_name(self): +# return self.name +# +# +# x = list() +# +# +# def randdec(maxprec, maxexp): +# return numeric_string(maxprec, maxexp) +# +# +# def ternary_optarg(prec, exp_range, itr): +# for _ in range(100): +# a = randdec(prec, 2 * exp_range) +# b = randdec(prec, 2 * exp_range) +# c = randdec(prec, 2 * exp_range) +# yield a, b, c, None +# yield a, b, c, None, None +# +# +# class Foo: +# CLASS_VAR = 1 +# REFERENCES_CLASS_VAR = {"CLASS_VAR": CLASS_VAR} +# ANNOTATED_CLASS_VAR: int = 2 +# class Class: def __init__(self): - # TODO(charlie): This should be recognized as a defined variable. - Class # noqa: F821 + Class - -try: - x = 1 / 0 -except Exception as e: - print(e) - - -y: int = 1 +# +# try: +# x = 1 / 0 +# except Exception as e: +# print(e) +# +# +# y: int = 1 diff --git a/src/ast_ops.rs b/src/ast_ops.rs index 8b32ec716f..b2c160142b 100644 --- a/src/ast_ops.rs +++ b/src/ast_ops.rs @@ -8,6 +8,7 @@ fn id() -> usize { COUNTER.fetch_add(1, Ordering::Relaxed) } +#[derive(Clone, Debug)] pub enum ScopeKind { Class, Function, @@ -15,6 +16,7 @@ pub enum ScopeKind { Module, } +#[derive(Clone, Debug)] pub struct Scope { pub id: usize, pub kind: ScopeKind, diff --git a/src/check_ast.rs b/src/check_ast.rs index 2eb5e28550..5a1e446c6a 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -25,15 +25,20 @@ struct Checker<'a> { checks: Vec, scopes: Vec, dead_scopes: Vec, - deferred: Vec, + deferred_annotations: Vec<&'a str>, + // I think I need to track the list of scope indexes... + // And then store all scopes. + // And track the current scope stack by index? + deferred_functions: Vec<(&'a Stmt, &'a [Scope])>, + stack: Vec<&'a Stmt>, in_f_string: bool, in_annotation: bool, seen_non_import: bool, seen_docstring: bool, } -impl Checker<'_> { - pub fn new<'a>( +impl<'a> Checker<'a> { + pub fn new( settings: &'a Settings, autofix: &'a autofix::Mode, path: &'a str, @@ -47,7 +52,9 @@ impl Checker<'_> { checks: vec![], scopes: vec![], dead_scopes: vec![], - deferred: vec![], + deferred_annotations: vec![], + deferred_functions: vec![], + stack: vec![], in_f_string: false, in_annotation: false, seen_non_import: false, @@ -70,8 +77,13 @@ fn convert_to_value(expr: &Expr) -> Option { } } -impl Visitor for Checker<'_> { - fn visit_stmt(&mut self, stmt: &Stmt) { +impl<'a, 'b> Visitor<'b> for Checker<'a> +where + 'b: 'a, +{ + fn visit_stmt(&mut self, stmt: &'b Stmt) { + self.stack.push(stmt); + match &stmt.node { StmtKind::Global { names } | StmtKind::Nonlocal { names } => { // TODO(charlie): Handle doctests. @@ -119,7 +131,10 @@ impl Visitor for Checker<'_> { location: stmt.location, }, ); - self.push_scope(Scope::new(ScopeKind::Function)); + + self.deferred_functions.push((stmt, &self.scopes)); + + // self.push_scope(Scope::new(ScopeKind::Function)); } StmtKind::Return { .. } => { if self @@ -434,29 +449,29 @@ impl Visitor for Checker<'_> { self.dead_scopes.push(scope); } } - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { - let scope = self.scopes.last().expect("No current scope found."); - for (name, binding) in scope.values.iter() { - // TODO(charlie): Ignore if using `locals`. - if self.settings.select.contains(&CheckCode::F841) - && binding.used.is_none() - && name != "_" - && name != "__tracebackhide__" - && name != "__traceback_info__" - && name != "__traceback_supplement__" - && matches!(binding.kind, BindingKind::Assignment) - { - self.checks.push(Check::new( - CheckKind::UnusedVariable(name.to_string()), - binding.location, - )); - } - } - - if let Some(scope) = self.scopes.pop() { - self.dead_scopes.push(scope); - } - } + // StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { + // let scope = self.scopes.last().expect("No current scope found."); + // for (name, binding) in scope.values.iter() { + // // TODO(charlie): Ignore if using `locals`. + // if self.settings.select.contains(&CheckCode::F841) + // && binding.used.is_none() + // && name != "_" + // && name != "__tracebackhide__" + // && name != "__traceback_info__" + // && name != "__traceback_supplement__" + // && matches!(binding.kind, BindingKind::Assignment) + // { + // self.checks.push(Check::new( + // CheckKind::UnusedVariable(name.to_string()), + // binding.location, + // )); + // } + // } + // + // if let Some(scope) = self.scopes.pop() { + // self.dead_scopes.push(scope); + // } + // } _ => {} }; @@ -471,14 +486,15 @@ impl Visitor for Checker<'_> { ); } } - fn visit_annotation(&mut self, expr: &Expr) { + + fn visit_annotation(&mut self, expr: &'b Expr) { let initial = self.in_annotation; self.in_annotation = true; self.visit_expr(expr, None); self.in_annotation = initial; } - fn visit_expr(&mut self, expr: &Expr, parent: Option<&Stmt>) { + fn visit_expr(&mut self, expr: &'b Expr, parent: Option<&Stmt>) { let initial = self.in_f_string; match &expr.node { ExprKind::Name { ctx, .. } => match ctx { @@ -718,7 +734,7 @@ impl Visitor for Checker<'_> { ExprKind::Constant { value: Constant::Str(value), .. - } if self.in_annotation => self.deferred.push(value.to_string()), + } if self.in_annotation => self.deferred_annotations.push(value), _ => {} }; @@ -741,7 +757,7 @@ impl Visitor for Checker<'_> { }; } - fn visit_excepthandler(&mut self, excepthandler: &Excepthandler) { + fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { match &excepthandler.node { ExcepthandlerKind::ExceptHandler { name, .. } => match name { Some(name) => { @@ -794,7 +810,7 @@ impl Visitor for Checker<'_> { } } - fn visit_arguments(&mut self, arguments: &Arguments) { + fn visit_arguments(&mut self, arguments: &'b Arguments) { if self .settings .select @@ -830,7 +846,7 @@ impl Visitor for Checker<'_> { visitor::walk_arguments(self, arguments); } - fn visit_arg(&mut self, arg: &Arg) { + fn visit_arg(&mut self, arg: &'b Arg) { self.add_binding( arg.node.arg.to_string(), Binding { @@ -1000,11 +1016,12 @@ impl Checker<'_> { } fn check_deferred(&mut self, path: &str) { - for value in self.deferred.clone() { - if let Ok(expr) = &parser::parse_expression(&value, path) { - self.visit_expr(expr, None); - } - } + // while !self.deferred.is_empty() { + // let value = self.deferred.pop().unwrap(); + // if let Ok(expr) = parser::parse_expression(&value, path) { + // self.visit_expr(&expr, None); + // } + // } } fn check_dead_scopes(&mut self) { @@ -1069,6 +1086,8 @@ pub fn check_ast( settings: &Settings, autofix: &autofix::Mode, path: &str, + stmt_allocator: &mut Vec, + expr_allocator: &mut Vec, ) -> Vec { let mut checker = Checker::new(settings, autofix, path, content); checker.push_scope(Scope::new(ScopeKind::Module)); @@ -1077,7 +1096,36 @@ pub fn check_ast( for stmt in python_ast { checker.visit_stmt(stmt); } - checker.check_deferred(path); + + // Check deferred annotations. + while !checker.deferred_annotations.is_empty() { + let value = checker.deferred_annotations.pop().unwrap(); + if let Ok(expr) = parser::parse_expression(&value, path) { + expr_allocator.push(expr); + } + } + for expr in expr_allocator { + checker.visit_expr(expr, None); + } + + // Check deferred annotations. + while !checker.deferred_functions.is_empty() { + let (stmt, scopes) = checker.deferred_functions.pop().unwrap(); + + checker.scopes = scopes.to_vec(); + checker.scopes.push(Scope::new(ScopeKind::Function)); + match &stmt.node { + StmtKind::FunctionDef { body, .. } | StmtKind::AsyncFunctionDef { body, .. } => { + for stmt in body { + checker.visit_stmt(stmt); + } + } + _ => {} + } + if let Some(scope) = checker.scopes.pop() { + checker.dead_scopes.push(scope); + } + } checker.pop_scope(); checker.check_dead_scopes(); diff --git a/src/linter.rs b/src/linter.rs index 723617c3ff..774b4f60a4 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -2,6 +2,7 @@ use std::path::Path; use anyhow::Result; use log::debug; +use rustpython_parser::ast::{Expr, Stmt}; use rustpython_parser::parser; use crate::autofix::fix_file; @@ -27,7 +28,17 @@ fn check_path(path: &Path, settings: &Settings, autofix: &autofix::Mode) -> Resu { let path = path.to_string_lossy(); let python_ast = parser::parse_program(&contents, &path)?; - checks.extend(check_ast(&python_ast, &contents, settings, autofix, &path)); + let mut stmt_allocator: Vec = vec![]; + let mut expr_allocator: Vec = vec![]; + checks.extend(check_ast( + &python_ast, + &contents, + settings, + autofix, + &path, + &mut stmt_allocator, + &mut expr_allocator, + )); } // Run the lines-based checks. diff --git a/src/visitor.rs b/src/visitor.rs index b0fadc8a43..d9780506b6 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -4,76 +4,76 @@ use rustpython_parser::ast::{ PatternKind, Stmt, StmtKind, Unaryop, Withitem, }; -pub trait Visitor { - fn visit_stmt(&mut self, stmt: &Stmt) { +pub trait Visitor<'a> { + fn visit_stmt(&mut self, stmt: &'a Stmt) { walk_stmt(self, stmt); } - fn visit_annotation(&mut self, expr: &Expr) { + fn visit_annotation(&mut self, expr: &'a Expr) { walk_expr(self, expr); } - fn visit_expr(&mut self, expr: &Expr, _parent: Option<&Stmt>) { + fn visit_expr(&mut self, expr: &'a Expr, _parent: Option<&Stmt>) { walk_expr(self, expr); } - fn visit_constant(&mut self, constant: &Constant) { + fn visit_constant(&mut self, constant: &'a Constant) { walk_constant(self, constant); } - fn visit_expr_context(&mut self, expr_content: &ExprContext) { + fn visit_expr_context(&mut self, expr_content: &'a ExprContext) { walk_expr_context(self, expr_content); } - fn visit_boolop(&mut self, boolop: &Boolop) { + fn visit_boolop(&mut self, boolop: &'a Boolop) { walk_boolop(self, boolop); } - fn visit_operator(&mut self, operator: &Operator) { + fn visit_operator(&mut self, operator: &'a Operator) { walk_operator(self, operator); } - fn visit_unaryop(&mut self, unaryop: &Unaryop) { + fn visit_unaryop(&mut self, unaryop: &'a Unaryop) { walk_unaryop(self, unaryop); } - fn visit_cmpop(&mut self, cmpop: &Cmpop) { + fn visit_cmpop(&mut self, cmpop: &'a Cmpop) { walk_cmpop(self, cmpop); } - fn visit_comprehension(&mut self, comprehension: &Comprehension) { + fn visit_comprehension(&mut self, comprehension: &'a Comprehension) { walk_comprehension(self, comprehension); } - fn visit_excepthandler(&mut self, excepthandler: &Excepthandler) { + fn visit_excepthandler(&mut self, excepthandler: &'a Excepthandler) { walk_excepthandler(self, excepthandler); } - fn visit_arguments(&mut self, arguments: &Arguments) { + fn visit_arguments(&mut self, arguments: &'a Arguments) { walk_arguments(self, arguments); } - fn visit_arg(&mut self, arg: &Arg) { + fn visit_arg(&mut self, arg: &'a Arg) { walk_arg(self, arg); } - fn visit_keyword(&mut self, keyword: &Keyword) { + fn visit_keyword(&mut self, keyword: &'a Keyword) { walk_keyword(self, keyword); } - fn visit_alias(&mut self, alias: &Alias) { + fn visit_alias(&mut self, alias: &'a Alias) { walk_alias(self, alias); } - fn visit_withitem(&mut self, withitem: &Withitem) { + fn visit_withitem(&mut self, withitem: &'a Withitem) { walk_withitem(self, withitem); } - fn visit_match_case(&mut self, match_case: &MatchCase) { + fn visit_match_case(&mut self, match_case: &'a MatchCase) { walk_match_case(self, match_case); } - fn visit_pattern(&mut self, pattern: &Pattern) { + fn visit_pattern(&mut self, pattern: &'a Pattern) { walk_pattern(self, pattern); } } -pub fn walk_stmt(visitor: &mut V, stmt: &Stmt) { +pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { match &stmt.node { StmtKind::FunctionDef { args, body, .. } => { visitor.visit_arguments(args); - for stmt in body { - visitor.visit_stmt(stmt) - } + // for stmt in body { + // visitor.visit_stmt(stmt) + // } } StmtKind::AsyncFunctionDef { args, body, .. } => { visitor.visit_arguments(args); - for stmt in body { - visitor.visit_stmt(stmt) - } + // for stmt in body { + // visitor.visit_stmt(stmt) + // } } StmtKind::ClassDef { body, .. } => { for stmt in body { @@ -238,7 +238,7 @@ pub fn walk_stmt(visitor: &mut V, stmt: &Stmt) { } } -pub fn walk_expr(visitor: &mut V, expr: &Expr) { +pub fn walk_expr<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) { match &expr.node { ExprKind::BoolOp { op, values } => { visitor.visit_boolop(op); @@ -399,7 +399,7 @@ pub fn walk_expr(visitor: &mut V, expr: &Expr) { } } -pub fn walk_constant(visitor: &mut V, constant: &Constant) { +pub fn walk_constant<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, constant: &'a Constant) { if let Constant::Tuple(constants) = constant { for constant in constants { visitor.visit_constant(constant) @@ -407,7 +407,10 @@ pub fn walk_constant(visitor: &mut V, constant: &Constant) } } -pub fn walk_comprehension(visitor: &mut V, comprehension: &Comprehension) { +pub fn walk_comprehension<'a, V: Visitor<'a> + ?Sized>( + visitor: &mut V, + comprehension: &'a Comprehension, +) { visitor.visit_expr(&comprehension.target, None); visitor.visit_expr(&comprehension.iter, None); for expr in &comprehension.ifs { @@ -415,7 +418,10 @@ pub fn walk_comprehension(visitor: &mut V, comprehension: & } } -pub fn walk_excepthandler(visitor: &mut V, excepthandler: &Excepthandler) { +pub fn walk_excepthandler<'a, V: Visitor<'a> + ?Sized>( + visitor: &mut V, + excepthandler: &'a Excepthandler, +) { match &excepthandler.node { ExcepthandlerKind::ExceptHandler { type_, body, .. } => { if let Some(expr) = type_ { @@ -428,7 +434,7 @@ pub fn walk_excepthandler(visitor: &mut V, excepthandler: & } } -pub fn walk_arguments(visitor: &mut V, arguments: &Arguments) { +pub fn walk_arguments<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, arguments: &'a Arguments) { for arg in &arguments.posonlyargs { visitor.visit_arg(arg); } @@ -452,24 +458,24 @@ pub fn walk_arguments(visitor: &mut V, arguments: &Argument } } -pub fn walk_arg(visitor: &mut V, arg: &Arg) { +pub fn walk_arg<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, arg: &'a Arg) { if let Some(expr) = &arg.node.annotation { visitor.visit_annotation(expr) } } -pub fn walk_keyword(visitor: &mut V, keyword: &Keyword) { +pub fn walk_keyword<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, keyword: &'a Keyword) { visitor.visit_expr(&keyword.node.value, None); } -pub fn walk_withitem(visitor: &mut V, withitem: &Withitem) { +pub fn walk_withitem<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, withitem: &'a Withitem) { visitor.visit_expr(&withitem.context_expr, None); if let Some(expr) = &withitem.optional_vars { visitor.visit_expr(expr, None); } } -pub fn walk_match_case(visitor: &mut V, match_case: &MatchCase) { +pub fn walk_match_case<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, match_case: &'a MatchCase) { visitor.visit_pattern(&match_case.pattern); if let Some(expr) = &match_case.guard { visitor.visit_expr(expr, None); @@ -479,7 +485,7 @@ pub fn walk_match_case(visitor: &mut V, match_case: &MatchC } } -pub fn walk_pattern(visitor: &mut V, pattern: &Pattern) { +pub fn walk_pattern<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, pattern: &'a Pattern) { match &pattern.node { PatternKind::MatchValue { value } => visitor.visit_expr(value, None), PatternKind::MatchSingleton { value } => visitor.visit_constant(value), @@ -527,24 +533,28 @@ pub fn walk_pattern(visitor: &mut V, pattern: &Pattern) { #[allow(unused_variables)] #[inline(always)] -pub fn walk_expr_context(visitor: &mut V, expr_context: &ExprContext) {} +pub fn walk_expr_context<'a, V: Visitor<'a> + ?Sized>( + visitor: &mut V, + expr_context: &'a ExprContext, +) { +} #[allow(unused_variables)] #[inline(always)] -pub fn walk_boolop(visitor: &mut V, boolop: &Boolop) {} +pub fn walk_boolop<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, boolop: &'a Boolop) {} #[allow(unused_variables)] #[inline(always)] -pub fn walk_operator(visitor: &mut V, operator: &Operator) {} +pub fn walk_operator<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, operator: &'a Operator) {} #[allow(unused_variables)] #[inline(always)] -pub fn walk_unaryop(visitor: &mut V, unaryop: &Unaryop) {} +pub fn walk_unaryop<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, unaryop: &'a Unaryop) {} #[allow(unused_variables)] #[inline(always)] -pub fn walk_cmpop(visitor: &mut V, cmpop: &Cmpop) {} +pub fn walk_cmpop<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, cmpop: &'a Cmpop) {} #[allow(unused_variables)] #[inline(always)] -pub fn walk_alias(visitor: &mut V, alias: &Alias) {} +pub fn walk_alias<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, alias: &'a Alias) {}