diff --git a/crates/pgls_pretty_print/src/nodes/a_expr.rs b/crates/pgls_pretty_print/src/nodes/a_expr.rs index 0fffb7817..1797c7519 100644 --- a/crates/pgls_pretty_print/src/nodes/a_expr.rs +++ b/crates/pgls_pretty_print/src/nodes/a_expr.rs @@ -223,7 +223,8 @@ fn emit_aexpr_distinct(e: &mut EventEmitter, n: &AExpr) { e.token(TokenKind::DISTINCT_KW); e.space(); e.token(TokenKind::FROM_KW); - e.space(); + // Like a comparison operator, the right-hand side wraps when the expression doesn't fit. + e.line(LineType::SoftOrSpace); if let Some(ref rexpr) = n.rexpr { emit_operand_with_parens(e, rexpr, parent_info, OperandSide::Right); @@ -250,7 +251,8 @@ fn emit_aexpr_not_distinct(e: &mut EventEmitter, n: &AExpr) { e.token(TokenKind::DISTINCT_KW); e.space(); e.token(TokenKind::FROM_KW); - e.space(); + // Like a comparison operator, the right-hand side wraps when the expression doesn't fit. + e.line(LineType::SoftOrSpace); if let Some(ref rexpr) = n.rexpr { emit_operand_with_parens(e, rexpr, parent_info, OperandSide::Right); @@ -341,7 +343,8 @@ fn emit_aexpr_like(e: &mut EventEmitter, n: &AExpr) { } e.token(TokenKind::LIKE_KW); - e.space(); + // Like a comparison operator, the pattern wraps when the expression doesn't fit. + e.line(LineType::SoftOrSpace); if let Some(ref rexpr) = n.rexpr { super::emit_node(rexpr, e); @@ -365,7 +368,8 @@ fn emit_aexpr_ilike(e: &mut EventEmitter, n: &AExpr) { } e.token(TokenKind::ILIKE_KW); - e.space(); + // Like a comparison operator, the pattern wraps when the expression doesn't fit. + e.line(LineType::SoftOrSpace); if let Some(ref rexpr) = n.rexpr { super::emit_node(rexpr, e); @@ -391,7 +395,8 @@ fn emit_aexpr_similar(e: &mut EventEmitter, n: &AExpr) { e.token(TokenKind::SIMILAR_KW); e.space(); e.token(TokenKind::TO_KW); - e.space(); + // Like a comparison operator, the pattern wraps when the expression doesn't fit. + e.line(LineType::SoftOrSpace); // PostgreSQL stores SIMILAR TO as a call to similar_to_escape(pattern) or similar_to_escape(pattern, escape) // We need to emit the original pattern (and ESCAPE clause if present) diff --git a/crates/pgls_pretty_print/src/nodes/func_call.rs b/crates/pgls_pretty_print/src/nodes/func_call.rs index 85919d7fa..f8233d150 100644 --- a/crates/pgls_pretty_print/src/nodes/func_call.rs +++ b/crates/pgls_pretty_print/src/nodes/func_call.rs @@ -337,16 +337,21 @@ fn emit_sql_syntax_call(e: &mut EventEmitter, n: &FuncCall) -> bool { ("extract", [field, source]) => { e.token(TokenKind::EXTRACT_KW); super::open_call_arguments(e); + // The arguments stay together on the indented line when they fit there. + e.group_start(GroupKind::List); emit_extract_field(e, field); e.space(); e.token(TokenKind::FROM_KW); - e.space(); + e.line(LineType::SoftOrSpace); super::emit_node(source, e); + e.group_end(); super::close_call_arguments(e); } ("overlay", [string, replacement, start, rest @ ..]) if rest.len() <= 1 => { e.token(TokenKind::OVERLAY_KW); super::open_call_arguments(e); + // The arguments stay together on the indented line when they fit there. + e.group_start(GroupKind::List); super::emit_node(string, e); e.line(LineType::SoftOrSpace); e.token(TokenKind::PLACING_KW); @@ -362,6 +367,7 @@ fn emit_sql_syntax_call(e: &mut EventEmitter, n: &FuncCall) -> bool { e.space(); super::emit_node(length, e); } + e.group_end(); super::close_call_arguments(e); } // Stored as position(string, substring), written POSITION(substring IN string). Both @@ -369,16 +375,21 @@ fn emit_sql_syntax_call(e: &mut EventEmitter, n: &FuncCall) -> bool { ("position", [string, substring]) => { e.token(TokenKind::POSITION_KW); super::open_call_arguments(e); + // The arguments stay together on the indented line when they fit there. + e.group_start(GroupKind::List); emit_parenthesized_unless_simple(e, substring); e.line(LineType::SoftOrSpace); e.token(TokenKind::IN_KW); e.space(); emit_parenthesized_unless_simple(e, string); + e.group_end(); super::close_call_arguments(e); } ("substring", [string, start, rest @ ..]) if rest.len() <= 1 => { e.token(TokenKind::SUBSTRING_KW); super::open_call_arguments(e); + // The arguments stay together on the indented line when they fit there. + e.group_start(GroupKind::List); super::emit_node(string, e); e.line(LineType::SoftOrSpace); e.token(TokenKind::FROM_KW); @@ -390,6 +401,7 @@ fn emit_sql_syntax_call(e: &mut EventEmitter, n: &FuncCall) -> bool { e.space(); super::emit_node(length, e); } + e.group_end(); super::close_call_arguments(e); } // Stored as btrim(string[, characters]), written TRIM(BOTH characters FROM string). @@ -491,11 +503,14 @@ fn emit_sql_syntax_call(e: &mut EventEmitter, n: &FuncCall) -> bool { ("xmlexists", [xpath, document]) => { e.token(TokenKind::XMLEXISTS_KW); super::open_call_arguments(e); + // The arguments stay together on the indented line when they fit there. + e.group_start(GroupKind::List); super::emit_node(xpath, e); e.line(LineType::SoftOrSpace); e.token(TokenKind::PASSING_KW); e.space(); emit_parenthesized_unless_simple(e, document); + e.group_end(); super::close_call_arguments(e); } ("system_user", []) => e.token(TokenKind::SYSTEM_USER_KW), diff --git a/crates/pgls_pretty_print/src/renderer.rs b/crates/pgls_pretty_print/src/renderer.rs index 551459905..728923625 100644 --- a/crates/pgls_pretty_print/src/renderer.rs +++ b/crates/pgls_pretty_print/src/renderer.rs @@ -289,7 +289,8 @@ impl Renderer { /// The group on one line, if it can be collapsed and fits on the current line followed by /// `trailing_width` columns that cannot move to the next line. Only the first line of a text - /// spanning several lines, a function body for instance, sits on the current one. + /// spanning several lines, a function body for instance, sits on the current one, and what + /// follows the text has to fit on its last line. /// /// Measuring stops as soon as the line is full, so a large group that cannot fit is not /// rendered in full for every level it is nested in. @@ -298,13 +299,15 @@ impl Renderer { group_events: &[LayoutEvent], trailing_width: usize, ) -> Option { - let available = self + let mut available = self .config .max_line_length .saturating_sub(self.effective_line_length()); let mut buffer = String::new(); - // Set once the text reaches a line break, after which the width no longer matters. - let mut first_line_fits = false; + // Where the line being measured starts in `buffer`. After a text spanning several lines, + // that is its last line, which starts at the left margin; the lines in between are the + // text's own. + let mut line_start = 0; for event in group_events { match event { @@ -335,15 +338,15 @@ impl Renderer { | LayoutEvent::AlignEnd => {} } - if !first_line_fits { - match buffer.split_once('\n') { - Some((first_line, _)) if first_line.len() <= available => { - first_line_fits = true; - } - Some(_) => return None, - None if buffer.len() + trailing_width > available => return None, - None => {} + if let Some(last_break) = buffer[line_start..].rfind('\n') { + if line_start == 0 && buffer.find('\n').is_some_and(|first| first > available) { + return None; } + line_start += last_break + 1; + available = self.config.max_line_length; + } + if buffer.len() - line_start + trailing_width > available { + return None; } } diff --git a/crates/pgls_pretty_print/tests/common/mod.rs b/crates/pgls_pretty_print/tests/common/mod.rs index a635a77dd..d7e9cd4e5 100644 --- a/crates/pgls_pretty_print/tests/common/mod.rs +++ b/crates/pgls_pretty_print/tests/common/mod.rs @@ -1,3 +1,6 @@ +// Every test crate includes this module and uses only some of it. +#![allow(dead_code)] + use pgls_query::protobuf::Token; /// Checks the whitespace the formatter wrote between tokens: no space before a comma or `)`, none @@ -85,7 +88,6 @@ pub fn whitespace_problems(output: &str) -> Vec { problems } -#[allow(dead_code)] // Not every test crate asserts directly. pub fn assert_clean_whitespace(output: &str) { let problems = whitespace_problems(output); assert!( @@ -99,7 +101,6 @@ pub fn assert_clean_whitespace(output: &str) { /// spaces, a space before a tab when indenting with tabs. Alignment after the tabs is written as /// spaces, so spaces after tabs are fine. Lines inside a multi-line token, such as a function body, /// keep the indentation of the source. -#[allow(dead_code)] // Not every test crate checks indentation. pub fn mixed_indentation(output: &str, indent_with_tabs: bool) -> Vec { let token_ranges = pgls_query::scan(output) .map(|scan| { @@ -125,3 +126,72 @@ pub fn mixed_indentation(output: &str, indent_with_tabs: bool) -> Vec { } problems } + +/// Columns a line may need besides its indentation and its longest token before the formatter is +/// expected to break it: a keyword or punctuation around an unbreakable token, `passing '...'` or +/// `as "..."` for instance. +const WIDTH_OVERHEAD: usize = 12; + +/// Lines longer than `max_line_length`, counting a tab as `tab_width` columns. +/// +/// A line is excused when its indentation and its longest token leave no room to break it: a long +/// string literal, quoted identifier or function body, or code nested so deeply that little room is +/// left. String literals count like any other token, so a call that merely has a literal among its +/// arguments still has to break. A `--` comment ending the line is written as in the source and is +/// not counted. +pub fn width_problems(output: &str, max_line_length: usize, tab_width: usize) -> Vec { + let tokens: Vec<_> = pgls_query::scan(output) + .map(|scan| { + scan.tokens + .iter() + .map(|token| { + ( + token.start as usize..token.end as usize, + Token::try_from(token.token) == Ok(Token::SqlComment), + ) + }) + .collect() + }) + .unwrap_or_default(); + let columns = |text: &str| -> usize { + text.chars() + .map(|c| if c == '\t' { tab_width } else { 1 }) + .sum() + }; + + let mut problems = Vec::new(); + let mut line_start = 0; + for (index, line) in output.split('\n').enumerate() { + let line_end = line_start + line.len(); + let code_end = tokens + .iter() + .find(|(range, line_comment)| { + *line_comment && range.start >= line_start && range.start < line_end + }) + .map_or(line_end, |(range, _)| range.start); + let code = output[line_start..code_end].trim_end(); + let width = columns(code); + if width > max_line_length { + let indent = columns(&code[..code.len() - code.trim_start().len()]); + let longest_token = tokens + .iter() + .map(|(range, _)| { + let start = range.start.max(line_start); + let end = range.end.min(code_end); + output + .get(start..end) + .map_or(0, |piece| piece.chars().count()) + }) + .max() + .unwrap_or(0); + if indent + longest_token + WIDTH_OVERHEAD <= max_line_length { + problems.push(format!( + "line {} has {width} columns, more than {max_line_length}: `{line}`", + index + 1 + )); + } + } + line_start = line_end + 1; + } + problems +} diff --git a/crates/pgls_pretty_print/tests/option_matrix.rs b/crates/pgls_pretty_print/tests/option_matrix.rs index 06ab8b82a..af32d37a1 100644 --- a/crates/pgls_pretty_print/tests/option_matrix.rs +++ b/crates/pgls_pretty_print/tests/option_matrix.rs @@ -174,6 +174,15 @@ fn check(sql: &str, config: &FormatConfig) -> Result<(), String> { return Ok(()); }; let mut problems = common::whitespace_problems(&output); + // At width 40 some statements can't fit. A `::` cast doesn't break either, so in a compact + // `select '...'::type` or `('...'::jsonb)['a']['b']` nothing can move to the next line. + if config.line_width >= 80 && matches!(config.cast_style, CastStyle::Cast) { + problems.extend(common::width_problems( + &output, + config.line_width, + config.indent_size, + )); + } problems.extend(common::mixed_indentation( &output, matches!(config.indent_style, IndentStyle::Tabs), diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_100.snap index 215b92b23..a6e6fba5f 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_100.snap @@ -146,8 +146,15 @@ set min_parallel_table_scan_size = 0; set max_parallel_workers_per_gather = 4; -select jsonb_pretty(explain_filter_to_json('explain (analyze, verbose, buffers, format json) - select * from tenk1 order by tenthous') #- '{0,Plan,Plans,0,Plans,0,Workers}' #- '{0,Plan,Plans,0,Workers}' #- '{0,Plan,Plans,0,Sort Method}' #- '{0,Plan,Plans,0,Sort Space Type}'); +select + jsonb_pretty( + explain_filter_to_json('explain (analyze, verbose, buffers, format json) + select * from tenk1 order by tenthous') #- + '{0,Plan,Plans,0,Plans,0,Workers}' #- + '{0,Plan,Plans,0,Workers}' #- + '{0,Plan,Plans,0,Sort Method}' #- + '{0,Plan,Plans,0,Sort Space Type}' + ); rollback; diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_80.snap index ce397bf07..5af93a095 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__explain_80.snap @@ -185,7 +185,11 @@ set max_parallel_workers_per_gather = 4; select jsonb_pretty( explain_filter_to_json('explain (analyze, verbose, buffers, format json) - select * from tenk1 order by tenthous') #- '{0,Plan,Plans,0,Plans,0,Workers}' #- '{0,Plan,Plans,0,Workers}' #- '{0,Plan,Plans,0,Sort Method}' #- '{0,Plan,Plans,0,Sort Space Type}' + select * from tenk1 order by tenthous') #- + '{0,Plan,Plans,0,Plans,0,Workers}' #- + '{0,Plan,Plans,0,Workers}' #- + '{0,Plan,Plans,0,Sort Method}' #- + '{0,Plan,Plans,0,Sort Space Type}' ); rollback; diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__jsonb_jsonpath_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__jsonb_jsonpath_100.snap index 8e941d0ed..5ca42671e 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__jsonb_jsonpath_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__jsonb_jsonpath_100.snap @@ -323,7 +323,8 @@ select y, jsonb_path_query('[true, false, null]', '$[*] ? (@ == true && ($x == true && $y == true) || @ == false && !($x == true && $y == true) || - @ == null && ($x == true && $y == true) is unknown)', jsonb_build_object('x', x, 'y', y)) as "x && y" + @ == null && ($x == true && $y == true) is unknown)', jsonb_build_object('x', x, 'y', y)) + as "x && y" from (values (cast('true' as jsonb)), ('false'), ('"null"')) as x (x), (values (cast('true' as jsonb)), ('false'), ('"null"')) as y (y); @@ -333,7 +334,8 @@ select y, jsonb_path_query('[true, false, null]', '$[*] ? (@ == true && ($x == true || $y == true) || @ == false && !($x == true || $y == true) || - @ == null && ($x == true || $y == true) is unknown)', jsonb_build_object('x', x, 'y', y)) as "x || y" + @ == null && ($x == true || $y == true) is unknown)', jsonb_build_object('x', x, 'y', y)) + as "x || y" from (values (cast('true' as jsonb)), ('false'), ('"null"')) as x (x), (values (cast('true' as jsonb)), ('false'), ('"null"')) as y (y); diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_100.snap index 90a0db345..445570515 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_100.snap @@ -317,26 +317,75 @@ select * from tenk1 as a inner join my_gen_series(1, 1000) as g on a.unique1 = g select * from tenk1 as a inner join my_gen_series(1, 10) as g on a.unique1 = g; -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMP ''2024-02-01'', TIMESTAMP ''2024-03-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMP ''2024-02-01'', TIMESTAMP ''2024-03-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'', ''UTC'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'', ''UTC'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''7 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''7 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''-1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''-1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''-infinity'', TIMESTAMPTZ ''infinity'', INTERVAL ''1 day'') g(s);', false, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''-infinity'', TIMESTAMPTZ ''infinity'', INTERVAL ''1 day'') g(s);', + false, + true, + false, + true + ); select * @@ -360,8 +409,15 @@ SELECT * FROM generate_series(25.0, 1.0, -1.0) g(s);', true, true, false, true); select explain_mask_costs(' SELECT * FROM generate_series(25.0, 1.0, 1.0) g(s);', true, true, false, true); -select explain_mask_costs(' -SELECT * FROM generate_series(''-infinity''::NUMERIC, ''infinity''::NUMERIC, 1.0) g(s);', false, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(''-infinity''::NUMERIC, ''infinity''::NUMERIC, 1.0) g(s);', + false, + true, + false, + true + ); select explain_mask_costs(' SELECT * FROM generate_series(1.0, 25.0, ''NaN''::NUMERIC) g(s);', false, true, false, true); diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_80.snap index ef61018bd..a690fc8de 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__misc_functions_80.snap @@ -398,26 +398,75 @@ from select * from tenk1 as a inner join my_gen_series(1, 10) as g on a.unique1 = g; -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMP ''2024-02-01'', TIMESTAMP ''2024-03-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMP ''2024-02-01'', TIMESTAMP ''2024-03-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'', ''UTC'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''1 day'', ''UTC'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''7 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-02-01'', TIMESTAMPTZ ''2024-03-01'', INTERVAL ''7 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''-1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''-1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''1 day'') g(s);', true, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''2024-03-01'', TIMESTAMPTZ ''2024-02-01'', INTERVAL ''1 day'') g(s);', + true, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(TIMESTAMPTZ ''-infinity'', TIMESTAMPTZ ''infinity'', INTERVAL ''1 day'') g(s);', false, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(TIMESTAMPTZ ''-infinity'', TIMESTAMPTZ ''infinity'', INTERVAL ''1 day'') g(s);', + false, + true, + false, + true + ); select * @@ -441,11 +490,25 @@ SELECT * FROM generate_series(25.0, 1.0, -1.0) g(s);', true, true, false, true); select explain_mask_costs(' SELECT * FROM generate_series(25.0, 1.0, 1.0) g(s);', true, true, false, true); -select explain_mask_costs(' -SELECT * FROM generate_series(''-infinity''::NUMERIC, ''infinity''::NUMERIC, 1.0) g(s);', false, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(''-infinity''::NUMERIC, ''infinity''::NUMERIC, 1.0) g(s);', + false, + true, + false, + true + ); -select explain_mask_costs(' -SELECT * FROM generate_series(1.0, 25.0, ''NaN''::NUMERIC) g(s);', false, true, false, true); +select + explain_mask_costs( + ' +SELECT * FROM generate_series(1.0, 25.0, ''NaN''::NUMERIC) g(s);', + false, + true, + false, + true + ); select explain_mask_costs(' SELECT * FROM generate_series(25.0, 2.0, 0.0) g(s);', false, true, false, true); diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__opr_sanity_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__opr_sanity_80.snap index f0a6328ca..30eabede9 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__opr_sanity_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__opr_sanity_80.snap @@ -470,8 +470,8 @@ select from pg_proc where - (proargmodes is not null and - 'v' = any (proargmodes)) is distinct from provariadic <> 0; + (proargmodes is not null and 'v' = any (proargmodes)) is distinct from + provariadic <> 0; select p1.oid, diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__strings_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__strings_80.snap index bf5eddf08..2b76c76f7 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__strings_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__strings_80.snap @@ -614,17 +614,13 @@ select 'i_dio' like pg_catalog.like_escape('i$_d%o', '$') as "true"; select 'i_dio' not like pg_catalog.like_escape('i$_d%o', '$') as "false"; select - cast('a_c' as bytea) like pg_catalog.like_escape( - cast('a$__' as bytea), - cast('$' as bytea) - ) + cast('a_c' as bytea) like + pg_catalog.like_escape(cast('a$__' as bytea), cast('$' as bytea)) as "true"; select - cast('a_c' as bytea) not like pg_catalog.like_escape( - cast('a$__' as bytea), - cast('$' as bytea) - ) + cast('a_c' as bytea) not like + pg_catalog.like_escape(cast('a$__' as bytea), cast('$' as bytea)) as "false"; select 'maca' like pg_catalog.like_escape('m%aca', '%') as "true"; @@ -1198,9 +1194,7 @@ select btrim(cast('\000trim\000' as bytea), cast('' as bytea)); select encode( overlay( - cast('Th\000omas' as bytea) - placing cast('Th\001omas' as bytea) - from 2 + cast('Th\000omas' as bytea) placing cast('Th\001omas' as bytea) from 2 ), 'escape' ); @@ -1208,9 +1202,7 @@ select select encode( overlay( - cast('Th\000omas' as bytea) - placing cast('\002\003' as bytea) - from 8 + cast('Th\000omas' as bytea) placing cast('\002\003' as bytea) from 8 ), 'escape' ); @@ -1218,10 +1210,7 @@ select select encode( overlay( - cast('Th\000omas' as bytea) - placing cast('\002\003' as bytea) - from 5 - for 3 + cast('Th\000omas' as bytea) placing cast('\002\003' as bytea) from 5 for 3 ), 'escape' ); diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__timetz_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__timetz_80.snap index 28a87138e..c37a20a7a 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__timetz_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__timetz_80.snap @@ -74,14 +74,14 @@ select f1 + cast('00:01' as time with time zone) as "Illegal" from timetz_tbl; select extract( - microsecond from cast('2020-05-26 13:30:25.575401-04' - as time with time zone) + microsecond from + cast('2020-05-26 13:30:25.575401-04' as time with time zone) ); select extract( - millisecond from cast('2020-05-26 13:30:25.575401-04' - as time with time zone) + millisecond from + cast('2020-05-26 13:30:25.575401-04' as time with time zone) ); select @@ -111,20 +111,20 @@ select select extract( - timezone from cast('2020-05-26 13:30:25.575401-04:30' - as time with time zone) + timezone from + cast('2020-05-26 13:30:25.575401-04:30' as time with time zone) ); select extract( - timezone_hour from cast('2020-05-26 13:30:25.575401-04:30' - as time with time zone) + timezone_hour from + cast('2020-05-26 13:30:25.575401-04:30' as time with time zone) ); select extract( - timezone_minute from cast('2020-05-26 13:30:25.575401-04:30' - as time with time zone) + timezone_minute from + cast('2020-05-26 13:30:25.575401-04:30' as time with time zone) ); select diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__tsearch_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__tsearch_80.snap index 4e0ff5778..ba86eac30 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__tsearch_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__tsearch_80.snap @@ -1,7 +1,6 @@ --- source: crates/pgls_pretty_print/tests/tests.rs input_file: crates/pgls_pretty_print/tests/data/multi/tsearch.sql -snapshot_kind: text --- select oid, @@ -908,7 +907,10 @@ Water, water, every where, S. T. Coleridge (1772-1834) ', to_tsquery('english', 'ocean & seahorse'), 'MaxFragments=1'); -select ts_headline('english', ' +select + ts_headline( + 'english', + ' Day after day, day after day, We stuck, nor breath nor motion, As idle as a painted Ship @@ -918,7 +920,10 @@ Water, water, every where Water, water, every where, Nor any drop to drink. S. T. Coleridge (1772-1834) -', to_tsquery('english', 'Coleridge & stuck'), 'MaxFragments=2,FragmentDelimiter=***'); +', + to_tsquery('english', 'Coleridge & stuck'), + 'MaxFragments=2,FragmentDelimiter=***' + ); select ts_headline( diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__txid_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__txid_80.snap index 525c1abdf..f0a4e2257 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__txid_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__txid_80.snap @@ -1,7 +1,6 @@ --- source: crates/pgls_pretty_print/tests/tests.rs input_file: crates/pgls_pretty_print/tests/data/multi/txid.sql -snapshot_kind: text --- select cast('12:13:' as txid_snapshot); @@ -94,8 +93,8 @@ select txid_current_if_assigned() is null; select txid_current(); select - txid_current_if_assigned() is not distinct from cast('txid_current' - as bigint); + txid_current_if_assigned() is not distinct from + cast('txid_current' as bigint); commit; diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__xid_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__xid_80.snap index 8b7ae5cca..9c0b3e51f 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__xid_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__xid_80.snap @@ -1,7 +1,6 @@ --- source: crates/pgls_pretty_print/tests/tests.rs input_file: crates/pgls_pretty_print/tests/data/multi/xid.sql -snapshot_kind: text --- select cast('010' as xid), @@ -189,8 +188,8 @@ select pg_current_xact_id_if_assigned() is null; select pg_current_xact_id(); select - pg_current_xact_id_if_assigned() is not distinct from cast('pg_current_xact_id' - as xid8); + pg_current_xact_id_if_assigned() is not distinct from + cast('pg_current_xact_id' as xid8); commit; diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index c81960382..3c4e52b0b 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -32,14 +32,6 @@ const WINDOWS_SKIP_FILES: &[&str] = &[ "tidscan", ]; -#[derive(Debug, Clone, PartialEq, Eq)] -enum StringState { - None, - Single, - Double, - Dollar(Vec), -} - /// A fixture may open with `-- pgls-format: key=value, key=value` to declare the configuration it /// must be rendered with. Keeping it in the fixture rather than in the harness is what lets an /// option that is off by default own its own test data. @@ -168,7 +160,7 @@ fn test_single(fixture: Fixture<&str>) { println!("Formatted content (width={max_line_length}):\n{output}"); - assert_line_lengths(&output, max_line_length); + assert_line_lengths(&output, max_line_length, fixture_config.indent_size); common::assert_clean_whitespace(&output); let parsed_output = pgls_query::parse(&output).expect("Failed to parse SQL"); @@ -257,7 +249,7 @@ fn test_multi(fixture: Fixture<&str>) { renderer.render(emitter.events).expect("Failed to render"); // Verify line length - assert_line_lengths(&output, max_line_length); + assert_line_lengths(&output, max_line_length, fixture_config.indent_size); common::assert_clean_whitespace(&output); // Verify AST equality @@ -292,146 +284,11 @@ fn test_multi(fixture: Fixture<&str>) { } } -fn assert_line_lengths(sql: &str, max_line_length: usize) { - let mut state = StringState::None; - - for line in sql.lines() { - let chars: Vec = line.chars().collect(); - let mut i = 0usize; - let mut current_outside_run = 0usize; - let mut max_outside_run = 0usize; - let mut longest_token = 0usize; - let mut current_token_len = 0usize; - - while i < chars.len() { - match state.clone() { - StringState::None => { - current_outside_run += 1; - if current_outside_run > max_outside_run { - max_outside_run = current_outside_run; - } - - // Track token length (identifier-like sequences and numeric literals) - // Numeric literals can include digits, '.', '-', '+', 'e', 'E' - let is_token_char = chars[i].is_alphanumeric() - || chars[i] == '_' - || chars[i] == '.' - || ((chars[i] == '-' || chars[i] == '+') - && (current_token_len == 0 - || (i > 0 && (chars[i - 1] == 'e' || chars[i - 1] == 'E')))); - - if is_token_char { - current_token_len += 1; - } else { - if current_token_len > longest_token { - longest_token = current_token_len; - } - current_token_len = 0; - } - - match chars[i] { - '\'' => { - state = StringState::Single; - current_outside_run = 0; - i += 1; - } - '"' => { - state = StringState::Double; - current_outside_run = 0; - i += 1; - } - '$' => { - if let Some((tag, len)) = parse_dollar_tag(&chars[i..]) { - state = StringState::Dollar(tag); - current_outside_run = 0; - i += len; - } else { - i += 1; - } - } - _ => { - i += 1; - } - } - } - StringState::Single => { - if chars[i] == '\'' { - if i + 1 < chars.len() && chars[i + 1] == '\'' { - i += 2; - } else { - state = StringState::None; - current_outside_run = 0; - i += 1; - } - } else { - i += 1; - } - } - StringState::Double => { - if chars[i] == '"' { - if i + 1 < chars.len() && chars[i + 1] == '"' { - i += 2; - } else { - state = StringState::None; - current_outside_run = 0; - i += 1; - } - } else { - i += 1; - } - } - StringState::Dollar(tag) => { - if chars[i] == '$' && slice_starts_with(&chars[i..], &tag) { - state = StringState::None; - current_outside_run = 0; - i += tag.len(); - } else { - i += 1; - } - } - } - } - - // Check final token - if current_token_len > longest_token { - longest_token = current_token_len; - } - - // If the longest token exceeds max length, the line can't be broken further - // Allow lines where the excess is due to an unbreakable identifier/literal - // Also allow some overhead for indentation, keywords, and punctuation (up to 20 chars) - // This accounts for things like "CREATE TRIGGER " or "EXECUTE FUNCTION " prefixes - let min_overhead = 20; - if max_outside_run > max_line_length && longest_token + min_overhead <= max_line_length { - panic!("Line exceeds max length of {max_line_length} outside literals: {line}"); - } - } -} - -fn parse_dollar_tag(chars: &[char]) -> Option<(Vec, usize)> { - if chars.is_empty() || chars[0] != '$' { - return None; - } - - let mut end = 1usize; - while end < chars.len() { - let c = chars[end]; - if c.is_ascii_alphanumeric() || c == '_' { - end += 1; - } else { - break; - } - } - - if end < chars.len() && chars[end] == '$' { - let mut tag = Vec::with_capacity(end + 1); - tag.extend_from_slice(&chars[..=end]); - Some((tag, end + 1)) - } else { - None - } -} - -fn slice_starts_with(haystack: &[char], needle: &[char]) -> bool { - haystack.len() >= needle.len() && haystack[..needle.len()] == needle[..] +fn assert_line_lengths(output: &str, max_line_length: usize, tab_width: usize) { + let problems = common::width_problems(output, max_line_length, tab_width); + assert!( + problems.is_empty(), + "lines past the width:\n{}\n\n{output}", + problems.join("\n") + ); } diff --git a/crates/pgls_pretty_print/tests/width_layout.rs b/crates/pgls_pretty_print/tests/width_layout.rs index 0af64a2a8..2667433b9 100644 --- a/crates/pgls_pretty_print/tests/width_layout.rs +++ b/crates/pgls_pretty_print/tests/width_layout.rs @@ -1,6 +1,8 @@ //! Statements whose parts used to run past the line width because they had no break opportunity //! or broke without an indent. +mod common; + use pgls_pretty_print::{FormatConfig, format_statement}; const WIDTH: usize = 60; @@ -116,3 +118,55 @@ fn explain_options_break_like_a_list() { "explain (analyze true, buffers true, verbose true, format json, costs false) select * from some_table", ); } + +#[test] +fn is_distinct_from_and_like_wrap_like_a_comparison() { + assert_eq!( + assert_fits( + "select pg_current_xact_id_if_assigned() is not distinct from cast('pg_current_xact_id' as xid8)" + ), + "select\n pg_current_xact_id_if_assigned() is not distinct from\n cast('pg_current_xact_id' as xid8);" + ); + assert_fits( + "select * from t where accounting_lines.accounting_class not similar to '70(1|2|4)%-some-longer-pattern'", + ); +} + +#[test] +fn text_after_a_multi_line_literal_has_to_fit_on_its_last_line() { + let output = format_idempotently( + "select f('first line\nsecond line', some_long_argument_name, another_long_argument_name, a_third_one) from t", + ); + assert!(output.contains("\nsecond line',\n"), "{output}"); + assert!( + common::width_problems(&output, WIDTH, 4).is_empty(), + "{output}" + ); +} + +#[test] +fn the_width_check_counts_literals_but_excuses_unbreakable_lines() { + // A call with a literal among its arguments could have broken. + assert_eq!( + common::width_problems( + "select f('a fairly long literal', 'and another one', 3);", + 40, + 4 + ) + .len(), + 1 + ); + // A literal that is longer than the line on its own could not. + assert!( + common::width_problems( + "select 'a literal far longer than the line can hold';", + 40, + 4 + ) + .is_empty() + ); + // Neither could a trailing comment. + assert!( + common::width_problems("select 1; -- a comment far longer than the line", 40, 4).is_empty() + ); +}