diff --git a/.gitignore b/.gitignore index fbd390a..57a13ca 100644 --- a/.gitignore +++ b/.gitignore @@ -163,3 +163,5 @@ dist .yarn/install-state.gz .pnp.* +temp/* +dist/* \ No newline at end of file diff --git a/pkg/format/body.go b/pkg/format/body.go new file mode 100644 index 0000000..c309570 --- /dev/null +++ b/pkg/format/body.go @@ -0,0 +1,182 @@ +package format + +import ( + "strings" + + "github.com/hein/pgtidy/pkg/config" + "github.com/hein/pgtidy/pkg/cst" + "github.com/hein/pgtidy/pkg/lexer" +) + +// formatBody applies house-style formatting to a PL/pgSQL dollar-quoted body +// token. Currently only the DECLARE section is formatted; the rest is verbatim. +func formatBody(bodyText string, st config.Style) string { + open, inner, close, ok := splitDollarQuote(bodyText) + if !ok { + return bodyText + } + return open + formatBodyInner(inner, st) + close +} + +// splitDollarQuote splits a dollar-quoted token (e.g. "$$...\n$$" or +// "$S$...$S$") into (open tag, inner text, close tag). The open and close tags +// are the same string; the last occurrence in s is taken as the close tag. +func splitDollarQuote(s string) (open, inner, close string, ok bool) { + if len(s) == 0 || s[0] != '$' { + return + } + end := strings.Index(s[1:], "$") + if end < 0 { + return + } + openLen := end + 2 + open = s[:openLen] + closeStart := strings.LastIndex(s, open) + if closeStart < openLen { + return + } + inner = s[openLen:closeStart] + close = s[closeStart:] + ok = true + return +} + +func formatBodyInner(inner string, st config.Style) string { + sig, _ := cst.Attach(lexer.Lex(inner)) + nl := st.Newline + + // Find DECLARE at depth 0. + declareIdx := -1 + for i, t := range sig { + if t.Tok.Kind == lexer.Ident && lowerASCII(t.Tok.Text) == "declare" { + declareIdx = i + break + } + } + if declareIdx < 0 { + return inner + } + + // Find BEGIN at depth 0 after DECLARE. + beginIdx := -1 + depth := 0 + for i := declareIdx + 1; i < len(sig); i++ { + switch sig[i].Tok.Kind { + case lexer.LParen, lexer.LBracket: + depth++ + case lexer.RParen, lexer.RBracket: + if depth > 0 { + depth-- + } + } + if depth == 0 && sig[i].Tok.Kind == lexer.Ident && lowerASCII(sig[i].Tok.Text) == "begin" { + beginIdx = i + break + } + } + if beginIdx < 0 { + return inner + } + + var b strings.Builder + + // Emit verbatim up to and including DECLARE (keyword-cased). + for i := 0; i <= declareIdx; i++ { + t := sig[i] + for _, tr := range t.Lead { + b.WriteString(tr.Text) + } + if i == declareIdx { + b.WriteString(applyCase(t.Tok.Text, st.KeywordCase)) + } else { + b.WriteString(t.Tok.Text) + } + } + b.WriteString(nl) + + // Format each variable declaration in the DECLARE section. + formatDeclareVars(&b, sig[declareIdx+1:beginIdx], st) + + // Emit BEGIN and everything after it verbatim from the original source. + // The newline before BEGIN is supplied by the last declaration's line end. + b.WriteString(inner[sig[beginIdx].Tok.Off:]) + + return b.String() +} + +// formatDeclareVars writes each variable declaration as a single indented line. +// Comments in the leading trivia of a declaration's first token are preserved +// on their own lines. If a declaration carries mid-body comments it is emitted +// verbatim to avoid losing them. +func formatDeclareVars(b *strings.Builder, toks []cst.Tok, st config.Style) { + nl := st.Newline + indent := st.Indent + depth := 0 + var cur []cst.Tok + var preComments []string + + emit := func() { + if len(cur) == 0 { + return + } + for _, c := range preComments { + b.WriteString(indent) + b.WriteString(c) + b.WriteString(nl) + } + preComments = nil + + // Graceful degradation: mid-declaration comments stay verbatim. + if anyComment(cur[1:]) { + b.WriteString(indent) + b.WriteString(verbatimSpan(cur)) + b.WriteString(nl) + cur = nil + return + } + + body := cur + hasSemi := len(body) > 0 && body[len(body)-1].Tok.Kind == lexer.Semicolon + if hasSemi { + body = body[:len(body)-1] + } + b.WriteString(indent) + for i, t := range body { + if i > 0 && needSpace(body[i-1].Tok, t.Tok) { + b.WriteByte(' ') + } + b.WriteString(caseText(t.Tok, st)) + } + if hasSemi { + b.WriteString(";") + } + b.WriteString(nl) + cur = nil + } + + for _, t := range toks { + if len(cur) == 0 { + for _, tr := range t.Lead { + if tr.Kind == lexer.LineComment || tr.Kind == lexer.BlockComment { + preComments = append(preComments, strings.TrimRight(tr.Text, " \t")) + } + } + } + + switch t.Tok.Kind { + case lexer.LParen, lexer.LBracket: + depth++ + case lexer.RParen, lexer.RBracket: + if depth > 0 { + depth-- + } + } + + cur = append(cur, t) + + if t.Tok.Kind == lexer.Semicolon && depth == 0 { + emit() + } + } + emit() +} diff --git a/pkg/format/format.go b/pkg/format/format.go index ba8a4ab..afe2153 100644 --- a/pkg/format/format.go +++ b/pkg/format/format.go @@ -104,7 +104,7 @@ func (p *printer) writeCreateFunction(cf *cst.CreateFunction) { } if cf.Body != nil { p.nl() - p.b.WriteString(cf.Body.Tok.Text) // body emitted verbatim (formatted later) + p.b.WriteString(formatBody(cf.Body.Tok.Text, p.st)) } for _, clause := range cf.Tail { p.nl() @@ -179,6 +179,11 @@ func needSpace(a, b lexer.Token) bool { case lexer.Ident, lexer.QuotedIdent, lexer.RParen, lexer.RBracket, lexer.Param: return false // function call / type modifier } + case lexer.LBracket: + switch a.Kind { + case lexer.Ident, lexer.QuotedIdent, lexer.RParen, lexer.RBracket: + return false // array subscript / array type modifier + } case lexer.Operator: if tightOps[b.Text] { return false diff --git a/pkg/format/format_test.go b/pkg/format/format_test.go index 2e7fefb..15032ca 100644 --- a/pkg/format/format_test.go +++ b/pkg/format/format_test.go @@ -83,8 +83,9 @@ func TestCorpusIdempotentAndSafe(t *testing.T) { // semanticallyEqual compares the non-trivia token streams of two sources, // treating unquoted identifiers/keywords case-insensitively and everything -// else (strings, numbers, dollar bodies, operators, punctuation) exactly. This -// validates that formatting changed only layout/casing, never meaning. +// else (strings, numbers, operators, punctuation) exactly. Dollar-quoted body +// tokens are compared recursively so body whitespace normalization does not +// trigger a false failure. func semanticallyEqual(a, b string) bool { ta := significant(a) tb := significant(b) @@ -95,12 +96,21 @@ func semanticallyEqual(a, b string) bool { if ta[i].Kind != tb[i].Kind { return false } - if ta[i].Kind == lexer.Ident { + switch ta[i].Kind { + case lexer.Ident: if !strings.EqualFold(ta[i].Text, tb[i].Text) { return false } - } else if ta[i].Text != tb[i].Text { - return false + case lexer.DollarString: + _, innerA, _, okA := splitDollarQuote(ta[i].Text) + _, innerB, _, okB := splitDollarQuote(tb[i].Text) + if okA != okB || (okA && !semanticallyEqual(innerA, innerB)) { + return false + } + default: + if ta[i].Text != tb[i].Text { + return false + } } } return true diff --git a/testdata/corpus/test_a.pgsql b/testdata/corpus/test_a.pgsql index 5934f17..2acd063 100755 --- a/testdata/corpus/test_a.pgsql +++ b/testdata/corpus/test_a.pgsql @@ -54,7 +54,7 @@ BEGIN ,encode(digest(format('%s',p_data->>'password'), 'md5'), 'hex') ]::citext[]; - if m_allow_hash_auth + if m_allow_hash_auth then m_pass_hashed := m_pass_hashed || array[ p_data->>'password' diff --git a/testdata/corpus/test_a_broken.pgsql b/testdata/corpus/test_a_broken.pgsql new file mode 100755 index 0000000..16404fa --- /dev/null +++ b/testdata/corpus/test_a_broken.pgsql @@ -0,0 +1,198 @@ +--select * from dropall('resolvespec_login'); +CREATE +OR REPLACE FUNCTION resolvespec_login( + INOUT p_data jsonb + ,OUT p_success boolean + ,OUT p_error text +) +LANGUAGE plpgsql VOLATILE +SECURITY DEFINER +AS +$$ +DECLARE + --Error Handling-- +m_funcname text = 'resolvespec_login'; + m_errmsg +text; + m_errcontext +text; + m_errdetail +text; + m_errhint +text; + m_errstate +text; + m_retval +integer; + --Error Handling-- + m_rid_user +integer; + m_rid_hub +integer; + m_pass_hashed +citext[]; + m_session +jsonb; + m_allow_hash_auth +boolean; +BEGIN + m_allow_hash_auth += _try_integer( p_data->'claims'->>'rid_user',0) > 0; + create +extension if not exists pgcrypto; + + perform +log_event(m_funcname,format('API Login username: %s hh=%s claims: %s',p_data->>'username',m_allow_hash_auth,p_data->'claims'), bt_enum('eventlog','local notice')); + +select h.rid_hub +from public.user h +where h.usercode = p_data ->>'username' +into m_rid_hub; + +if +m_rid_hub is null + and exists (select 1 from information_schema.tables t where t.table_schema = 'public' and t.table_name = 'users') + then +select u.rid_hub, u.rid_user +from public.users u +where u.rid_user = _try_integer(p_data - > 'claims' ->>'rid_user', 0) into m_rid_hub,m_rid_user; + +end if; + + if +m_rid_hub is null + then + raise exception 'Invalid username / password'; +end if; + + m_pass_hashed += array[encode(digest(format('%s:%s',p_data->>'username',p_data->>'password'), 'sha512'), 'hex') + ,encode(digest(format('%s:%s',p_data->>'username',p_data->>'password'), 'md5'), 'hex') + ,encode(digest(format('%s',p_data->>'password'), 'md5'), 'hex') + ]::citext[]; + + if +m_allow_hash_auth + then + m_pass_hashed := m_pass_hashed || array[ + p_data->>'password' + ]::citext[]; +end if; + + --select $A${"meta": null, "claims": {"rid_user": 30000024}, "password": "c4ca4238a0b923820dcc509a6f75849b", "username": "SUPPORT"}$A$::jsonb->>'password' + --c4ca4238a0b923820dcc509a6f75849b + --select * from v_eventlog + + if +exists ( + select 1 + from information_schema.tables t + where t.table_schema = 'public' + and t.table_name = 'users' + ) + then + if not exists ( + select 1 + from public.user h + left outer join public.users usr on usr.rid_hub = h.rid_hub + where h.rid_hub = m_rid_hub + and ( + h.password = any (m_pass_hashed) + and nv(h.password) <> '' + or usr.password = any(m_pass_hashed) + and nv(usr.password) <> '' + ) + ) + then + raise exception 'Password incorrect'; +end if; + + elsif +not exists ( + select 1 + from public.user h + where h.rid_hub = m_rid_hub + and h.password = any(m_pass_hashed) + and nv(h.password) <> '' + ) + then + raise exception 'Password incorrect'; +end if; + + if +_try_bool(p_data->'jsonvalue'->>'issecurity',false) + and not exists ( + select h.rid_hub + from public.user h + inner join public.user_all_parents(m_rid_hub) p on p.parent_rid_hub = h.rid_hub + where h.hubname ilike '%Access Control%' + ) + then + raise exception 'Cannot login with security mode. User must be in Access Control group'; +end if; + +with newsession as ( +insert +into core._loginsession (createtm, modifytm, rid_user, usertable, sessionid, token, useragent, location, + ipaddress, expiretm, jsonvalue) +select now(), + now(), + m_rid_hub, + 'hub', + newid(), + newid(), + p_data ->>'user-agent', p_data->>'fromurl' + , p_data->>'host', (now() + '31 days':: interval), p_data->'jsonvalue' + returning * + ) +select to_jsonb(newsession) +from newsession into m_session; + +if +_try_integer(m_session->>'rid_user',0) > 0 + then +update public.user u +set jsonvalue = _jsonb_object_cat(u.jsonvalue, jsonb_build_object('lastlogin', to_char(now(), 'YYYY-MM-DD HH24:mi:SS'))) +where u.rid_hub = m_rid_hub; +end if; + + + +select jsonb_build_object('token', m_session ->>'token' + , 'session', m_session ->>'session' + , 'user', _jsonb_object_cat(jsonb_build_object( + 'user_id', h.rid_hub + , 'username', h.usercode + , 'email', null + , 'user_level', 0 + , 'roles', jsonb_build_array() + , 'session_id', m_session ->>'sessionid' + , 'token', m_session ->>'token' + , 'session_rid', _try_integer(m_session ->>'id') + ), + (select jsonb_build_object('program_user_table', r.tablename, 'program_user_id', + _try_integer(r.key, 0)) + from public.user_tableinfo(m_rid_hub) r) + ) + , 'expires_in', 86400 + ) +from public.user h +where h.rid_hub = m_rid_hub into p_data; + +p_success += true; +EXCEPTION +WHEN others THEN + GET STACKED DIAGNOSTICS + m_errmsg = MESSAGE_TEXT + ,m_errcontext = PG_EXCEPTION_CONTEXT + ,m_errdetail = PG_EXCEPTION_DETAIL + ,m_errhint = PG_EXCEPTION_HINT + ,m_errstate = RETURNED_SQLSTATE; + + p_error +:= get_err_msg(m_funcname, m_errmsg, m_errcontext, m_errdetail, m_errhint, m_errstate); + p_success +:= false; +END; +$$; \ No newline at end of file diff --git a/todo.md b/todo.md index 3835ab7..c327b70 100644 --- a/todo.md +++ b/todo.md @@ -38,22 +38,61 @@ Legend: โœ… done ยท ๐Ÿšง in progress ยท โฌœ not started - **Status:** all tests pass. - _Still TODO (later): DML/other-DDL structuring (currently Raw) for full formatting._ -### โฌœ PL/pgSQL body parser โ† NEXT (the remaining V1 piece) -- Parse `$$ ... $$` bodies: DECLARE/BEGIN/END, IF/ELSIF/ELSE/END IF, LOOP/FOR/WHILE, - CASE, assignments (`:=` / `=`), RAISE, nested SQL statements, EXCEPTION blocks. -- The crux for stored-procedure formatting (primary use case). Currently the body is - emitted verbatim; this milestone formats inside it. Reuse `inline`/spacing/casing from - `pkg/format` and keep verbatim fallback for unparsable constructs. +### ๐Ÿšง PL/pgSQL body parser โ† NEXT (the remaining V1 piece) + +#### โœ… DECLARE section โ€” `pkg/format/body.go` +- `formatBody` splits the dollar-quote tag, calls `formatBodyInner`. +- `formatBodyInner` locates `DECLARE` and `BEGIN` at depth 0, formats the declare block, + then emits `BEGIN` onwards verbatim. +- `formatDeclareVars`: each variable declaration collapsed to one line + (` name type [= expr];`), `--Block--` comment markers preserved on their own lines, + mid-declaration block comments trigger verbatim fallback. +- `needSpace` fixed for `LBracket` โ€” no space before `[` after ident/closing bracket + (fixes `citext[]`, array subscripts). +- `semanticallyEqual` in tests updated to recurse into dollar-quoted body tokens so + whitespace normalization inside the body does not falsely fail the semantic check. +- Added `testdata/corpus/test_a_broken.pgsql` โ€” a CRLF corpus file with intentionally + broken layout (split-line variables + split-line body statements) used as a formatting + target; `testdata/corpus/test_a.pgsql` is the golden output. +- **Status:** all tests pass; DECLARE section formats correctly. + +#### โฌœ Body statement formatter โ€” `pkg/format/body.go` (next step) +Two-phase formatter for the `BEGIN โ€ฆ END` block: + +1. **Block-depth tracker** โ€” recognise `BEGIN`/`END`, `IF`/`THEN`/`ELSIF`/`ELSE`/`END IF`, + `LOOP`/`END LOOP`, `FOR`/`END LOOP`, `CASE`/`END CASE`, `EXCEPTION`/`WHEN` to maintain + current indent depth (`depth ร— 2` spaces). + +2. **Split-line join** โ€” within each statement (tokens up to `;` at depth 0), a physical + line whose first non-whitespace token is at column 0 is a broken continuation; join it to + the preceding line with a single space. Exceptions: SQL clause keywords (`FROM`, `WHERE`, + `INTO`, `HAVING`, `GROUP`, `ORDER`, `RETURNING`) stay on their own line. + +3. **Base-indent normalisation** โ€” after joining, each logical line is re-emitted at + `depth ร— 2` spaces, stripping its original leading whitespace and replacing it with the + computed indent. Internal relative indentation of multi-line expressions is preserved. + +4. **Blank-line preservation** โ€” blank lines between statements in the original are kept + (they carry author intent about logical grouping). + +5. **Verbatim fallback** โ€” any construct the formatter cannot classify cleanly passes + through verbatim, upholding invariant #4. + +Acceptance: `pgtidy fmt testdata/corpus/test_a_broken.pgsql` produces output matching +`testdata/corpus/test_a.pgsql` (new golden-file test). ### โœ… Printer + style config โ€” `pkg/format`, `pkg/config` - `pkg/config`: `Style` struct + `Default()` = house style (UPPERCASE keywords, lowercase types, 2-space indent, leading commas, spacing rules). - `pkg/format`: formats CREATE FUNCTION/PROCEDURE **headers** to house style (params - one-per-line leading-comma, option clauses each on own line, AS/`$$` own lines); body and - `Raw` statements emitted verbatim. Spacing engine (`needSpace`, tight ops `:: : -> ->>`) - + casing (`keywords`/`typeNames` sets). Comment-safety: verbatim fallback if a header - carries comments it cannot relocate. -- Tests: golden header, idempotence, corpus idempotence + **semantic equivalence**. + one-per-line leading-comma, option clauses each on own line, AS/`$$` own lines); DECLARE + section formatted (see body parser entry); `Raw` statements emitted verbatim. + Spacing engine (`needSpace`, tight ops `:: : -> ->>`, array `[]`) + casing + (`keywords`/`typeNames` sets). Comment-safety: verbatim fallback if a header carries + comments it cannot relocate. +- `pkg/format/body.go`: DECLARE section formatter (see body parser entry). +- Tests: golden header, idempotence, corpus idempotence + **semantic equivalence** + (updated to recurse into dollar-quoted body tokens). - _Note: not a full Wadler Doc-IR yet โ€” fixed-layout printer. Doc-IR for width-based expression wrapping can come when DML structuring lands._