Skip to content

Commit

Permalink
Replace verbatim text with NOT_YET_IMPLEMENTED (#4904)
Browse files Browse the repository at this point in the history
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

This PR replaces the `verbatim_text` builder with a `not_yet_implemented` builder that emits `NOT_YET_IMPLEMENTED_<NodeKind>` for not yet implemented nodes. 

The motivation for this change is that partially formatting compound statements can result in incorrectly indented code, which is a syntax error:

```python
def func_no_args():
  a; b; c
  if True: raise RuntimeError
  if False: ...
  for i in range(10):
    print(i)
    continue
```

Get's reformatted to

```python
def func_no_args():
    a; b; c
    if True: raise RuntimeError
    if False: ...
    for i in range(10):
    print(i)
    continue
```

because our formatter does not yet support `for` statements and just inserts the text from the source. 

## Downsides

Using an identifier will not work in all situations. For example, an identifier is invalid in an `Arguments ` position. That's why I kept `verbatim_text` around and e.g. use it in the `Arguments` formatting logic where incorrect indentations are impossible (to my knowledge). Meaning, `verbatim_text` we can opt in to `verbatim_text` when we want to iterate quickly on nodes that we don't want to provide a full implementation yet and using an identifier would be invalid. 

## Upsides

Running this on main discovered stability issues with the newline handling that were previously "hidden" because of the verbatim formatting. I guess that's an upside :)

## Test Plan

None?
  • Loading branch information
MichaReiser authored Jun 7, 2023
1 parent 2f125f4 commit bcf745c
Show file tree
Hide file tree
Showing 134 changed files with 5,283 additions and 4,360 deletions.
30 changes: 15 additions & 15 deletions crates/ruff_python_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,16 @@ no_leading_newline = 30

assert_eq!(
&printed,
r#"a = 10
r#"NOT_YET_IMPLEMENTED_StmtAssign
three_leading_newlines = 80
NOT_YET_IMPLEMENTED_StmtAssign
two_leading_newlines = 20
NOT_YET_IMPLEMENTED_StmtAssign
one_leading_newline = 10
no_leading_newline = 30"#
NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign"#
);
}

Expand All @@ -211,14 +211,14 @@ no_leading_newline = 30"#

assert_eq!(
&printed,
r#"a = 10
r#"NOT_YET_IMPLEMENTED_StmtAssign
three_leading_newlines = 80
NOT_YET_IMPLEMENTED_StmtAssign
two_leading_newlines = 20
NOT_YET_IMPLEMENTED_StmtAssign
one_leading_newline = 10
no_leading_newline = 30"#
NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign"#
);
}

Expand All @@ -229,11 +229,11 @@ no_leading_newline = 30"#

assert_eq!(
&printed,
r#"a = 10
three_leading_newlines = 80
two_leading_newlines = 20
one_leading_newline = 10
no_leading_newline = 30"#
r#"NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign
NOT_YET_IMPLEMENTED_StmtAssign"#
);
}
}
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_attribute.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprAttribute;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprAttribute;

impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
fn fmt_fields(&self, item: &ExprAttribute, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_await.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprAwait;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprAwait;

impl FormatNodeRule<ExprAwait> for FormatExprAwait {
fn fmt_fields(&self, item: &ExprAwait, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_bool_op.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprBoolOp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprBoolOp;

impl FormatNodeRule<ExprBoolOp> for FormatExprBoolOp {
fn fmt_fields(&self, item: &ExprBoolOp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_call.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprCall;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprCall;

impl FormatNodeRule<ExprCall> for FormatExprCall {
fn fmt_fields(&self, item: &ExprCall, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_compare.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprCompare;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprCompare;

impl FormatNodeRule<ExprCompare> for FormatExprCompare {
fn fmt_fields(&self, item: &ExprCompare, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_constant.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprConstant;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprConstant;

impl FormatNodeRule<ExprConstant> for FormatExprConstant {
fn fmt_fields(&self, item: &ExprConstant, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_dict.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprDict;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprDict;

impl FormatNodeRule<ExprDict> for FormatExprDict {
fn fmt_fields(&self, item: &ExprDict, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_dict_comp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprDictComp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprDictComp;

impl FormatNodeRule<ExprDictComp> for FormatExprDictComp {
fn fmt_fields(&self, item: &ExprDictComp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprFormattedValue;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprFormattedValue;

impl FormatNodeRule<ExprFormattedValue> for FormatExprFormattedValue {
fn fmt_fields(&self, item: &ExprFormattedValue, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprGeneratorExp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprGeneratorExp;

impl FormatNodeRule<ExprGeneratorExp> for FormatExprGeneratorExp {
fn fmt_fields(&self, item: &ExprGeneratorExp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_if_exp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprIfExp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprIfExp;

impl FormatNodeRule<ExprIfExp> for FormatExprIfExp {
fn fmt_fields(&self, item: &ExprIfExp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprJoinedStr;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprJoinedStr;

impl FormatNodeRule<ExprJoinedStr> for FormatExprJoinedStr {
fn fmt_fields(&self, item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_lambda.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprLambda;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprLambda;

impl FormatNodeRule<ExprLambda> for FormatExprLambda {
fn fmt_fields(&self, item: &ExprLambda, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_list_comp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprListComp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprListComp;

impl FormatNodeRule<ExprListComp> for FormatExprListComp {
fn fmt_fields(&self, item: &ExprListComp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprNamedExpr;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprNamedExpr;

impl FormatNodeRule<ExprNamedExpr> for FormatExprNamedExpr {
fn fmt_fields(&self, item: &ExprNamedExpr, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_set.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprSet;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprSet;

impl FormatNodeRule<ExprSet> for FormatExprSet {
fn fmt_fields(&self, item: &ExprSet, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_set_comp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprSetComp;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprSetComp;

impl FormatNodeRule<ExprSetComp> for FormatExprSetComp {
fn fmt_fields(&self, item: &ExprSetComp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_slice.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{verbatim_text, FormatNodeRule, PyFormatter};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprSlice;

Expand All @@ -10,7 +10,7 @@ pub struct FormatExprSlice;

impl FormatNodeRule<ExprSlice> for FormatExprSlice {
fn fmt_fields(&self, item: &ExprSlice, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [verbatim_text(item.range)])
write!(f, [not_yet_implemented(item)])
}
}

Expand Down
Loading

0 comments on commit bcf745c

Please sign in to comment.