From 04711cf7b21b509cba0bdfacbed9bc0cd61ab37c Mon Sep 17 00:00:00 2001 From: Hein Date: Wed, 1 Jul 2026 12:53:25 +0200 Subject: [PATCH] fix(cmd): handle errors and improve output formatting * update error handling in various commands to use blank identifier * enhance output formatting for better readability * add golangci-lint to Makefile for linting checks --- .golangci.yml | 4 ++++ Makefile | 3 ++- cmd/pgtidy/config.go | 16 ++++++++-------- cmd/pgtidy/fmt.go | 20 ++++++++++---------- cmd/pgtidy/lint.go | 20 ++++++++++---------- cmd/pgtidy/lsp.go | 4 ++-- cmd/pgtidy/main.go | 6 +++--- pkg/config/config.go | 36 ++++++++++++++++++------------------ pkg/format/dml.go | 25 +++++++++---------------- pkg/format/keywords.go | 6 +++--- pkg/lsp/server.go | 14 +++++++------- pkg/lsp/server_test.go | 4 ++-- pkg/parser/parser.go | 2 +- pkg/pgast/pgast.go | 2 +- 14 files changed, 80 insertions(+), 82 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..ca457a2 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,4 @@ +version: "2" + +linters: + default: standard diff --git a/Makefile b/Makefile index 0b33838..5b26b93 100644 --- a/Makefile +++ b/Makefile @@ -20,9 +20,10 @@ vet: fmt: go fmt ./... -## lint: vet + format check +## lint: vet + format check + golangci-lint lint: vet test -z "$$(gofmt -l .)" || (echo "gofmt needed:"; gofmt -l .; exit 1) + golangci-lint run ./... ## clean: remove build artifacts clean: diff --git a/cmd/pgtidy/config.go b/cmd/pgtidy/config.go index 231ac9e..fc01dd0 100644 --- a/cmd/pgtidy/config.go +++ b/cmd/pgtidy/config.go @@ -11,7 +11,7 @@ import ( func cmdConfig(args []string, stdin io.Reader, stdout, stderr io.Writer) int { for _, a := range args { if a == "-h" || a == "--help" { - fmt.Fprintln(stdout, "pgtidy config — print effective configuration resolved from .pgtidy.yaml") + _, _ = fmt.Fprintln(stdout, "pgtidy config — print effective configuration resolved from .pgtidy.yaml") return 0 } } @@ -21,14 +21,14 @@ func cmdConfig(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } st, err := config.Load(wd) if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) return 2 } - fmt.Fprintf(stdout, "indent: %q\n", st.Indent) - fmt.Fprintf(stdout, "newline: %q\n", st.Newline) - fmt.Fprintf(stdout, "keyword_case: %s\n", st.KeywordCase) - fmt.Fprintf(stdout, "ident_case: %s\n", st.IdentCase) - fmt.Fprintf(stdout, "type_case: %s\n", st.TypeCase) - fmt.Fprintf(stdout, "commas: %s\n", st.Commas) + _, _ = fmt.Fprintf(stdout, "indent: %q\n", st.Indent) + _, _ = fmt.Fprintf(stdout, "newline: %q\n", st.Newline) + _, _ = fmt.Fprintf(stdout, "keyword_case: %s\n", st.KeywordCase) + _, _ = fmt.Fprintf(stdout, "ident_case: %s\n", st.IdentCase) + _, _ = fmt.Fprintf(stdout, "type_case: %s\n", st.TypeCase) + _, _ = fmt.Fprintf(stdout, "commas: %s\n", st.Commas) return 0 } diff --git a/cmd/pgtidy/fmt.go b/cmd/pgtidy/fmt.go index 66c4389..2905db5 100644 --- a/cmd/pgtidy/fmt.go +++ b/cmd/pgtidy/fmt.go @@ -37,7 +37,7 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return 0 default: if len(a) > 1 && a[0] == '-' { - fmt.Fprintf(stderr, "pgtidy fmt: unknown flag %q\n", a) + _, _ = fmt.Fprintf(stderr, "pgtidy fmt: unknown flag %q\n", a) return 2 } files = append(files, a) @@ -51,7 +51,7 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } st, err := config.Load(wd) if err != nil { - fmt.Fprintf(stderr, "%v\n", err) + _, _ = fmt.Fprintf(stderr, "%v\n", err) return 2 } @@ -59,7 +59,7 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if len(files) == 0 { src, err := io.ReadAll(stdin) if err != nil { - fmt.Fprintf(stderr, "pgtidy: reading stdin: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: reading stdin: %v\n", err) return 2 } out := format.File(parser.Parse(string(src)), st) @@ -69,9 +69,9 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return 1 } case diff: - io.WriteString(stdout, unifiedDiff(string(src), out, "stdin")) + _, _ = io.WriteString(stdout, unifiedDiff(string(src), out, "stdin")) default: - io.WriteString(stdout, out) + _, _ = io.WriteString(stdout, out) } return 0 } @@ -81,7 +81,7 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { for _, path := range files { src, err := os.ReadFile(path) if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) exit = 2 continue } @@ -94,22 +94,22 @@ func cmdFmt(args []string, stdin io.Reader, stdout, stderr io.Writer) int { case write: if changed { if err := os.WriteFile(path, []byte(out), 0o644); err != nil { - fmt.Fprintf(stderr, "pgtidy: writing %s: %v\n", path, err) + _, _ = fmt.Fprintf(stderr, "pgtidy: writing %s: %v\n", path, err) exit = 2 } } case list: if changed { - fmt.Fprintln(stdout, path) + _, _ = fmt.Fprintln(stdout, path) } case diff: if changed { - io.WriteString(stdout, unifiedDiff(string(src), out, path)) + _, _ = io.WriteString(stdout, unifiedDiff(string(src), out, path)) } case check: // handled after loop via anyDiff default: - io.WriteString(stdout, out) + _, _ = io.WriteString(stdout, out) } } if check && anyDiff && exit == 0 { diff --git a/cmd/pgtidy/lint.go b/cmd/pgtidy/lint.go index c2c0aa3..1a12689 100644 --- a/cmd/pgtidy/lint.go +++ b/cmd/pgtidy/lint.go @@ -37,7 +37,7 @@ func cmdLint(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } } case len(a) > 1 && a[0] == '-': - fmt.Fprintf(stderr, "pgtidy lint: unknown flag %q\n", a) + _, _ = fmt.Fprintf(stderr, "pgtidy lint: unknown flag %q\n", a) return 2 default: files = append(files, a) @@ -75,7 +75,7 @@ func cmdLint(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if loc == "" { loc = "stdin" } - fmt.Fprintf(stdout, "%s: [%s] %s: %s\n", loc, d.RuleID, d.Severity, d.Message) + _, _ = fmt.Fprintf(stdout, "%s: [%s] %s: %s\n", loc, d.RuleID, d.Severity, d.Message) } } @@ -84,18 +84,18 @@ func cmdLint(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if len(files) == 0 { src, err := io.ReadAll(stdin) if err != nil { - fmt.Fprintf(stderr, "pgtidy: reading stdin: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: reading stdin: %v\n", err) return 2 } diags, err := check(string(src), "") if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) return 2 } if fix { fixed := lint.ApplyFixes(string(src), diags) if fixed != string(src) { - io.WriteString(stdout, fixed) + _, _ = io.WriteString(stdout, fixed) return 0 } } @@ -107,25 +107,25 @@ func cmdLint(args []string, stdin io.Reader, stdout, stderr io.Writer) int { for _, path := range files { src, err := os.ReadFile(path) if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) return 2 } diags, err := check(string(src), path) if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) return 2 } if fix { fixed := lint.ApplyFixes(string(src), diags) if fixed != string(src) { if err := os.WriteFile(path, []byte(fixed), 0o644); err != nil { - fmt.Fprintf(stderr, "pgtidy: writing %s: %v\n", path, err) + _, _ = fmt.Fprintf(stderr, "pgtidy: writing %s: %v\n", path, err) return 2 } // Re-check to report any remaining unfixed diagnostics. diags, err = check(fixed, path) if err != nil { - fmt.Fprintf(stderr, "pgtidy: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy: %v\n", err) return 2 } } @@ -144,7 +144,7 @@ func cmdLint(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } func lintUsage(w io.Writer) { - fmt.Fprint(w, `Usage: pgtidy lint [flags] [file ...] + _, _ = fmt.Fprint(w, `Usage: pgtidy lint [flags] [file ...] Read SQL from files (or stdin) and report lint findings. diff --git a/cmd/pgtidy/lsp.go b/cmd/pgtidy/lsp.go index 936c07f..32bb119 100644 --- a/cmd/pgtidy/lsp.go +++ b/cmd/pgtidy/lsp.go @@ -12,7 +12,7 @@ import ( func cmdLsp(args []string, stdin io.Reader, stdout, stderr io.Writer) int { for _, a := range args { if a == "-h" || a == "--help" { - fmt.Fprintln(stdout, "pgtidy lsp — start the Language Server Protocol server (stdio transport)") + _, _ = fmt.Fprintln(stdout, "pgtidy lsp — start the Language Server Protocol server (stdio transport)") return 0 } } @@ -21,7 +21,7 @@ func cmdLsp(args []string, stdin io.Reader, stdout, stderr io.Writer) int { wd = "." } if err := lsp.Serve(context.Background(), stdin, stdout, wd); err != nil { - fmt.Fprintf(stderr, "pgtidy lsp: %v\n", err) + _, _ = fmt.Fprintf(stderr, "pgtidy lsp: %v\n", err) return 1 } return 0 diff --git a/cmd/pgtidy/main.go b/cmd/pgtidy/main.go index 04cdae5..7396767 100644 --- a/cmd/pgtidy/main.go +++ b/cmd/pgtidy/main.go @@ -29,20 +29,20 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { case "config": return cmdConfig(args[1:], stdin, stdout, stderr) case "version", "--version", "-v": - fmt.Fprintf(stdout, "pgtidy %s\n", version) + _, _ = fmt.Fprintf(stdout, "pgtidy %s\n", version) return 0 case "help", "-h", "--help": usage(stdout) return 0 default: - fmt.Fprintf(stderr, "pgtidy: unknown command %q\n", args[0]) + _, _ = fmt.Fprintf(stderr, "pgtidy: unknown command %q\n", args[0]) usage(stderr) return 2 } } func usage(w io.Writer) { - fmt.Fprint(w, `pgtidy — PostgreSQL formatter and linter + _, _ = fmt.Fprint(w, `pgtidy — PostgreSQL formatter and linter Usage: pgtidy fmt [flags] [files...] Format SQL/PL-pgSQL (stdin if no files) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7a7b59a..6db8828 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -61,21 +61,21 @@ type Style struct { CustomTypeCase Case // user-defined / domain types not in the built-in set // --- Query layout --- - Commas CommaStyle - AlignColumns bool // pad SELECT list so values align - AlignLineComments bool // align trailing -- comments in a block - SelectAlignAs bool // pad between expression and AS in SELECT list - SetAlignEqual bool // align = in UPDATE SET list - IndentJoin bool // extra indentation for JOIN … ON lines - JoinIndentSize int // extra indent levels for JOINs (default 1) - WhereWrap WrapMode // always|when_long|never — each AND/OR on its own line - WhereAndOrIndent bool // AND/OR indented one level under WHERE + Commas CommaStyle + AlignColumns bool // pad SELECT list so values align + AlignLineComments bool // align trailing -- comments in a block + SelectAlignAs bool // pad between expression and AS in SELECT list + SetAlignEqual bool // align = in UPDATE SET list + IndentJoin bool // extra indentation for JOIN … ON lines + JoinIndentSize int // extra indent levels for JOINs (default 1) + WhereWrap WrapMode // always|when_long|never — each AND/OR on its own line + WhereAndOrIndent bool // AND/OR indented one level under WHERE // --- Subqueries --- - SubqueryOpening Placement // opening ( placement: same_line|new_line - SubqueryContent Placement // content indentation: same_line|new_line - SubqueryClosing Placement // closing ) placement: same_line|new_line - SubquerySpaceBeforeParen bool // space before ( in subqueries + SubqueryOpening Placement // opening ( placement: same_line|new_line + SubqueryContent Placement // content indentation: same_line|new_line + SubqueryClosing Placement // closing ) placement: same_line|new_line + SubquerySpaceBeforeParen bool // space before ( in subqueries // --- INSERT --- InsertCollapseValues bool // fold multiple VALUES rows onto fewer lines @@ -85,11 +85,11 @@ type Style struct { RoutineAsWrap bool // newline before AS $$ // --- PL/pgSQL body --- - PlpgsqlMaxBlankLines int // max consecutive blank lines in body - PlpgsqlDeclareAlignType bool // align type column in DECLARE block - PlpgsqlDeclareAlignEq bool // align := / = in DECLARE block - PlpgsqlIfThenNewline bool // THEN on its own line - PlpgsqlLoopCollapse bool // collapse empty loop bodies to one line + PlpgsqlMaxBlankLines int // max consecutive blank lines in body + PlpgsqlDeclareAlignType bool // align type column in DECLARE block + PlpgsqlDeclareAlignEq bool // align := / = in DECLARE block + PlpgsqlIfThenNewline bool // THEN on its own line + PlpgsqlLoopCollapse bool // collapse empty loop bodies to one line // --- Expressions --- BinaryOpAlign bool // align =, <>, || etc. vertically in WHERE/expr lists diff --git a/pkg/format/dml.go b/pkg/format/dml.go index f19d85c..81af369 100644 --- a/pkg/format/dml.go +++ b/pkg/format/dml.go @@ -10,16 +10,13 @@ import ( // isDMLStart reports whether toks begins with a DML statement keyword. func isDMLStart(toks []cst.Tok) bool { - for _, t := range toks { - if t.Tok.Kind == lexer.Ident { - switch lowerASCII(t.Tok.Text) { - case "select", "insert", "update", "delete", "with": - return true - } - return false - } + if len(toks) == 0 || toks[0].Tok.Kind != lexer.Ident { return false } + switch lowerASCII(toks[0].Tok.Text) { + case "select", "insert", "update", "delete", "with": + return true + } return false } @@ -510,13 +507,6 @@ func dmlSplitCommas(toks []cst.Tok) [][]cst.Tok { return items } -// dmlColList formats kwText followed by a comma-separated body. -// One item: kept on the same line as the keyword. -// Multiple items: each on its own line with the configured comma style. -func dmlColList(kwText string, items [][]cst.Tok, st config.Style) string { - return dmlColListSelect(kwText, items, st) -} - // dmlColListSelect formats a SELECT / RETURNING column list with optional // align_columns and select_align_as settings. func dmlColListSelect(kwText string, items [][]cst.Tok, st config.Style) string { @@ -627,7 +617,10 @@ func dmlColListSet(kwText string, items [][]cst.Tok, st config.Style) string { // alias names align vertically. func alignSelectItems(texts []string, st config.Style) []string { // Split each text into (expr, " AS ", alias) or keep as-is. - type part struct{ expr, alias string; hasAs bool } + type part struct { + expr, alias string + hasAs bool + } parts := make([]part, len(texts)) maxExpr := 0 for i, t := range texts { diff --git a/pkg/format/keywords.go b/pkg/format/keywords.go index 1049ed6..feca6aa 100644 --- a/pkg/format/keywords.go +++ b/pkg/format/keywords.go @@ -68,6 +68,6 @@ to_char to_date to_json to_jsonb to_number to_timestamp to_tsvector translate trim trunc unnest upper width_bucket `) -func isKeyword(lower string) bool { return keywords[lower] } -func isTypeName(lower string) bool { return typeNames[lower] } -func isBuiltinFunc(lower string) bool { return builtinFunctions[lower] } +func isKeyword(lower string) bool { return keywords[lower] } +func isTypeName(lower string) bool { return typeNames[lower] } +func isBuiltinFunc(lower string) bool { return builtinFunctions[lower] } diff --git a/pkg/lsp/server.go b/pkg/lsp/server.go index abfdaf5..aea1c8e 100644 --- a/pkg/lsp/server.go +++ b/pkg/lsp/server.go @@ -89,12 +89,12 @@ func (s *server) handle(raw []byte) bool { return true case "textDocument/didOpen": var p didOpenParams - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) s.docs[p.TextDocument.URI] = p.TextDocument.Text s.pushDiagnostics(p.TextDocument.URI, p.TextDocument.Text) case "textDocument/didChange": var p didChangeParams - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) if len(p.ContentChanges) > 0 { text := p.ContentChanges[len(p.ContentChanges)-1].Text s.docs[p.TextDocument.URI] = text @@ -104,7 +104,7 @@ func (s *server) handle(raw []byte) bool { var p struct { TextDocument textDocID `json:"textDocument"` } - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) delete(s.docs, p.TextDocument.URI) delete(s.fixes, p.TextDocument.URI) s.notify("textDocument/publishDiagnostics", publishDiagnosticsParams{ @@ -113,7 +113,7 @@ func (s *server) handle(raw []byte) bool { }) case "textDocument/formatting": var p formattingParams - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) text, ok := s.docs[p.TextDocument.URI] if !ok { s.reply(req.ID, []textEdit{}) @@ -127,7 +127,7 @@ func (s *server) handle(raw []byte) bool { s.reply(req.ID, []textEdit{fullReplace(text, formatted)}) case "textDocument/rangeFormatting": var p rangeFormattingParams - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) text, ok := s.docs[p.TextDocument.URI] if !ok { s.reply(req.ID, []textEdit{}) @@ -136,7 +136,7 @@ func (s *server) handle(raw []byte) bool { s.reply(req.ID, s.rangeFormat(text, p.Range)) case "textDocument/codeAction": var p codeActionParams - json.Unmarshal(req.Params, &p) + _ = json.Unmarshal(req.Params, &p) s.handleCodeAction(req.ID, p) case "$/cancelRequest": // ignore default: @@ -360,7 +360,7 @@ func (s *server) send(v interface{}) { if err != nil { return } - fmt.Fprintf(s.w, "Content-Length: %d\r\n\r\n", len(data)) + _, _ = fmt.Fprintf(s.w, "Content-Length: %d\r\n\r\n", len(data)) s.w.Write(data) //nolint:errcheck } diff --git a/pkg/lsp/server_test.go b/pkg/lsp/server_test.go index 7499836..c322247 100644 --- a/pkg/lsp/server_test.go +++ b/pkg/lsp/server_test.go @@ -51,7 +51,7 @@ func readResp(t *testing.T, buf *bytes.Buffer) map[string]interface{} { } lenStr := string(data[idx+16 : idx+eol]) var n int - fmt.Sscanf(lenStr, "%d", &n) + _, _ = fmt.Sscanf(lenStr, "%d", &n) sep := bytes.Index(data, []byte("\r\n\r\n")) if sep < 0 || len(data) < sep+4+n { time.Sleep(5 * time.Millisecond) @@ -60,7 +60,7 @@ func readResp(t *testing.T, buf *bytes.Buffer) map[string]interface{} { body := data[sep+4 : sep+4+n] buf.Next(sep + 4 + n) var result map[string]interface{} - json.Unmarshal(body, &result) + _ = json.Unmarshal(body, &result) return result } t.Fatal("timeout waiting for response") diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 341adca..807ef89 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -88,7 +88,7 @@ func parseCreateFunction(stmt []cst.Tok) (*cst.CreateFunction, bool) { if i+1 < len(stmt) && stmt[i].Is("or") && stmt[i+1].Is("replace") { i += 2 } - if i >= len(stmt) || !(stmt[i].Is("function") || stmt[i].Is("procedure")) { + if i >= len(stmt) || (!stmt[i].Is("function") && !stmt[i].Is("procedure")) { return nil, false } i++ diff --git a/pkg/pgast/pgast.go b/pkg/pgast/pgast.go index a5c5a52..f257876 100644 --- a/pkg/pgast/pgast.go +++ b/pkg/pgast/pgast.go @@ -41,7 +41,7 @@ func FirstTokenOffset(sql string, start int) int { } case i+1 < n && sql[i] == '/' && sql[i+1] == '*': i += 2 - for i+1 < n && !(sql[i] == '*' && sql[i+1] == '/') { + for i+1 < n && (sql[i] != '*' || sql[i+1] != '/') { i++ } if i+1 < n {