Skip to content

Commit

Permalink
Compare formatted and unformatted ASTs during formatter tests (#8624)
Browse files Browse the repository at this point in the history
## Summary

This PR implements validation in the formatter tests to ensure that we
don't modify the AST during formatting. Black has similar logic.

In implementing this, I learned that Black actually _does_ modify the
AST, and their test infrastructure normalizes the AST to wipe away those
differences. Specifically, Black changes the indentation of docstrings,
which _does_ modify the AST; and it also inserts parentheses in `del`
statements, which changes the AST too.

Ruff also does both these things, so we _also_ implement the same
normalization using a new visitor that allows for modifying the AST.

Closes #8184.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Nov 13, 2023
1 parent 3592f44 commit d574fcd
Show file tree
Hide file tree
Showing 5 changed files with 930 additions and 19 deletions.
41 changes: 41 additions & 0 deletions crates/ruff_python_ast/src/comparable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,3 +1480,44 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> {
}
}
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub enum ComparableMod<'a> {
Module(ComparableModModule<'a>),
Expression(ComparableModExpression<'a>),
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ComparableModModule<'a> {
body: Vec<ComparableStmt<'a>>,
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ComparableModExpression<'a> {
body: Box<ComparableExpr<'a>>,
}

impl<'a> From<&'a ast::Mod> for ComparableMod<'a> {
fn from(mod_: &'a ast::Mod) -> Self {
match mod_ {
ast::Mod::Module(module) => Self::Module(module.into()),
ast::Mod::Expression(expr) => Self::Expression(expr.into()),
}
}
}

impl<'a> From<&'a ast::ModModule> for ComparableModModule<'a> {
fn from(module: &'a ast::ModModule) -> Self {
Self {
body: module.body.iter().map(Into::into).collect(),
}
}
}

impl<'a> From<&'a ast::ModExpression> for ComparableModExpression<'a> {
fn from(expr: &'a ast::ModExpression) -> Self {
Self {
body: (&expr.body).into(),
}
}
}
5 changes: 4 additions & 1 deletion crates/ruff_python_ast/src/visitor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! AST visitor trait and walk functions.
pub mod preorder;
pub mod transformer;

use crate::{
self as ast, Alias, Arguments, BoolOp, CmpOp, Comprehension, Decorator, ElifElseClause,
Expand All @@ -14,8 +15,10 @@ use crate::{
/// Prefer [`crate::statement_visitor::StatementVisitor`] for visitors that only need to visit
/// statements.
///
/// Use the [`PreorderVisitor`](self::preorder::PreorderVisitor) if you want to visit the nodes
/// Use the [`PreorderVisitor`](preorder::PreorderVisitor) if you want to visit the nodes
/// in pre-order rather than evaluation order.
///
/// Use the [`Transformer`](transformer::Transformer) if you want to modify the nodes.
pub trait Visitor<'a> {
fn visit_stmt(&mut self, stmt: &'a Stmt) {
walk_stmt(self, stmt);
Expand Down
Loading

0 comments on commit d574fcd

Please sign in to comment.