Skip to content

Commit b49b4bd

Browse files
Add sql-lint: style linter for PostgreSQL SQL files (#49)
Add a linter, vendored as a git submodule at `.vendor/linter` (https://github.com/Postgres-Extensions/linter) . Add to CI, and fix all linting issues.
1 parent 5a4729c commit b49b4bd

9 files changed

Lines changed: 152 additions & 83 deletions

File tree

.github/workflows/ci.yml

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,26 @@ jobs:
308308
# (pgxntool marks installcheck `.IGNORE`), so failures would pass silently.
309309
make verify-results
310310
311+
# Style linter (https://github.com/Postgres-Extensions/linter, vendored at
312+
# .vendor/linter). Deliberately checked out WITHOUT submodules -- `make
313+
# lint` is the same command a developer runs locally, and lint.mk
314+
# self-initializes the submodule on first use (see its comment). Using the
315+
# exact same entry point here is what actually proves that self-init works,
316+
# rather than papering over it with a submodules: true checkout. The
317+
# linter's own test suite (fixtures + scanner edge cases) is that repo's
318+
# own CI's job, not this one's. No PostgreSQL needed -- sql-lint is a
319+
# standalone Perl script -- so this doesn't use the pgxn-tools container.
320+
lint:
321+
needs: [changes]
322+
if: needs.changes.outputs.docs_only != 'true'
323+
name: 🧹 SQL Lint
324+
runs-on: ubuntu-latest
325+
steps:
326+
- name: Check out the repo
327+
uses: actions/checkout@v6
328+
- name: Lint SQL
329+
run: make lint
330+
311331
# Proves cat_tools survives a BINARY pg_upgrade (in-place catalog migration to a
312332
# newer PostgreSQL major), not just an in-place extension update. Each leg is a
313333
# SINGLE jump: install an old cat_tools on an old cluster, plant a dependency
@@ -872,7 +892,7 @@ jobs:
872892
# the heavy jobs gated off by the `changes` job on a docs-only push), and
873893
# fails if any failed or were cancelled.
874894
all-checks-passed:
875-
needs: [changes, test, pg-upgrade-test, pg-upgrade-stepwise, extension-update-test, pg-tle-test, pg-tle-upgrade-test]
895+
needs: [changes, test, lint, pg-upgrade-test, pg-upgrade-stepwise, extension-update-test, pg-tle-test, pg-tle-upgrade-test]
876896
if: always()
877897
runs-on: ubuntu-latest
878898
steps:

.gitmodules

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[submodule ".vendor/linter"]
2+
path = .vendor/linter
3+
url = https://github.com/Postgres-Extensions/linter.git

.vendor/linter

Submodule linter added at b8632c2

Makefile

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,16 @@ $(DESTDIR)$(datadir)/extension/cat_tools--0.2.0.sql:
9292
.PHONY: clean_old_version
9393
clean_old_version:
9494
pgxn uninstall --unstable 'cat_tools=0.2.0'
95+
96+
# Style linter (see https://github.com/Postgres-Extensions/linter, vendored
97+
# at .vendor/linter -- lint.mk is the thin local hand-off, see its comment).
98+
# Scoped to the actively-maintained source rather than the default
99+
# `sql/ test/`: version-specific install/update scripts under sql/ are
100+
# frozen once released (SQL file conventions rule 5 in CLAUDE.md — never
101+
# hand-edited again), so linting them would produce permanent, unfixable
102+
# findings and make `make lint` unusable as a CI gate. Lint the current
103+
# source instead; a version file still under active development can be
104+
# linted directly, e.g.
105+
# `.vendor/linter/sql/bin/sql-lint sql/cat_tools--0.3.0.sql.in`.
106+
LINT_TARGETS = sql/cat_tools.sql.in sql/omit_column.sql test/
107+
include lint.mk

lint.mk

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# lint.mk — thin wrapper; the whole local footprint for consuming
2+
# https://github.com/Postgres-Extensions/linter. Everything else lives in
3+
# the .vendor/linter submodule; see its README for available targets/rules.
4+
#
5+
# Self-initializing (via the rule below) so `make lint` works right after a
6+
# plain `git clone`, with no --recurse-submodules needed, and so CI can rely
7+
# on the exact same entry point a developer would use locally.
8+
.vendor/linter/lint.mk:
9+
git submodule update --init -- .vendor/linter
10+
11+
include .vendor/linter/lint.mk

sql.mk

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -199,23 +199,51 @@ define _apply_version_seds
199199
{print}' $@.tmp > $@.tmp2 && mv $@.tmp2 $@.tmp
200200
endef
201201

202-
# The recipe builds $@.tmp then moves it into place atomically.
202+
# ----------------------------------------------------------------------------
203+
# Two .sql.in sources feed the same .sql.in -> .sql transform below (wrap with
204+
# @generated@ markers, then _apply_version_seds), producing two different
205+
# targets:
206+
#
207+
# sql/cat_tools.sql.in (hand-maintained master, committed)
208+
# --pattern rule--> sql/cat_tools.sql
209+
#
210+
# sql/cat_tools.sql.in
211+
# --copy rule, tags @generated@ as "VERSIONED FILE!"-->
212+
# sql/cat_tools--X.Y.Z.sql.in (committed per-version snapshot; frozen
213+
# once released, see CLAUDE.md's SQL file conventions)
214+
# --override rule, same transform as the pattern rule-->
215+
# sql/cat_tools--X.Y.Z.sql = $(EXTENSION_VERSION_FILES)
216+
# (what CREATE EXTENSION actually installs)
217+
#
218+
# The bottom (override) rule can't just be left to the sql/%.sql pattern rule
219+
# matching it: control.mk (auto-generated by pgxntool from cat_tools.control,
220+
# see pgxntool/base.mk) already defines its OWN recipe for
221+
# $(EXTENSION_VERSION_FILES) -- straight from cat_tools.sql, no .sql.in layer,
222+
# no version seds -- and GNU Make always prefers an explicit rule over a
223+
# pattern rule for the same target. Overriding it here is the only way to
224+
# route that target through the same .sql.in / version-sed pipeline as
225+
# everything else; its "overriding recipe" warning is expected. Both final
226+
# steps build $@.tmp then move it into place atomically.
227+
#
203228
# TODO: refactor the version handling into a function.
229+
# ----------------------------------------------------------------------------
230+
231+
# @generated@ becomes the "-- GENERATED FILE! DO NOT EDIT!" marker below via a
232+
# plain, unanchored substring match -- it also fires on a handful of
233+
# coincidental @generated@ occurrences inside real-code comments in
234+
# cat_tools.sql.in, which is harmless (already inside a -- comment).
204235
sql/%.sql: sql/%.sql.in pgxntool/safesed
205236
(echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp
206237
$(_apply_version_seds)
207238
mv $@.tmp $@
208239

209-
# Make the current version's .sql.in by copying the base source; the pattern
210-
# rule above then turns it into the final .sql with SED substitutions applied.
211-
# (EXTENSION_VERSION_FILES is just sql/cat_tools--<current version>.sql)
240+
# Appends " VERSIONED FILE!" after every @generated@ occurrence; the pattern
241+
# rule above resolves the leading @generated@ either way, so the tag rides
242+
# through into the final marker text untouched.
212243
$(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control
213-
cp $< $@
244+
sed -e 's/@generated@/@generated@ VERSIONED FILE!/' $< > $@
214245

215-
# Override control.mk's rule (which builds EXTENSION_VERSION_FILES straight from
216-
# cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with
217-
# version-conditional substitutions applied. GNU Make's "overriding recipe"
218-
# warning for this target is expected.
246+
# See the overview above for why this duplicates the pattern rule's recipe.
219247
$(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed
220248
(echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp
221249
$(_apply_version_seds)

0 commit comments

Comments
 (0)