Preserve trailing semicolon for Notebooks

[dhruvmanila]

Summary

This PR updates the formatter to preserve trailing semicolon for Jupyter Notebooks.

The motivation behind the change is that semicolons in notebooks are typically used to hide the output, for example when plotting. This is highlighted in the linked issue.

The conditions required as to when the trailing semicolon should be preserved are:

1. It should be a top-level statement which is last in the module.
2. For statement, it can be either assignment, annotated assignment, or augmented assignment. Here, the target should only be a single identifier i.e., multiple assignments or tuple unpacking isn't considered.
3. For expression, it can be any.

Test Plan

Add a new integration test in ruff_cli. The test notebook basically acts as a document as to which trailing semicolons are to be preserved.

fixes: #8254

[dhruvmanila]

Current dependencies on/for this PR:

• main
  • PR Preserve trailing semicolon for Notebooks #8590 👈

This stack of pull requests is managed by Graphite.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[dhruvmanila]

I'm not sure if this is the correct place for this test case. We would need to refactor the formatter test suite to include notebook files.

[dhruvmanila]

We could avoid cloning the iterator by making it peekable but that involves updating the function signature wherever this iterator is being passed. This is because it expects std::slice::Iter.

[konstin]

This clone is effectively free (we just clone the slice reference), so this is fine. The peakable things with the function signatures is annoying though, i had also already hit that

[konstin]

Would it simplify the implementation to insert the semicolon at

ruff/crates/ruff_python_formatter/src/statement/suite.rs

Line 399 in 3076d76

following.format().fmt(f)?;

instead? Then we shouldn't need TopLevelStatementPosition

[konstin]

This clone is effectively free (we just clone the slice reference), so this is fine. The peakable things with the function signatures is annoying though, i had also already hit that

[dhruvmanila]

Would it simplify the implementation to insert the semicolon at

ruff/crates/ruff_python_formatter/src/statement/suite.rs

Line 399 in 3076d76

following.format().fmt(f)?;

instead? Then we shouldn't need TopLevelStatementPosition

I'm assuming what you're recommending is to check at the linked position if this is the last statement and then preserve the semicolon, if any. Is this correct?

I think that'll involve matching against the relevant nodes where the semicolon needs to be preserved as it's only required for a subset of nodes - StmtAssign, StmtAugAssign, StmtAnnAssign, and StmtExpr. Additionally, certain conditions needs to be checked like is this a multiple assignment or not? I'd prefer to keep the logic in the respective node.

[MichaReiser]

I prefer to avoid using the context whenever possible because it is harder to reason about where the context value is set/changed.

An alternative to the context would have been to parameterized FormatStmt, FormatStmtAssign etc (by implementing FormatWithOptions) and explicitly pass a value whether it is the last statement.

[MichaReiser]

I think this can be simplified to

SuiteKind::TopLevel => NodeLevel::TopLevel(if statements.len() < 2 {
    TopLevelStatementPosition::Last
} else {
    TopLevelStatementPosition::Other
}),

[dhruvmanila]

Yes, that can be done.

[dhruvmanila]

An alternative to the context would have been to parameterized FormatStmt, FormatStmtAssign etc (by implementing FormatWithOptions) and explicitly pass a value whether it is the last statement.

I thought about this but I felt like this kind of information is suited well for the context. And, it already provided the node level information.