From 3eefe4ce13ffc73d8ecc5fe718290d85f4ca6601 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 18:35:56 -0500 Subject: [PATCH] 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 --- .github/workflows/ci.yml | 20 ++++++++++++++++++++ .gitmodules | 3 +++ .vendor/linter | 1 + Makefile | 20 ++++++++++++++++++++ lint.mk | 11 +++++++++++ sql/object_reference--stable.sql | 8 ++++---- sql/object_reference.sql | 8 ++++---- test/deps.sql | 3 +-- test/helpers/object_table.sql | 4 +++- test/sql/capture.sql | 2 +- test/sql/event_trigger.sql | 2 +- test/sql/object_group.sql | 4 ++-- 12 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .gitmodules create mode 160000 .vendor/linter create mode 100644 lint.mk diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4b3e970 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,20 @@ +name: CI + +on: [push, pull_request] + +jobs: + # Style linter (https://github.com/Postgres-Extensions/linter, vendored at + # .vendor/linter -- lint.mk is the thin local hand-off, see its comment). + # Deliberately checked out WITHOUT submodules -- `make lint` is the same + # command a developer runs locally, and lint.mk self-initializes the + # submodule on first use. Using the exact same entry point here is what + # actually proves that self-init works, rather than papering over it with + # a submodules: true checkout. + lint: + name: 🧹 SQL Lint + runs-on: ubuntu-latest + steps: + - name: Check out the repo + uses: actions/checkout@v4 + - name: Lint SQL + run: make lint diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..9443c64 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule ".vendor/linter"] + path = .vendor/linter + url = https://github.com/Postgres-Extensions/linter.git diff --git a/.vendor/linter b/.vendor/linter new file mode 160000 index 0000000..b40aaf7 --- /dev/null +++ b/.vendor/linter @@ -0,0 +1 @@ +Subproject commit b40aaf70be8af80f048da777e551c5b790bd9e69 diff --git a/Makefile b/Makefile index 9ae665d..006453e 100644 --- a/Makefile +++ b/Makefile @@ -32,3 +32,23 @@ test_factory: $(DESTDIR)$(datadir)/extension/test_factory.control $(DESTDIR)$(datadir)/extension/test_factory.control: pgxn install test_factory + +# Style linter (see https://github.com/Postgres-Extensions/linter, vendored +# at .vendor/linter -- lint.mk is the thin local hand-off, see its comment). +# Scoped to sql/object_reference.sql rather than the default `sql/ test/`: +# the versioned install/update files under sql/ (object_reference--*.sql, +# e.g. object_reference--0.1.0.sql/--stable.sql) are frozen once released and +# never hand-edited again (see this repo's CLAUDE.md / memory), so linting +# them would produce permanent, unfixable findings and make `make lint` +# unusable as a CI gate. +# +# Guarded on .git being present: a tarball build (PGXN distribution, or any +# `git archive` checkout with no .git) has no submodule to initialize, and +# Make resolves every `include` before running any target regardless of +# which target was requested -- so an unguarded self-init rule in lint.mk +# would break `make`/`make install` entirely for a tarball build, not just +# `make lint`. +ifneq ($(wildcard .git),) +LINT_TARGETS = sql/object_reference.sql test/ +include lint.mk +endif diff --git a/lint.mk b/lint.mk new file mode 100644 index 0000000..0d18abf --- /dev/null +++ b/lint.mk @@ -0,0 +1,11 @@ +# lint.mk — thin wrapper; the whole local footprint for consuming +# https://github.com/Postgres-Extensions/linter. Everything else lives in +# the .vendor/linter submodule; see its README for available targets/rules. +# +# Self-initializing (via the rule below) so `make lint` works right after a +# plain `git clone`, with no --recurse-submodules needed, and so CI can rely +# on the exact same entry point a developer would use locally. +.vendor/linter/lint.mk: + git submodule update --init -- .vendor/linter + +include .vendor/linter/lint.mk diff --git a/sql/object_reference--stable.sql b/sql/object_reference--stable.sql index 4a68ebc..5333ba2 100644 --- a/sql/object_reference--stable.sql +++ b/sql/object_reference--stable.sql @@ -14,7 +14,7 @@ BEGIN RAISE DEBUG 'search_path changed to %', current_setting('search_path'); END $$; -/* +/* EXCLUDED CODE: schema-restriction check below not currently enforced DO $$ DECLARE c_schema CONSTANT name := (SELECT extnamespace::regnamespace::text FROM pg_extension WHERE extname = 'cat_tools'); @@ -180,7 +180,7 @@ CREATE TABLE _object_reference.object( , object_names text[] NOT NULL , object_args text[] NOT NULL , CONSTRAINT object__u_object_names__object_args UNIQUE( object_type, object_names, object_args ) - /* TODO: this can't be a trigger because some objects won't exist when a dump is loaded + /* EXCLUDED CODE: TODO: this can't be a trigger because some objects won't exist when a dump is loaded , CONSTRAINT object__address_sanity -- pg_get_object_address will throw an error if anything is wrong, so the IS NOT NULL is mostly pointless CHECK( pg_catalog.pg_get_object_address(object_type::text, object_names, object_args) IS NOT NULL ) @@ -193,7 +193,7 @@ GRANT REFERENCES ON _object_reference.object TO object_reference__dependency; CREATE TABLE _object_reference._object_oid( object_id int PRIMARY KEY REFERENCES _object_reference.object ON DELETE CASCADE ON UPDATE CASCADE , classid regclass NOT NULL - /* TODO: needs to be a trigger + /* EXCLUDED CODE: TODO: needs to be a trigger CONSTRAINT classid_must_match__object__address_classid CHECK( classid IS NOT DISTINCT FROM cat_tools.object__address_classid(object_type) ) */ @@ -1254,7 +1254,7 @@ BEGIN RETURN c_next_level; EXCEPTION WHEN undefined_table THEN - /* + /* EXCLUDED CODE CREATE TEMP TABLE __object_reference__ddl_capture AS SELECT c_next_level, capture__start.object_group_id ; diff --git a/sql/object_reference.sql b/sql/object_reference.sql index e83b461..bc4ee0d 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -13,7 +13,7 @@ BEGIN RAISE DEBUG 'search_path changed to %', current_setting('search_path'); END $$; -/* +/* EXCLUDED CODE: schema-restriction check below not currently enforced DO $$ DECLARE c_schema CONSTANT name := (SELECT extnamespace::regnamespace::text FROM pg_extension WHERE extname = 'cat_tools'); @@ -179,7 +179,7 @@ CREATE TABLE _object_reference.object( , object_names text[] NOT NULL , object_args text[] NOT NULL , CONSTRAINT object__u_object_names__object_args UNIQUE( object_type, object_names, object_args ) - /* TODO: this can't be a trigger because some objects won't exist when a dump is loaded + /* EXCLUDED CODE: TODO: this can't be a trigger because some objects won't exist when a dump is loaded , CONSTRAINT object__address_sanity -- pg_get_object_address will throw an error if anything is wrong, so the IS NOT NULL is mostly pointless 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; CREATE TABLE _object_reference._object_oid( object_id int PRIMARY KEY REFERENCES _object_reference.object ON DELETE CASCADE ON UPDATE CASCADE , classid regclass NOT NULL - /* TODO: needs to be a trigger + /* EXCLUDED CODE: TODO: needs to be a trigger CONSTRAINT classid_must_match__object__address_classid CHECK( classid IS NOT DISTINCT FROM cat_tools.object__address_classid(object_type) ) */ @@ -1253,7 +1253,7 @@ BEGIN RETURN c_next_level; EXCEPTION WHEN undefined_table THEN - /* + /* EXCLUDED CODE CREATE TEMP TABLE __object_reference__ddl_capture AS SELECT c_next_level, capture__start.object_group_id ; diff --git a/test/deps.sql b/test/deps.sql index e1a53c8..b0ee45f 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -2,8 +2,7 @@ -- Add any test dependency statements here -/* - * Normally these should be loaded by the cascade! +/* EXCLUDED CODE: normally these should be loaded by the cascade! CREATE EXTENSION IF NOT EXISTS count_nulls; CREATE EXTENSION IF NOT EXISTS cat_tools; */ diff --git a/test/helpers/object_table.sql b/test/helpers/object_table.sql index 67f07ee..ca90234 100644 --- a/test/helpers/object_table.sql +++ b/test/helpers/object_table.sql @@ -144,6 +144,7 @@ INSERT INTO test_prereq VALUES ; -- \N is null character +-- sql-lint:disable-block prefer-short-type: secondary column mirrors pg_catalog's own type display name (format_type), not a style choice COPY test_object(object_type, object_name, secondary, create_command, drop_command) FROM STDIN (DELIMITER '|'); table|test table||%("test column" int)| 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 (" 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 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" \. +-- sql-lint:enable-block -/* Not supported +/* EXCLUDED CODE: Not supported composite type|test complex type||CREATE TYPE "test complex type" AS(r real, i real)|DROP TYPE "test complex type" view column|test view|test column|\N|\N 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" diff --git a/test/sql/capture.sql b/test/sql/capture.sql index 8eb1414..073f3f3 100644 --- a/test/sql/capture.sql +++ b/test/sql/capture.sql @@ -134,7 +134,7 @@ SELECT bag_eq( , $$SELECT object_id FROM obj_ref$$ , 'Verify captured object IDs match' ); -/* +/* EXCLUDED CODE SELECT * FROM og_o; SELECT * FROM _object_reference.object;-- WHERE object_id IN(6,9); */ diff --git a/test/sql/event_trigger.sql b/test/sql/event_trigger.sql index 2115dc7..402a5b4 100644 --- a/test/sql/event_trigger.sql +++ b/test/sql/event_trigger.sql @@ -143,7 +143,7 @@ $body$; /* - *Rename column + * Rename column */ SELECT lives_ok( $$ALTER TABLE table_under_test RENAME column_test TO test_column2$$ diff --git a/test/sql/object_group.sql b/test/sql/object_group.sql index 6304892..d90957b 100644 --- a/test/sql/object_group.sql +++ b/test/sql/object_group.sql @@ -82,7 +82,7 @@ SELECT is( ); -- __object__add -/* TODO +/* EXCLUDED CODE: TODO SELECT pg_temp.bogus_group( format( $$SELECT object_reference.object_group__object__add(%%s, %s)$$ @@ -107,7 +107,7 @@ SELECT throws_ok( -- Can't use helper here , 'object group "absurd group name used only for testing purposes ktxbye" does not exist' , 'object__getsert with bogus group name' ); -/* TODO +/* EXCLUDED CODE: TODO SELECT throws_ok( -- Can't use helper here $$CREATE TEMP TABLE col1_id AS SELECT * FROM object_reference.object__getsert_w_group_id('table column', 'test_table_1', 'col1', -1)$$ , ''