Skip to content

Commit 161e4ab

Browse files
jnasbyupgradeclaude
andcommitted
Add SQL style linter (vendored Postgres-Extensions/linter)
Vendor Postgres-Extensions/linter as a git submodule at .vendor/linter, following the same pattern already adopted in cat_tools: a thin self-initializing lint.mk hand-off (so `make lint` works right after a plain `git clone`, no --recurse-submodules needed), LINT_TARGETS scoped to sql/object_reference.sql and test/ (excluding the frozen, never hand-edited versioned install files under sql/, e.g. object_reference--0.1.0.sql/--stable.sql), and a CI job that runs `make lint` directly -- the same entry point a developer uses locally -- so the self-init logic is actually exercised, not just the rule checking. The `include lint.mk` is guarded on .git being present: a tarball build (PGXN distribution, `git archive` with no .git) has no submodule to initialize, and Make resolves every `include` before running any target regardless of which one was requested, so an unguarded rule would break `make`/`make install` entirely for a tarball build, not just `make lint`. Fixes the real pre-existing style findings this first run turned up (52 total): most were commented-out SQL marked as prose comments instead of using the linter's `EXCLUDED CODE` disabled-code convention (missing " * " prefixes flagged as comment-line-prefix/comment-opening violations); one COPY data block's `secondary` column intentionally mirrors pg_catalog's own type display name ("integer" for int4) rather than following prefer-short-type, so it's suppressed via a scoped disable-block region instead of being "fixed" into incorrect test data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e1f99f8 commit 161e4ab

11 files changed

Lines changed: 87 additions & 12 deletions

File tree

.github/workflows/ci.yml

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,24 @@ jobs:
9696
json=$(printf '%s\n' $supported | paste -sd, - | sed 's/^/[/; s/$/]/')
9797
echo "supported_pg=$json" >> "$GITHUB_OUTPUT"
9898
99+
# Style linter (https://github.com/Postgres-Extensions/linter, vendored at
100+
# .vendor/linter -- lint.mk is the thin local hand-off, see its comment).
101+
# Deliberately checked out WITHOUT submodules -- `make lint` is the same
102+
# command a developer runs locally, and lint.mk self-initializes the
103+
# submodule on first use. Using the exact same entry point here is what
104+
# actually proves that self-init works, rather than papering over it with
105+
# a submodules: true checkout.
106+
lint:
107+
needs: [changes]
108+
if: needs.changes.outputs.docs_only != 'true'
109+
name: 🧹 SQL Lint
110+
runs-on: ubuntu-latest
111+
steps:
112+
- name: Check out the repo
113+
uses: actions/checkout@v4
114+
- name: Lint SQL
115+
run: make lint
116+
99117
test:
100118
needs: [changes]
101119
if: needs.changes.outputs.docs_only != 'true'
@@ -128,7 +146,7 @@ jobs:
128146
# (e.g. test, on a docs-only push), and fails if any failed or were
129147
# cancelled.
130148
all-checks-passed:
131-
needs: [changes, test]
149+
needs: [changes, lint, test]
132150
if: always()
133151
runs-on: ubuntu-latest
134152
steps:
@@ -138,3 +156,24 @@ jobs:
138156
echo "One or more jobs failed or were cancelled"
139157
exit 1
140158
fi
159+
=======
160+
161+
on: [push, pull_request]
162+
163+
jobs:
164+
# Style linter (https://github.com/Postgres-Extensions/linter, vendored at
165+
# .vendor/linter -- lint.mk is the thin local hand-off, see its comment).
166+
# Deliberately checked out WITHOUT submodules -- `make lint` is the same
167+
# command a developer runs locally, and lint.mk self-initializes the
168+
# submodule on first use. Using the exact same entry point here is what
169+
# actually proves that self-init works, rather than papering over it with
170+
# a submodules: true checkout.
171+
lint:
172+
name: 🧹 SQL Lint
173+
runs-on: ubuntu-latest
174+
steps:
175+
- name: Check out the repo
176+
uses: actions/checkout@v4
177+
- name: Lint SQL
178+
run: make lint
179+
>>>>>>> 3eefe4c (Add SQL style linter (vendored Postgres-Extensions/linter))

.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 b40aaf7

Makefile

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,23 @@ test_factory: $(DESTDIR)$(datadir)/extension/test_factory.control
2525
$(DESTDIR)$(datadir)/extension/test_factory.control:
2626
pgxn install test_factory
2727

28+
29+
# Style linter (see https://github.com/Postgres-Extensions/linter, vendored
30+
# at .vendor/linter -- lint.mk is the thin local hand-off, see its comment).
31+
# Scoped to sql/object_reference.sql rather than the default `sql/ test/`:
32+
# the versioned install/update files under sql/ (object_reference--*.sql,
33+
# e.g. object_reference--0.1.0.sql/--stable.sql) are frozen once released and
34+
# never hand-edited again (see this repo's CLAUDE.md / memory), so linting
35+
# them would produce permanent, unfixable findings and make `make lint`
36+
# unusable as a CI gate.
37+
#
38+
# Guarded on .git being present: a tarball build (PGXN distribution, or any
39+
# `git archive` checkout with no .git) has no submodule to initialize, and
40+
# Make resolves every `include` before running any target regardless of
41+
# which target was requested -- so an unguarded self-init rule in lint.mk
42+
# would break `make`/`make install` entirely for a tarball build, not just
43+
# `make lint`.
44+
ifneq ($(wildcard .git),)
45+
LINT_TARGETS = sql/object_reference.sql test/
46+
include lint.mk
47+
endif

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/object_reference.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ BEGIN
1313
RAISE DEBUG 'search_path changed to %', current_setting('search_path');
1414
END
1515
$$;
16-
/*
16+
/* EXCLUDED CODE: schema-restriction check below not currently enforced
1717
DO $$
1818
DECLARE
1919
c_schema CONSTANT name := (SELECT extnamespace::regnamespace::text FROM pg_extension WHERE extname = 'cat_tools');
@@ -179,7 +179,7 @@ CREATE TABLE _object_reference.object(
179179
, object_names text[] NOT NULL
180180
, object_args text[] NOT NULL
181181
, CONSTRAINT object__u_object_names__object_args UNIQUE( object_type, object_names, object_args )
182-
/* TODO: this can't be a trigger because some objects won't exist when a dump is loaded
182+
/* EXCLUDED CODE: TODO: this can't be a trigger because some objects won't exist when a dump is loaded
183183
, CONSTRAINT object__address_sanity
184184
-- pg_get_object_address will throw an error if anything is wrong, so the IS NOT NULL is mostly pointless
185185
CHECK( pg_catalog.pg_get_object_address(object_type::text, object_names, object_args) IS NOT NULL )
@@ -192,7 +192,7 @@ GRANT REFERENCES ON _object_reference.object TO object_reference__dependency;
192192
CREATE TABLE _object_reference._object_oid(
193193
object_id int PRIMARY KEY REFERENCES _object_reference.object ON DELETE CASCADE ON UPDATE CASCADE
194194
, classid regclass NOT NULL
195-
/* TODO: needs to be a trigger
195+
/* EXCLUDED CODE: TODO: needs to be a trigger
196196
CONSTRAINT classid_must_match__object__address_classid
197197
CHECK( classid IS NOT DISTINCT FROM cat_tools.object__address_classid(object_type) )
198198
*/
@@ -1258,7 +1258,7 @@ BEGIN
12581258
RETURN c_next_level;
12591259

12601260
EXCEPTION WHEN undefined_table THEN
1261-
/*
1261+
/* EXCLUDED CODE
12621262
CREATE TEMP TABLE __object_reference__ddl_capture AS
12631263
SELECT c_next_level, capture__start.object_group_id
12641264
;

test/deps.sql

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22

33
-- Add any test dependency statements here
44

5-
/*
6-
* Normally these should be loaded by the cascade!
5+
/* EXCLUDED CODE: normally these should be loaded by the cascade!
76
CREATE EXTENSION IF NOT EXISTS count_nulls;
87
CREATE EXTENSION IF NOT EXISTS cat_tools;
98
*/

test/helpers/object_table.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ INSERT INTO test_prereq VALUES
144144
;
145145

146146
-- \N is null character
147+
-- sql-lint:disable-block prefer-short-type: secondary column mirrors pg_catalog's own type display name (format_type), not a style choice
147148
COPY test_object(object_type, object_name, secondary, create_command, drop_command) FROM STDIN (DELIMITER '|');
148149
table|test table||%("test column" int)|
149150
index|test table test index||%ON "test table"("test column")|
@@ -159,8 +160,9 @@ cast|test type|integer|CREATE CAST ("test type" AS int4) WITH INOUT|DROP CAST ("
159160
default value|test table|test column|ALTER TABLE "test table" ALTER "test column" SET DEFAULT 0|ALTER TABLE "test table" ALTER "test column" DROP DEFAULT
160161
trigger|test table|test trigger|CREATE TRIGGER "test trigger" AFTER INSERT ON "test table" FOR EACH ROW EXECUTE PROCEDURE tg_null()|DROP TRIGGER "test trigger" ON "test table"
161162
\.
163+
-- sql-lint:enable-block
162164

163-
/* Not supported
165+
/* EXCLUDED CODE: Not supported
164166
composite type|test complex type||CREATE TYPE "test complex type" AS(r real, i real)|DROP TYPE "test complex type"
165167
view column|test view|test column|\N|\N
166168
materialized view column|test materialized view 2|test materialized view column|CREATE MATERIALIZED VIEW "test materialized view 2" AS SELECT (1,2)::"test complex type" AS "test materialized view column"|DROP MATERIALIZED VIEW "test materialized view 2"

test/sql/capture.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ SELECT bag_eq(
134134
, $$SELECT object_id FROM obj_ref$$
135135
, 'Verify captured object IDs match'
136136
);
137-
/*
137+
/* EXCLUDED CODE
138138
SELECT * FROM og_o;
139139
SELECT * FROM _object_reference.object;-- WHERE object_id IN(6,9);
140140
*/

test/sql/event_trigger.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ $body$;
143143

144144

145145
/*
146-
*Rename column
146+
* Rename column
147147
*/
148148
SELECT lives_ok(
149149
$$ALTER TABLE table_under_test RENAME column_test TO test_column2$$

0 commit comments

Comments
 (0)