Skip to content

Commit

Permalink
Replace macro_rules! visitors with dedicated methods (#4402)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh authored May 12, 2023
1 parent f5be3d8 commit 490301f
Showing 1 changed file with 80 additions and 86 deletions.
166 changes: 80 additions & 86 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,48 +137,6 @@ impl<'a> Checker<'a> {
}
}

/// Visit an body of [`Stmt`] nodes within a type-checking block.
macro_rules! visit_type_checking_block {
($self:ident, $body:expr) => {{
let snapshot = $self.ctx.flags;
$self.ctx.flags |= ContextFlags::TYPE_CHECKING_BLOCK;
$self.visit_body($body);
$self.ctx.flags = snapshot;
}};
}

/// Visit an [`Expr`], and treat it as a type definition.
macro_rules! visit_type_definition {
($self:ident, $expr:expr) => {{
let snapshot = $self.ctx.flags;
$self.ctx.flags |= ContextFlags::TYPE_DEFINITION;
$self.visit_expr($expr);
$self.ctx.flags = snapshot;
}};
}

/// Visit an [`Expr`], and treat it as _not_ a type definition.
macro_rules! visit_non_type_definition {
($self:ident, $expr:expr) => {{
let snapshot = $self.ctx.flags;
$self.ctx.flags -= ContextFlags::TYPE_DEFINITION;
$self.visit_expr($expr);
$self.ctx.flags = snapshot;
}};
}

/// Visit an [`Expr`], and treat it as a boolean test. This is useful for detecting whether an
/// expressions return value is significant, or whether the calling context only relies on
/// its truthiness.
macro_rules! visit_boolean_test {
($self:ident, $expr:expr) => {{
let snapshot = $self.ctx.flags;
$self.ctx.flags |= ContextFlags::BOOLEAN_TEST;
$self.visit_expr($expr);
$self.ctx.flags = snapshot;
}};
}

impl<'a, 'b> Visitor<'b> for Checker<'a>
where
'b: 'a,
Expand Down Expand Up @@ -1919,7 +1877,7 @@ where
for arg in &args.posonlyargs {
if let Some(expr) = &arg.node.annotation {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
Expand All @@ -1928,7 +1886,7 @@ where
for arg in &args.args {
if let Some(expr) = &arg.node.annotation {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
Expand All @@ -1937,7 +1895,7 @@ where
if let Some(arg) = &args.vararg {
if let Some(expr) = &arg.node.annotation {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
Expand All @@ -1946,7 +1904,7 @@ where
for arg in &args.kwonlyargs {
if let Some(expr) = &arg.node.annotation {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
Expand All @@ -1955,15 +1913,15 @@ where
if let Some(arg) = &args.kwarg {
if let Some(expr) = &arg.node.annotation {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
}
}
for expr in returns {
if runtime_annotation {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_annotation(expr);
};
Expand Down Expand Up @@ -2170,39 +2128,39 @@ where
};

if runtime_annotation {
visit_type_definition!(self, annotation);
self.visit_type_definition(annotation);
} else {
self.visit_annotation(annotation);
}
if let Some(expr) = value {
if self.ctx.match_typing_expr(annotation, "TypeAlias") {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
} else {
self.visit_expr(expr);
}
}
self.visit_expr(target);
}
StmtKind::Assert(ast::StmtAssert { test, msg }) => {
visit_boolean_test!(self, test);
self.visit_boolean_test(test);
if let Some(expr) = msg {
self.visit_expr(expr);
}
}
StmtKind::While(ast::StmtWhile { test, body, orelse }) => {
visit_boolean_test!(self, test);
self.visit_boolean_test(test);
self.visit_body(body);
self.visit_body(orelse);
}
StmtKind::If(ast::StmtIf { test, body, orelse }) => {
visit_boolean_test!(self, test);
self.visit_boolean_test(test);

if flake8_type_checking::helpers::is_type_checking_block(&self.ctx, test) {
if self.settings.rules.enabled(Rule::EmptyTypeCheckingBlock) {
flake8_type_checking::rules::empty_type_checking_block(self, stmt, body);
}

visit_type_checking_block!(self, body);
self.visit_type_checking_block(body);
} else {
self.visit_body(body);
}
Expand Down Expand Up @@ -2245,7 +2203,7 @@ where
fn visit_annotation(&mut self, expr: &'b Expr) {
let flags_snapshot = self.ctx.flags;
self.ctx.flags |= ContextFlags::ANNOTATION;
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
self.ctx.flags = flags_snapshot;
}

Expand Down Expand Up @@ -3663,7 +3621,7 @@ where
self.deferred.lambdas.push((expr, self.ctx.snapshot()));
}
ExprKind::IfExp(ast::ExprIfExp { test, body, orelse }) => {
visit_boolean_test!(self, test);
self.visit_boolean_test(test);
self.visit_expr(body);
self.visit_expr(orelse);
}
Expand Down Expand Up @@ -3705,7 +3663,7 @@ where
Some(Callable::Bool) => {
self.visit_expr(func);
if !args.is_empty() {
visit_boolean_test!(self, &args[0]);
self.visit_boolean_test(&args[0]);
}
for expr in args.iter().skip(1) {
self.visit_expr(expr);
Expand All @@ -3714,7 +3672,7 @@ where
Some(Callable::Cast) => {
self.visit_expr(func);
if !args.is_empty() {
visit_type_definition!(self, &args[0]);
self.visit_type_definition(&args[0]);
}
for expr in args.iter().skip(1) {
self.visit_expr(expr);
Expand All @@ -3723,21 +3681,21 @@ where
Some(Callable::NewType) => {
self.visit_expr(func);
for expr in args.iter().skip(1) {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
}
}
Some(Callable::TypeVar) => {
self.visit_expr(func);
for expr in args.iter().skip(1) {
visit_type_definition!(self, expr);
self.visit_type_definition(expr);
}
for keyword in keywords {
let KeywordData { arg, value } = &keyword.node;
if let Some(id) = arg {
if id == "bound" {
visit_type_definition!(self, value);
self.visit_type_definition(value);
} else {
visit_non_type_definition!(self, value);
self.visit_non_type_definition(value);
}
}
}
Expand All @@ -3755,8 +3713,8 @@ where
ExprKind::List(ast::ExprList { elts, .. })
| ExprKind::Tuple(ast::ExprTuple { elts, .. }) => {
if elts.len() == 2 {
visit_non_type_definition!(self, &elts[0]);
visit_type_definition!(self, &elts[1]);
self.visit_non_type_definition(&elts[0]);
self.visit_type_definition(&elts[1]);
}
}
_ => {}
Expand All @@ -3770,7 +3728,7 @@ where
// Ex) NamedTuple("a", a=int)
for keyword in keywords {
let KeywordData { value, .. } = &keyword.node;
visit_type_definition!(self, value);
self.visit_type_definition(value);
}
}
Some(Callable::TypedDict) => {
Expand All @@ -3780,42 +3738,42 @@ where
if args.len() > 1 {
if let ExprKind::Dict(ast::ExprDict { keys, values }) = &args[1].node {
for key in keys.iter().flatten() {
visit_non_type_definition!(self, key);
self.visit_non_type_definition(key);
}
for value in values {
visit_type_definition!(self, value);
self.visit_type_definition(value);
}
}
}

// Ex) TypedDict("a", a=int)
for keyword in keywords {
let KeywordData { value, .. } = &keyword.node;
visit_type_definition!(self, value);
self.visit_type_definition(value);
}
}
Some(Callable::MypyExtension) => {
self.visit_expr(func);

if let Some(arg) = args.first() {
// Ex) DefaultNamedArg(bool | None, name="some_prop_name")
visit_type_definition!(self, arg);
self.visit_type_definition(arg);

for arg in args.iter().skip(1) {
visit_non_type_definition!(self, arg);
self.visit_non_type_definition(arg);
}
for keyword in keywords {
let KeywordData { value, .. } = &keyword.node;
visit_non_type_definition!(self, value);
self.visit_non_type_definition(value);
}
} else {
// Ex) DefaultNamedArg(type="bool", name="some_prop_name")
for keyword in keywords {
let KeywordData { value, arg } = &keyword.node;
if arg.as_ref().map_or(false, |arg| arg == "type") {
visit_type_definition!(self, value);
self.visit_type_definition(value);
} else {
visit_non_type_definition!(self, value);
self.visit_non_type_definition(value);
}
}
}
Expand All @@ -3826,11 +3784,11 @@ where
// any strings as deferred type definitions).
self.visit_expr(func);
for arg in args {
visit_non_type_definition!(self, arg);
self.visit_non_type_definition(arg);
}
for keyword in keywords {
let KeywordData { value, .. } = &keyword.node;
visit_non_type_definition!(self, value);
self.visit_non_type_definition(value);
}
}
}
Expand All @@ -3856,7 +3814,7 @@ where
// Ex) Optional[int]
SubscriptKind::AnnotatedSubscript => {
self.visit_expr(value);
visit_type_definition!(self, slice);
self.visit_type_definition(slice);
self.visit_expr_context(ctx);
}
// Ex) Annotated[int, "Hello, world!"]
Expand All @@ -3870,7 +3828,7 @@ where
if let Some(expr) = elts.first() {
self.visit_expr(expr);
for expr in elts.iter().skip(1) {
visit_non_type_definition!(self, expr);
self.visit_non_type_definition(expr);
}
self.visit_expr_context(ctx);
}
Expand Down Expand Up @@ -3921,7 +3879,7 @@ where
self.visit_expr(&comprehension.iter);
self.visit_expr(&comprehension.target);
for expr in &comprehension.ifs {
visit_boolean_test!(self, expr);
self.visit_boolean_test(expr);
}
}

Expand Down Expand Up @@ -4220,6 +4178,50 @@ where
}

impl<'a> Checker<'a> {
/// Visit a [`Module`]. Returns `true` if the module contains a module-level docstring.
fn visit_module(&mut self, python_ast: &'a Suite) -> bool {
if self.settings.rules.enabled(Rule::FStringDocstring) {
flake8_bugbear::rules::f_string_docstring(self, python_ast);
}
let docstring = docstrings::extraction::docstring_from(python_ast);
docstring.is_some()
}

/// Visit an body of [`Stmt`] nodes within a type-checking block.
fn visit_type_checking_block(&mut self, body: &'a [Stmt]) {
let snapshot = self.ctx.flags;
self.ctx.flags |= ContextFlags::TYPE_CHECKING_BLOCK;
self.visit_body(body);
self.ctx.flags = snapshot;
}

/// Visit an [`Expr`], and treat it as a type definition.
pub fn visit_type_definition(&mut self, expr: &'a Expr) {
let snapshot = self.ctx.flags;
self.ctx.flags |= ContextFlags::TYPE_DEFINITION;
self.visit_expr(expr);
self.ctx.flags = snapshot;
}

/// Visit an [`Expr`], and treat it as _not_ a type definition.
pub fn visit_non_type_definition(&mut self, expr: &'a Expr) {
let snapshot = self.ctx.flags;
self.ctx.flags -= ContextFlags::TYPE_DEFINITION;
self.visit_expr(expr);
self.ctx.flags = snapshot;
}

/// Visit an [`Expr`], and treat it as a boolean test. This is useful for detecting whether an
/// expressions return value is significant, or whether the calling context only relies on
/// its truthiness.
pub fn visit_boolean_test(&mut self, expr: &'a Expr) {
let snapshot = self.ctx.flags;
self.ctx.flags |= ContextFlags::BOOLEAN_TEST;
self.visit_expr(expr);
self.ctx.flags = snapshot;
}

/// Add a [`Binding`] to the current scope, bound to the given name.
fn add_binding(&mut self, name: &'a str, binding: Binding<'a>) {
let binding_id = self.ctx.bindings.next_id();
if let Some((stack_index, existing_binding_id)) = self
Expand Down Expand Up @@ -4790,14 +4792,6 @@ impl<'a> Checker<'a> {
));
}

fn visit_module(&mut self, python_ast: &'a Suite) -> bool {
if self.settings.rules.enabled(Rule::FStringDocstring) {
flake8_bugbear::rules::f_string_docstring(self, python_ast);
}
let docstring = docstrings::extraction::docstring_from(python_ast);
docstring.is_some()
}

fn check_deferred_future_type_definitions(&mut self) {
while !self.deferred.future_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.deferred.future_type_definitions);
Expand Down

0 comments on commit 490301f

Please sign in to comment.