From 5cba6beeb1d3c56225c36dae036063f3daf2c657 Mon Sep 17 00:00:00 2001 From: Hein Date: Wed, 15 Jul 2026 21:11:58 +0200 Subject: [PATCH] fix: align PLpgSQL formatting clauses --- pkg/format/body.go | 135 +++++++++++++++++++++++++++-- pkg/format/format_test.go | 60 +++++++++++++ testdata/corpus/test_mm_proc.pgsql | 36 ++++---- 3 files changed, 206 insertions(+), 25 deletions(-) diff --git a/pkg/format/body.go b/pkg/format/body.go index 88bb116..10a97cc 100644 --- a/pkg/format/body.go +++ b/pkg/format/body.go @@ -441,13 +441,9 @@ func formatBodyStatements(text string, st config.Style) string { stmtLines = joinThenToCondition(stmt) } - for i, ll := range stmtLines { - if i == 0 || ll.indent == "" { - result.WriteString(baseIndent) - } else { - result.WriteString(ll.indent) - } - result.WriteString(ll.text) + formattedLines := formatBodyStmtLines(stmtLines, baseIndent, st) + for _, line := range formattedLines { + result.WriteString(line) result.WriteString(nl) } @@ -574,6 +570,131 @@ func formatBodyStatements(text string, st config.Style) string { return result.String() } +// formatBodyStmtLines formats one flushed PL/pgSQL statement at its contextual +// base indent. Multi-line UPDATE/DELETE statements inside PL/pgSQL get their +// top-level SET/WHERE/AND/OR clauses realigned under the statement while nested +// subqueries keep their original indentation. Non-DML statements keep +// continuation indentation, except that standalone structural keywords such as +// THEN are aligned with the block opener. +func formatBodyStmtLines(lines []bline, baseIndent string, st config.Style) []string { + if len(lines) == 0 { + return nil + } + + if looksLikeMultiLineBodyDML(lines) { + return reindentBodyDML(lines, baseIndent, st) + } + + out := make([]string, 0, len(lines)) + for i, ll := range lines { + text := ll.text + indent := baseIndent + if i > 0 && ll.indent != "" && !isStandaloneBodyKeyword(ll.text, "then", "else", "elsif", "elseif") { + indent = ll.indent + } + out = append(out, indent+text) + } + return out +} + +func looksLikeMultiLineBodyDML(lines []bline) bool { + if len(lines) < 2 { + return false + } + kw := lowerASCII(firstBodyKeyword(lines[0].text)) + return kw == "update" || kw == "delete" +} + +func reindentBodyDML(lines []bline, baseIndent string, st config.Style) []string { + out := make([]string, 0, len(lines)+1) + afterWhere := false + parenDepth := 0 + for i, ll := range lines { + text := strings.TrimRight(ll.text, " ") + lineDepth := parenDepth + kw := lowerASCII(firstBodyKeyword(text)) + if afterWhere && lineDepth == 0 && kw != "and" && kw != "or" { + out = append(out, baseIndent+st.Indent+st.Indent+strings.TrimSpace(text)) + afterWhere = false + updateBodyParenDepth(text, &parenDepth) + continue + } + if lineDepth == 0 && (kw == "set" || kw == "where" || kw == "values" || kw == "returning") { + if kw == "where" { + whereText := strings.TrimSpace(text) + fields := strings.Fields(whereText) + nextKw := "" + if i+1 < len(lines) { + nextKw = lowerASCII(firstBodyKeyword(lines[i+1].text)) + } + if len(fields) > 1 && (nextKw == "and" || nextKw == "or") { + out = append(out, baseIndent+fields[0]) + out = append(out, baseIndent+st.Indent+st.Indent+strings.TrimSpace(whereText[len(fields[0]):])) + afterWhere = false + continue + } + afterWhere = len(fields) == 1 + } + out = append(out, baseIndent+strings.TrimSpace(text)) + continue + } + if lineDepth == 0 && (kw == "and" || kw == "or") { + out = append(out, baseIndent+st.Indent+strings.TrimSpace(text)) + afterWhere = false + updateBodyParenDepth(text, &parenDepth) + continue + } + if i == 0 { + out = append(out, baseIndent+strings.TrimSpace(text)) + } else if ll.indent != "" { + out = append(out, ll.indent+strings.TrimSpace(text)) + } else { + out = append(out, baseIndent+strings.TrimSpace(text)) + } + afterWhere = false + updateBodyParenDepth(text, &parenDepth) + } + return out +} + +func updateBodyParenDepth(s string, depth *int) { + for _, tok := range lexer.Lex(s) { + switch tok.Kind { + case lexer.LParen, lexer.LBracket: + (*depth)++ + case lexer.RParen, lexer.RBracket: + if *depth > 0 { + (*depth)-- + } + } + } +} + +func significantBodyTokens(s string) []cst.Tok { + var toks []cst.Tok + for _, tok := range lexer.Lex(s) { + if tok.IsTrivia() || tok.Kind == lexer.EOF { + continue + } + toks = append(toks, cst.Tok{Tok: tok}) + } + return toks +} + +func isStandaloneBodyKeyword(s string, kws ...string) bool { + toks := significantBodyTokens(s) + if len(toks) != 1 || toks[0].Tok.Kind != lexer.Ident { + return false + } + low := lowerASCII(toks[0].Tok.Text) + for _, kw := range kws { + if low == kw { + return true + } + } + return false +} + // joinThenToCondition merges a THEN line (on its own bline) into the preceding // condition line when plpgsql_if_then_newline is false. func joinThenToCondition(lines []bline) []bline { diff --git a/pkg/format/format_test.go b/pkg/format/format_test.go index 0bdd9ee..31ee278 100644 --- a/pkg/format/format_test.go +++ b/pkg/format/format_test.go @@ -107,6 +107,66 @@ func TestFormatMmProcBroken(t *testing.T) { } } +func TestFormatIssue1PLpgSQLIndenting(t *testing.T) { + src := "CREATE FUNCTION f() RETURNS void LANGUAGE plpgsql AS $$\n" + + "DECLARE\n" + + " r_lp record;\n" + + "BEGIN\n" + + " if r_lp.total > 0\n" + + " and r_lp.totaldone >= r_lp.total\n" + + " then\n" + + " update core.process u\n" + + " set status = 'done'\n" + + " where u.rid_process = r_lp.rid_process\n" + + " and nv(u.status) <> 'done';\n" + + " elsif r_lp.total > 0\n" + + " then\n" + + " update core.process u\n" + + " set status = 'open'\n" + + " where u.rid_process = r_lp.rid_process\n" + + " and nv(u.status) <> 'open';\n" + + "\n" + + " end if;\n" + + "$$;\n" + + want := "CREATE FUNCTION f(\n" + + ")\n" + + "RETURNS void\n" + + "LANGUAGE plpgsql\n" + + "AS\n" + + "$$\n" + + "DECLARE\n" + + " r_lp record;\n" + + "BEGIN\n" + + " if r_lp.total > 0\n" + + " and r_lp.totaldone >= r_lp.total\n" + + " then\n" + + " update core.process u\n" + + " set status = 'done'\n" + + " where\n" + + " u.rid_process = r_lp.rid_process\n" + + " and nv(u.status) <> 'done';\n" + + " elsif r_lp.total > 0\n" + + " then\n" + + " update core.process u\n" + + " set status = 'open'\n" + + " where\n" + + " u.rid_process = r_lp.rid_process\n" + + " and nv(u.status) <> 'open';\n" + + "\n" + + " end if;\n" + + "$$;\n" + + got := format(src) + if got != want { + t.Errorf("issue #1 PL/pgSQL indenting\n--- got ---\n%s\n--- want ---\n%s", got, want) + } + checkDML(t, "issue #1 PL/pgSQL indenting", got) + if !semanticallyEqual(src, got) { + t.Errorf("issue #1 PL/pgSQL indenting changed semantics") + } +} + func TestCorpusIdempotentAndSafe(t *testing.T) { dir := filepath.Join("..", "..", "testdata", "corpus") entries, err := os.ReadDir(dir) diff --git a/testdata/corpus/test_mm_proc.pgsql b/testdata/corpus/test_mm_proc.pgsql index 3b420a3..e39f5e5 100644 --- a/testdata/corpus/test_mm_proc.pgsql +++ b/testdata/corpus/test_mm_proc.pgsql @@ -632,7 +632,7 @@ BEGIN end if;*/ if G_BENCHMARK = 1 - then + then perform log_event(m_funcname,format('Perf Merge Replace SinceStart: %s Duration: %s', clock_timestamp() - m_start, clock_timestamp() - m_ltime),bt_enum('eventlog','local notice')); m_ltime = clock_timestamp(); end if; @@ -735,7 +735,7 @@ BEGIN , '\[(.*?)\]', 'ig') r(v) into m_errmsg,m_retval; if m_retval > 0 - then + then raise exception E'The following data fields could not be found for prefix % \r\n%', ifblnk(m_data_prefix, 'Null'), m_errmsg using hint = 'in ID replace process'; end if; @@ -1136,16 +1136,16 @@ BEGIN -- ); if r_lp_t.table_name = any(a_inner_selected) and nv(r_lp_t.table_name ) <> '' - then + then raise notice 'Table used as inner table: %', r_lp_t.table_name; continue; end if; --raise notice 'Field:% Table:% Tag: % Type: %', r_lp_t.field_name, r_lp_t.table_name, r_lp_t.mergetag, r_lp_t.merge_type; if nv(r_lp_t.field_name) = '' and r_lp_t.merge_type not in (G_MTYPE_TBLROOT,G_MTYPE_SPECIAL) - then + then if G_DEBUG - then + then perform log_event(m_funcname,format('Blank field name on Complex merge for p_doctype=%s, p_commtype=%s, p_data_prefix=%s, p_data_rid=%s field_name=%s, merge_type=%s, table_name=%s ' ,p_doctype, p_commtype,p_data_prefix,p_data_rid @@ -1159,7 +1159,7 @@ BEGIN end if; if nv(m_exec_orderstr) = '' and nv(r_lp_t.parent_order_string) <> '' - then + then m_exec_orderstr = r_lp_t.parent_order_string; --raise notice 'Applying order % by for % %.', r_lp_t.parent_order_string, r_lp_t.parent_table_name,r_lp_t.field_name; /* @@ -1176,7 +1176,7 @@ BEGIN end if; if nv(r_lp_t.ops_string) = '' - then + then r_lp_t.ops_string = r_lp_t.field_name; end if; @@ -1192,19 +1192,19 @@ BEGIN end loop; if r_lp_t.merge_type in (G_MTYPE_TBLFIELD, G_MTYPE_CONDFIELD) - then + then m_execstr = format($S$%s|| '%s"%s":' || json_build_object('value',json_agg(%s::text %s), 'type', '%s')::text %s$S$ ,m_execstr,m_comma,r_lp_t.mergetag ,r_lp_t.ops_string , m_exec_orderstr, r_lp_t.merge_type, E'\r\n'); elseif r_lp_t.merge_type = G_MTYPE_SPECIAL--special fields - then + then m_execstr = format($S$%s|| '%s"%s":' || json_build_object('value',%s, 'type', '%s')::text %s$S$ ,m_execstr,m_comma,r_lp_t.mergetag,quote_literal(r_lp_t.tagvalue), r_lp_t.merge_type, E'\r\n'); --raise notice 'Special Field: %s',r_lp_t; elseif r_lp.merge_type = G_MTYPE_PICTURE - then + then m_execstr = format($S$%s|| '%s"%s":' || json_build_object('value',%s::text, 'type', '%s' , 'w', mailmerge_specialfield('width', '%s', %s) ,'h', mailmerge_specialfield('height', '%s', %s))::text %s$S$ @@ -1212,7 +1212,7 @@ BEGIN ,r_lp.mergetag,quote_nullable(m_data_rid),r_lp.mergetag,quote_nullable(m_data_rid), E'\r\n'); elseif nv(r_lp_t.field_name) <> '' - then + then m_execstr = format($S$%s|| '%s"%s":' || json_build_object('value',%s::text, 'type', '%s')::text %s$S$ ,m_execstr,m_comma,r_lp_t.mergetag , r_lp_t.ops_string @@ -1225,7 +1225,7 @@ BEGIN m_blankexec = format($S$%s|| '%s"%s":' || json_build_object('value','', 'type', '%s')::text %s$S$,m_blankexec,m_comma,r_lp_t.mergetag, r_lp_t.merge_type, E'\r\n'); if r_lp_t.rn = 1 - then + then --Inner level tables (2) --raise notice 'Begin: parent: %', r_lp_t; for r_lp_c in ( @@ -1345,7 +1345,7 @@ BEGIN end loop; if ifblnk(r_lp_t.parent_table_name,'') = '' - then + then m_execstr = format(E'select (''{'' %s \r\n || ''}'')::json ;',m_execstr ); else select string_agg(s.filter_string, ' ') @@ -1364,7 +1364,7 @@ BEGIN from exec_json(m_execstr, 'str json') r into m_retval,m_errmsg, m_json; if m_json is null - then + then select r.p_retval, r.p_errmsg, r.p_json - > 'str' from exec_json(m_execstr, 'str json') r into m_retval,m_errmsg, m_json; @@ -1373,12 +1373,12 @@ BEGIN m_debug_exestr = nv(m_debug_exestr) || E'\r\n/*'|| nv(r_lp_t.parent_table_name) || ' len:' || nv(length(m_json::text)) ||E'*/ \r\n' || nv(m_execstr) || E'\r\n '; if m_json_full_complex is null - then + then m_json_full_complex = jsonb_build_object(r_lp_t.tblid::text,m_json); end if; if (m_json_full_complex->r_lp_t.tblid::text) is null - then + then m_json_full_complex = jsonb_set(m_json_full_complex, format('{%s}',r_lp_t.tblid)::text[], m_json::jsonb,true); else m_json_full_complex = jsonb_set(m_json_full_complex, format('{%s}',r_lp_t.tblid)::text[], _jsonb_object_cat(m_json_full_complex->r_lp_t.tblid,m_json::jsonb),true); @@ -1399,13 +1399,13 @@ BEGIN end if; if nv(m_comma) = '' and length(m_execstr) > 2 - then + then m_comma = ','; end if; end loop; if G_DEBUG - then + then perform pl_writefile(r_template.debugsql_filename, convert_to(m_debug_exestr,'utf8')); end if; -- 2.54.0