Add tests for skip magic trailing comma

<!--
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 adds tests that verify that the magic trailing comma is not respected if disabled in the formatter options. 

Our test setup now allows to create a `<fixture-name>.options.json` file that contains an array of configurations that should be tested. 

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

It's all about tests :) 

<!-- How was it tested? -->
This commit is contained in:
Micha Reiser 2023-06-26 14:15:55 +02:00 committed by GitHub
parent dd0d1afb66
commit f18a1f70de
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 268 additions and 79 deletions

3
Cargo.lock generated
View file

@ -2065,8 +2065,9 @@ dependencies = [
"ruff_text_size",
"rustc-hash",
"rustpython-parser",
"serde",
"serde_json",
"similar",
"test-case",
]
[[package]]

View file

@ -59,10 +59,8 @@ use std::num::ParseIntError;
use std::str::FromStr;
#[derive(Debug, Eq, PartialEq, Clone, Copy, Hash)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[derive(Default)]
pub enum IndentStyle {
/// Tab
@ -112,10 +110,8 @@ impl std::fmt::Display for IndentStyle {
///
/// The allowed range of values is 1..=320
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct LineWidth(u16);
impl LineWidth {
@ -278,10 +274,8 @@ impl FormatOptions for SimpleFormatOptions {
/// Lightweight sourcemap marker between source and output tokens
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct SourceMarker {
/// Position of the marker in the original source
pub source: TextSize,
@ -340,10 +334,8 @@ where
pub type PrintResult<T> = Result<T, PrintError>;
#[derive(Debug, Clone, Eq, PartialEq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct Printed {
code: String,
range: Option<TextRange>,

View file

@ -25,8 +25,23 @@ itertools = { workspace = true }
once_cell = { workspace = true }
rustc-hash = { workspace = true }
rustpython-parser = { workspace = true }
serde = { workspace = true, optional = true }
[dev-dependencies]
ruff_formatter = { path = "../ruff_formatter", features = ["serde"]}
insta = { workspace = true, features = ["glob"] }
test-case = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
similar = { workspace = true }
[[test]]
name = "ruff_python_formatter_fixtures"
path = "tests/fixtures.rs"
test = true
required-features = [ "serde" ]
[features]
serde = ["dep:serde", "ruff_formatter/serde"]
default = ["serde"]

View file

@ -0,0 +1,8 @@
[
{
"magic_trailing_comma": "respect"
},
{
"magic_trailing_comma": "ignore"
}
]

View file

@ -0,0 +1,22 @@
(
"First entry",
"Second entry",
"last with trailing comma",
)
(
"First entry",
"Second entry",
"last without trailing comma"
)
(
"First entry",
"Second entry",
"third entry",
"fourth entry",
"fifth entry",
"sixt entry",
"seventh entry",
"eigth entry",
)

View file

@ -220,7 +220,7 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
if let Some(last_end) = self.last_end.take() {
if_group_breaks(&text(",")).fmt(self.fmt)?;
if self.fmt.options().magic_trailing_comma().is_preserve()
if self.fmt.options().magic_trailing_comma().is_respect()
&& matches!(
first_non_trivia_token(last_end, self.fmt.context().contents()),
Some(Token {

View file

@ -2,6 +2,11 @@ use ruff_formatter::printer::{LineEnding, PrinterOptions};
use ruff_formatter::{FormatOptions, IndentStyle, LineWidth};
#[derive(Clone, Debug)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize),
serde(default)
)]
pub struct PyFormatOptions {
/// Specifies the indent style:
/// * Either a tab
@ -9,6 +14,7 @@ pub struct PyFormatOptions {
indent_style: IndentStyle,
/// The preferred line width at which the formatter should wrap lines.
#[cfg_attr(feature = "serde", serde(default = "default_line_width"))]
line_width: LineWidth,
/// The preferred quote style to use (single vs double quotes).
@ -18,6 +24,10 @@ pub struct PyFormatOptions {
magic_trailing_comma: MagicTrailingComma,
}
fn default_line_width() -> LineWidth {
LineWidth::try_from(88).unwrap()
}
impl PyFormatOptions {
pub fn magic_trailing_comma(&self) -> MagicTrailingComma {
self.magic_trailing_comma
@ -36,6 +46,16 @@ impl PyFormatOptions {
self.magic_trailing_comma = trailing_comma;
self
}
pub fn with_indent_style(&mut self, indent_style: IndentStyle) -> &mut Self {
self.indent_style = indent_style;
self
}
pub fn with_line_width(&mut self, line_width: LineWidth) -> &mut Self {
self.line_width = line_width;
self
}
}
impl FormatOptions for PyFormatOptions {
@ -69,6 +89,11 @@ impl Default for PyFormatOptions {
}
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize),
serde(rename_all = "kebab-case")
)]
pub enum QuoteStyle {
Single,
#[default]
@ -105,14 +130,19 @@ impl TryFrom<char> for QuoteStyle {
}
#[derive(Copy, Clone, Debug, Default)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize),
serde(rename_all = "kebab-case")
)]
pub enum MagicTrailingComma {
#[default]
Preserve,
Skip,
Respect,
Ignore,
}
impl MagicTrailingComma {
pub const fn is_preserve(self) -> bool {
matches!(self, Self::Preserve)
pub const fn is_respect(self) -> bool {
matches!(self, Self::Respect)
}
}

View file

@ -1,16 +1,18 @@
use ruff_formatter::FormatOptions;
use ruff_python_formatter::{format_module, PyFormatOptions};
use similar::TextDiff;
use std::fmt::{Formatter, Write};
use std::fs;
use std::io::BufReader;
use std::path::Path;
use std::{fmt, fs};
#[test]
fn black_compatibility() {
let test_file = |input_path: &Path| {
let content = fs::read_to_string(input_path).unwrap();
let printed =
format_module(&content, PyFormatOptions::default()).expect("Formatting to succeed");
let options = PyFormatOptions::default();
let printed = format_module(&content, options.clone()).expect("Formatting to succeed");
let expected_path = input_path.with_extension("py.expect");
let expected_output = fs::read_to_string(&expected_path)
@ -18,7 +20,7 @@ fn black_compatibility() {
let formatted_code = printed.as_code();
ensure_stability_when_formatting_twice(formatted_code);
ensure_stability_when_formatting_twice(formatted_code, options);
if formatted_code == expected_output {
// Black and Ruff formatting matches. Delete any existing snapshot files because the Black output
@ -66,7 +68,7 @@ fn black_compatibility() {
write!(snapshot, "{}", CodeFrame::new("diff", &diff)).unwrap();
write!(snapshot, "{}", Header::new("Ruff Output")).unwrap();
write!(snapshot, "{}", CodeFrame::new("py", formatted_code)).unwrap();
write!(snapshot, "{}", CodeFrame::new("py", &formatted_code)).unwrap();
write!(snapshot, "{}", Header::new("Black Output")).unwrap();
write!(snapshot, "{}", CodeFrame::new("py", &expected_output)).unwrap();
@ -89,21 +91,52 @@ fn format() {
let test_file = |input_path: &Path| {
let content = fs::read_to_string(input_path).unwrap();
let printed =
format_module(&content, PyFormatOptions::default()).expect("Formatting to succeed");
let options = PyFormatOptions::default();
let printed = format_module(&content, options.clone()).expect("Formatting to succeed");
let formatted_code = printed.as_code();
ensure_stability_when_formatting_twice(formatted_code);
ensure_stability_when_formatting_twice(formatted_code, options);
let snapshot = format!(
r#"## Input
{}
let mut snapshot = format!("## Input\n{}", CodeFrame::new("py", &content));
## Output
{}"#,
CodeFrame::new("py", &content),
CodeFrame::new("py", formatted_code)
);
let options_path = input_path.with_extension("options.json");
if let Ok(options_file) = fs::File::open(options_path) {
let reader = BufReader::new(options_file);
let options: Vec<PyFormatOptions> =
serde_json::from_reader(reader).expect("Options to be a valid Json file");
writeln!(snapshot, "## Outputs").unwrap();
for (i, options) in options.into_iter().enumerate() {
let printed =
format_module(&content, options.clone()).expect("Formatting to succeed");
let formatted_code = printed.as_code();
ensure_stability_when_formatting_twice(formatted_code, options.clone());
writeln!(
snapshot,
"### Output {}\n{}{}",
i + 1,
CodeFrame::new("", &DisplayPyOptions(&options)),
CodeFrame::new("py", &formatted_code)
)
.unwrap();
}
} else {
let options = PyFormatOptions::default();
let printed = format_module(&content, options.clone()).expect("Formatting to succeed");
let formatted_code = printed.as_code();
ensure_stability_when_formatting_twice(formatted_code, options);
writeln!(
snapshot,
"## Output\n{}",
CodeFrame::new("py", &formatted_code)
)
.unwrap();
}
insta::with_settings!({
omit_expression => true,
@ -118,8 +151,8 @@ fn format() {
}
/// Format another time and make sure that there are no changes anymore
fn ensure_stability_when_formatting_twice(formatted_code: &str) {
let reformatted = match format_module(formatted_code, PyFormatOptions::default()) {
fn ensure_stability_when_formatting_twice(formatted_code: &str, options: PyFormatOptions) {
let reformatted = match format_module(formatted_code, options) {
Ok(reformatted) => reformatted,
Err(err) => {
panic!(
@ -170,11 +203,11 @@ impl std::fmt::Display for Header<'_> {
struct CodeFrame<'a> {
language: &'a str,
code: &'a str,
code: &'a dyn std::fmt::Display,
}
impl<'a> CodeFrame<'a> {
fn new(language: &'a str, code: &'a str) -> Self {
fn new(language: &'a str, code: &'a dyn std::fmt::Display) -> Self {
Self { language, code }
}
}
@ -187,3 +220,21 @@ impl std::fmt::Display for CodeFrame<'_> {
writeln!(f)
}
}
struct DisplayPyOptions<'a>(&'a PyFormatOptions);
impl fmt::Display for DisplayPyOptions<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
writeln!(
f,
r#"indent-style = {indent_style}
line-width = {line_width}
quote-style = {quote_style:?}
magic-trailing-comma = {magic_trailing_comma:?}"#,
indent_style = self.0.indent_style(),
line_width = self.0.line_width().value(),
quote_style = self.0.quote_style(),
magic_trailing_comma = self.0.magic_trailing_comma()
)
}
}

View file

@ -109,8 +109,6 @@ x53 = (
```
## Output
```py
NOT_YET_IMPLEMENTED_StmtImportFrom
@ -196,3 +194,4 @@ x53 = a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlas
```

View file

@ -200,8 +200,6 @@ for user_id in set(target_user_ids) - {u.user_id for u in updates}:
updates.append(UserPresenceState.default(user_id))
```
## Output
```py
(
@ -447,3 +445,4 @@ for (
```

View file

@ -70,8 +70,6 @@ if (
pass
```
## Output
```py
if (
@ -141,3 +139,4 @@ if (
```

View file

@ -67,8 +67,6 @@ return 1 == 2 and (
]
```
## Output
```py
a == b
@ -187,3 +185,4 @@ return (
```

View file

@ -64,8 +64,6 @@ a = {
}
```
## Output
```py
# before
@ -131,3 +129,4 @@ a = {
```

View file

@ -16,8 +16,6 @@ a3 = [
]
```
## Output
```py
# Dangling comment placement in empty lists
@ -33,3 +31,4 @@ a3 = [
```

View file

@ -88,8 +88,6 @@ e202 = "e"[a() :: a()]
e210 = "e"[a() : 1 :]
```
## Output
```py
# Handle comments both when lower and upper exist and when they don't
@ -175,3 +173,4 @@ e210 = "e"[NOT_IMPLEMENTED_call() : 1 :]
```

View file

@ -58,8 +58,6 @@ String \"\"\"
'''
```
## Output
```py
"' test"
@ -117,3 +115,4 @@ String \"\"\"
```

View file

@ -65,8 +65,6 @@ h2 = ((((1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurp
h3 = 1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurpqurpqurpqurpqurpqurüqurqpuriq"
```
## Output
```py
# Non-wrapping parentheses checks
@ -264,3 +262,4 @@ h3 = (
```

View file

@ -144,8 +144,6 @@ if not \
pass
```
## Output
```py
if (
@ -300,3 +298,4 @@ if not a:
```

View file

@ -0,0 +1,88 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/skip_magic_trailing_comma.py
---
## Input
```py
(
"First entry",
"Second entry",
"last with trailing comma",
)
(
"First entry",
"Second entry",
"last without trailing comma"
)
(
"First entry",
"Second entry",
"third entry",
"fourth entry",
"fifth entry",
"sixt entry",
"seventh entry",
"eigth entry",
)
```
## Outputs
### Output 1
```
indent-style = Spaces, size: 4
line-width = 88
quote-style = Double
magic-trailing-comma = Respect
```
```py
(
"First entry",
"Second entry",
"last with trailing comma",
)
("First entry", "Second entry", "last without trailing comma")
(
"First entry",
"Second entry",
"third entry",
"fourth entry",
"fifth entry",
"sixt entry",
"seventh entry",
"eigth entry",
)
```
### Output 2
```
indent-style = Spaces, size: 4
line-width = 88
quote-style = Double
magic-trailing-comma = Ignore
```
```py
("First entry", "Second entry", "last with trailing comma")
("First entry", "Second entry", "last without trailing comma")
(
"First entry",
"Second entry",
"third entry",
"fourth entry",
"fifth entry",
"sixt entry",
"seventh entry",
"eigth entry",
)
```

View file

@ -16,8 +16,6 @@ a2 = (
a = asdf = fjhalsdljfalflaflapamsakjsdhflakjdslfjhalsdljfalflaflapamsakjsdhflakjdslfjhalsdljfal = 1
```
## Output
```py
# break left hand side
@ -31,3 +29,4 @@ a = asdf = fjhalsdljfalflaflapamsakjsdhflakjdslfjhalsdljfalflaflapamsakjsdhflakj
```

View file

@ -11,8 +11,6 @@ while True: # block comment
# post comment
```
## Output
```py
# leading comment
@ -23,3 +21,4 @@ while True: # block comment
```

View file

@ -42,8 +42,6 @@ class Test(Aaaa): # trailing comment
pass
```
## Output
```py
class Test(
@ -95,3 +93,4 @@ class Test(Aaaa): # trailing comment
```

View file

@ -40,8 +40,6 @@ for x in (): # type: int
...
```
## Output
```py
for x in y: # trailing test comment
@ -85,3 +83,4 @@ for x in (): # type: int
```

View file

@ -239,8 +239,6 @@ def f42(
pass
```
## Output
```py
# Dangling comments
@ -516,3 +514,4 @@ def f42(
```

View file

@ -79,8 +79,6 @@ else: # Comment
pass
```
## Output
```py
if x == y: # trailing if condition
@ -158,3 +156,4 @@ else: # Comment
```

View file

@ -36,8 +36,6 @@ while (
print("Do something")
```
## Output
```py
while 34: # trailing test comment
@ -74,3 +72,4 @@ while (
```

View file

@ -38,8 +38,6 @@ while b == 20:
e = 50 # one empty line before
```
## Output
```py
# Removes the line above
@ -79,3 +77,4 @@ e = 50 # one empty line before
```