From e5fa303886d8f18db93dfc188fd751111989644f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 15:15:30 -0500 Subject: [PATCH 01/11] Add sql-lint: style linter for PostgreSQL SQL files Adds a Perl-based style linter enforcing this project's SQL coding conventions (block comment formatting, leading-comma style, short type-name preferences). Adds `make lint`, gated in CI (`lint` job) alongside the existing test matrix. The linter supports both plain .sql files and cat_tools' .sql.in template convention (the directory walk in bin/sql-lint matches both extensions), and its generated-file skip check matches a first line containing both "GENERATED" and "DO NOT EDIT" in either order, so it recognizes this project's build marker ("GENERATED FILE! DO NOT EDIT!"). See lint/sql/ DESIGN.md's "Portability" section. lint.mk gained a LINT_TARGETS override (default sql/ test/, unchanged). The root Makefile sets it to sql/cat_tools.sql.in, sql/omit_column.sql, and test/ -- excluding version-specific install/update scripts, which are frozen once released (never hand-edited again per this repo's SQL file conventions) and would otherwise produce permanent, unfixable findings. Also fixes the resulting findings in the current source so `make lint` passes clean: two inline /* */ comments converted to --, two stacked -- runs converted to /* */ blocks, a borrowed SELECT list (pg_all_foreign_keys) converted from trailing to leading commas, and a commented-out test fixture marked with the EXCLUDED CODE convention instead of being reformatted line by line. sql/cat_tools--0.3.0.sql.in is regenerated to match (make install && make test both verified green after these changes). --- .github/workflows/ci.yml | 20 +- Makefile | 11 + lint/Makefile | 8 + lint/lint.mk | 29 + lint/sql/DESIGN.md | 91 +++ lint/sql/Makefile | 4 + lint/sql/README.md | 257 +++++++++ lint/sql/bin/sql-lint | 525 ++++++++++++++++++ lint/sql/test/01-fixtures.t | 72 +++ lint/sql/test/02-scanner.t | 287 ++++++++++ .../sql/test/fixtures/comment-closing.bad.sql | 7 + .../test/fixtures/comment-closing.good.sql | 7 + .../test/fixtures/comment-line-prefix.bad.sql | 6 + .../fixtures/comment-line-prefix.good.sql | 20 + .../sql/test/fixtures/comment-opening.bad.sql | 7 + .../test/fixtures/comment-opening.good.sql | 9 + .../sql/test/fixtures/comment-padding.bad.sql | 10 + .../test/fixtures/comment-padding.good.sql | 9 + .../test/fixtures/comment-single-line.bad.sql | 4 + .../fixtures/comment-single-line.good.sql | 7 + .../fixtures/comment-stacked-dashes.bad.sql | 7 + .../fixtures/comment-stacked-dashes.good.sql | 17 + .../fixtures/generated-file-skipped.good.sql | 5 + .../test/fixtures/prefer-short-type.bad.sql | 12 + .../test/fixtures/prefer-short-type.good.sql | 19 + lint/sql/test/fixtures/trailing-comma.bad.sql | 30 + .../sql/test/fixtures/trailing-comma.good.sql | 46 ++ sql/cat_tools--0.3.0.sql.in | 54 +- sql/cat_tools.sql.in | 54 +- test/sql/pg_depends.sql | 2 +- 30 files changed, 1584 insertions(+), 52 deletions(-) create mode 100644 lint/Makefile create mode 100644 lint/lint.mk create mode 100644 lint/sql/DESIGN.md create mode 100644 lint/sql/Makefile create mode 100644 lint/sql/README.md create mode 100755 lint/sql/bin/sql-lint create mode 100644 lint/sql/test/01-fixtures.t create mode 100644 lint/sql/test/02-scanner.t create mode 100644 lint/sql/test/fixtures/comment-closing.bad.sql create mode 100644 lint/sql/test/fixtures/comment-closing.good.sql create mode 100644 lint/sql/test/fixtures/comment-line-prefix.bad.sql create mode 100644 lint/sql/test/fixtures/comment-line-prefix.good.sql create mode 100644 lint/sql/test/fixtures/comment-opening.bad.sql create mode 100644 lint/sql/test/fixtures/comment-opening.good.sql create mode 100644 lint/sql/test/fixtures/comment-padding.bad.sql create mode 100644 lint/sql/test/fixtures/comment-padding.good.sql create mode 100644 lint/sql/test/fixtures/comment-single-line.bad.sql create mode 100644 lint/sql/test/fixtures/comment-single-line.good.sql create mode 100644 lint/sql/test/fixtures/comment-stacked-dashes.bad.sql create mode 100644 lint/sql/test/fixtures/comment-stacked-dashes.good.sql create mode 100644 lint/sql/test/fixtures/generated-file-skipped.good.sql create mode 100644 lint/sql/test/fixtures/prefer-short-type.bad.sql create mode 100644 lint/sql/test/fixtures/prefer-short-type.good.sql create mode 100644 lint/sql/test/fixtures/trailing-comma.bad.sql create mode 100644 lint/sql/test/fixtures/trailing-comma.good.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 336f866..c03606d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,6 +211,24 @@ jobs: # (pgxntool marks installcheck `.IGNORE`), so failures would pass silently. make verify-results + # Style linter (see lint/sql/README.md). Runs the linter's own test suite + # first (fixtures + scanner edge cases), then lints the actively-maintained + # SQL (LINT_TARGETS in the root Makefile). No PostgreSQL needed -- sql-lint + # is a standalone Perl script -- so this doesn't use the pgxn-tools + # container. + lint: + needs: [changes] + if: needs.changes.outputs.docs_only != 'true' + name: ๐Ÿงน SQL Lint + runs-on: ubuntu-latest + steps: + - name: Check out the repo + uses: actions/checkout@v6 + - name: Run the linter's own test suite + run: make -C lint test + - name: Lint SQL + run: make lint + # Proves cat_tools survives a BINARY pg_upgrade (in-place catalog migration to a # newer PostgreSQL major), not just an in-place extension update. Each leg is a # SINGLE jump: install an old cat_tools on an old cluster, plant a dependency @@ -555,7 +573,7 @@ jobs: # the heavy jobs gated off by the `changes` job on a docs-only push), and # fails if any failed or were cancelled. all-checks-passed: - needs: [changes, test, pg-upgrade-test, pg-upgrade-stepwise, extension-update-test] + needs: [changes, test, lint, pg-upgrade-test, pg-upgrade-stepwise, extension-update-test] if: always() runs-on: ubuntu-latest steps: diff --git a/Makefile b/Makefile index bb7e699..a6de719 100644 --- a/Makefile +++ b/Makefile @@ -92,3 +92,14 @@ $(DESTDIR)$(datadir)/extension/cat_tools--0.2.0.sql: .PHONY: clean_old_version clean_old_version: pgxn uninstall --unstable 'cat_tools=0.2.0' + +# Style linter (see lint/sql/README.md). Scoped to the actively-maintained +# source rather than the default `sql/ test/` (lint/lint.mk): version-specific +# install/update scripts under sql/ are frozen once released (SQL file +# conventions rule 5 in CLAUDE.md โ€” never hand-edited again), so linting them +# would produce permanent, unfixable findings and make `make lint` unusable +# as a CI gate. Lint the current source instead; a version file still under +# active development can be linted directly, e.g. +# `lint/sql/bin/sql-lint sql/cat_tools--0.3.0.sql.in`. +LINT_TARGETS = sql/cat_tools.sql.in sql/omit_column.sql test/ +include lint/lint.mk diff --git a/lint/Makefile b/lint/Makefile new file mode 100644 index 0000000..5c19f0a --- /dev/null +++ b/lint/Makefile @@ -0,0 +1,8 @@ +# Parent Makefile for all linters. +# Add new linter subdirectories to LINTERS as they are created. + +LINTERS = sql + +.PHONY: test +test: + for linter in $(LINTERS); do $(MAKE) -C $$linter test || exit 1; done diff --git a/lint/lint.mk b/lint/lint.mk new file mode 100644 index 0000000..ae90c99 --- /dev/null +++ b/lint/lint.mk @@ -0,0 +1,29 @@ +# lint.mk โ€” shared Make fragment that adds a `lint` target to any extension. +# +# Include from an extension Makefile (one level below the repo root): +# +# include ../lint/lint.mk +# +# Provides: +# make lint โ€” run sql-lint on all SQL we own under the extension dir (sql/ and +# test/, or LINT_TARGETS if set). sql-lint skips vendored trees +# (deps/, pgxntool/) and auto-generated files (e.g. +# sql/--.sql). +# +# Set LINT_TARGETS before this include to lint a different set of paths, e.g. +# to exclude frozen version-snapshot files that are never hand-edited: +# +# LINT_TARGETS = sql/cat_tools.sql.in sql/omit_column.sql test/ +# include lint/lint.mk + +# Resolve the repo root relative to this file's location (lint/ is one level +# below root, so go up one more). +_LINT_MK_DIR := $(dir $(lastword $(MAKEFILE_LIST))) +_REPO_ROOT := $(abspath $(_LINT_MK_DIR)/..) +SQL_LINT := $(_REPO_ROOT)/lint/sql/bin/sql-lint + +LINT_TARGETS ?= sql/ test/ + +.PHONY: lint +lint: + $(SQL_LINT) $(LINT_TARGETS) diff --git a/lint/sql/DESIGN.md b/lint/sql/DESIGN.md new file mode 100644 index 0000000..320ccc8 --- /dev/null +++ b/lint/sql/DESIGN.md @@ -0,0 +1,91 @@ +# sql-lint Design + +## Type Name Preferences + +Preferred names were determined by querying `pg_type` on PG17. All pairs are +true aliases (same OID). Preferences: + +- SQL standard names where shorter: `int` (not `integer`), `varchar` (not + `character varying`) +- SQL standard names where clearer: `smallint`/`bigint` (not `int2`/`int8`), + `boolean` (not `bool`), `real` (not `float4`), `double precision` (not + `float8`) +- Common short form: `numeric` (not `decimal`), `timestamp`/`timestamptz` + (not the verbose `without/with time zone` forms) + +NOT flagged: `text` vs `varchar` (semantically different โ€” `varchar` has an +optional length constraint). + +## Portability + +Two spots are kept generic rather than hardcoded to this project, since +build conventions for what counts as "generated" vary: + +- The directory walk in `bin/sql-lint` matches both `*.sql` and `*.sql.in` โ€” + cat_tools' hand-maintained SQL source is a `.sql.in` template expanded to + `.sql` at build time, not a plain `.sql` file. +- `file_is_generated()` matches any first line containing both "GENERATED" + and "DO NOT EDIT" (in either order), rather than one fixed marker string. + +`lint.mk` also exposes `LINT_TARGETS` (default `sql/ test/`) so `make lint` +can be scoped to specific paths โ€” e.g. to skip frozen version-snapshot files +that are tracked for update-path testing but, per project convention, are +never hand-edited again and so have no fixable findings. + +## Future Rules + +### Ready Now (line-level, no new infrastructure) + +| Rule | Complexity | Description | +|---|---|---| +| **coalesce-boolean** | ~15 lines | Flag `COALESCE(expr, true/false)` โ€” use `IS [NOT] TRUE` instead | +| **dollar-quote-naming** | ~15 lines | Flag bare `$$` and single-char `$x$` tags on multi-line blocks | + +### Needs Small Helper (~10 lines of infra) + +| Rule | Complexity | Description | +|---|---|---| +| **when-others-comment** | ~25 lines | `WHEN OTHERS` without explanatory comment on same/next line | +| **security-definer** | ~30 lines | `SECURITY DEFINER` without `SET search_path` nearby | + +### Needs Statement Tracking Infrastructure + +| Rule | Complexity | Description | +|---|---|---| +| **semicolon-placement** | ~50 lines + tracker | `;` on own line for multi-line statements | +| **comment-ordering** | ~80 lines + tracker + DDL extractor | `COMMENT ON` must follow its object | +| **variable-naming** | ~50 lines + dollar-quote awareness | PL/pgSQL `c_`/`v_`/`a_`/`r_` prefixes | + +The statement boundary tracker is the key infrastructure piece โ€” building it +unlocks semicolon-placement and comment-ordering. Variable-naming needs +separate dollar-quote block awareness. + +The `lint/` directory structure supports adding linters for other languages +in the future (e.g. `lint/plpgsql/`, `lint/bash/`). Each linter is a +subdirectory with its own `Makefile`; `lint/Makefile` aggregates them. + +## History + +### Language Choice + +The linter went through two iterations: + +1. **Initial: Bash + AWK** (7 files, ~530 lines, 3 languages). Shipped fast + but hit pain points: subshell variable loss, temp-file workarounds, awk + embedded inside bash single-quotes (`\047` for single-quote), and no path + to statement-level rules. + +2. **Current: single Perl script**. Same rules, same output โ€” but the scanner, + suppression, and all rules live in one file with real data structures. + +Why Perl over Go/Rust/Python: + +| | Perl | Go/Rust | Python | +|---|---|---|---| +| Text processing | Native strength | Adequate | Adequate | +| Available everywhere | Yes (ships with every Unix) | Needs build pipeline | Yes (3.6+) | +| Statement-level rules | Natural (slurp + split) | Natural | Natural | +| Docker footprint | 15 MB | N/A (static binary) | 50 MB | + +Go and Rust require a build/release pipeline that doesn't exist for this repo. +Python was considered but Perl is more concise for regex-heavy text processing. diff --git a/lint/sql/Makefile b/lint/sql/Makefile new file mode 100644 index 0000000..80404d9 --- /dev/null +++ b/lint/sql/Makefile @@ -0,0 +1,4 @@ +# Run lint/sql tests locally (requires perl on $PATH). +.PHONY: test +test: + prove -v test/ diff --git a/lint/sql/README.md b/lint/sql/README.md new file mode 100644 index 0000000..3342ffa --- /dev/null +++ b/lint/sql/README.md @@ -0,0 +1,257 @@ +# sql-lint + +Style linter for PostgreSQL SQL files. Enforces this project's SQL coding +conventions (block comments, leading commas, short type names โ€” see the +"SQL file conventions" and "Code Style" sections of `CLAUDE.md`). + +## Quick Start + +```bash +# Lint the project's SQL from the repo root (paths are set in Makefile's +# LINT_TARGETS; see lint/lint.mk) +make lint + +# Run the linter directly on specific files or directories +lint/sql/bin/sql-lint sql/cat_tools.sql.in test/ +``` + +## Usage + +``` +sql-lint [OPTIONS] FILE... +sql-lint [OPTIONS] DIRECTORY... +``` + +Files are checked against all enabled rules. Directories are searched +recursively for `*.sql` and `*.sql.in` files (excluding `deps/`, +`docker/deps/`, `pgxntool/`, and `.claude/worktrees/`). + +### Options + +| Flag | Description | +|------|-------------| +| `-h`, `--help` | Show help message | +| `-q`, `--quiet` | Suppress the summary line | + +### Exit Codes + +| Code | Meaning | +|------|---------| +| 0 | No findings | +| 1-10 | Reserved for generic errors (2 = usage error) | +| 11+ | Lint findings: `exit_code - 10` = number of findings | + +Note: exit codes are capped at 255 by POSIX, so 245+ findings all exit 255. + +## Rules + +All comment formatting rules share the `comment-` prefix so they sort +together in fixture listings. + +### comment-closing + +Block comments must end with `*/` alone on its own line. + +```sql +/* + * Bad: text before closing */ + +/* + * Good: closing on its own line. + */ +``` + +### comment-line-prefix + +Lines inside a block comment must start with ` * `. + +```sql +/* + Bad: missing the star prefix. + * Good: has the star prefix. + */ +``` + +### comment-opening + +Block comments must start with `/*` alone on the opening line. + +```sql +/* Bad: text on the opening line + * of a multi-line comment + */ + +/* + * Good: opening line has only /* + */ +``` + +### comment-padding + +Block comments must not have blank `*` lines immediately after `/*` +or immediately before `*/`. + +```sql +/* + * + * Bad: leading blank line above. + */ + +/* + * Good: content starts immediately. + */ +``` + +### comment-single-line + +Single-line `/* text */` comments are not allowed. Use `--` for +single-line comments. + +```sql +/* Bad: single-line block comment */ + +-- Good: use a line comment instead +``` + +### comment-stacked-dashes + +Multi-line comments must use `/* */` syntax, not consecutive `--` lines. + +```sql +-- Bad: stacked dashes +-- spanning multiple lines + +/* + * Good: block comment + * spanning multiple lines + */ +``` + +### trailing-comma + +Multi-line constructs must use leading commas (`, item`), not trailing +commas (`item,`). + +```sql +-- Bad +SELECT + column1, + column2, + column3 + +-- Good +SELECT + column1 + , column2 + , column3 +``` + +### prefer-short-type + +Use short-form type names where PostgreSQL provides them. `text` and +`varchar` are not interchangeable and are not flagged against each other. + +| Don't use | Use instead | +|-------------|-----------| +| `integer`, `int4` | `int` | +| `bool` | `boolean` | +| `character varying` | `varchar` | +| `float8` | `double precision` | +| `float4` | `real` | +| `int2` | `smallint` | +| `int8` | `bigint` | +| `decimal` | `numeric` | +| `timestamp without time zone` | `timestamp` | +| `timestamp with time zone` | `timestamptz` | +| `time without time zone` | `time` | +| `time with time zone` | `timetz` | + +## Inline Suppression + +Suppress a finding on a specific line: + +```sql +SELECT something, -- sql-lint:disable trailing-comma +``` + +`disable` and `disable-line` both apply to the line they appear on; +`disable-next-line` applies to the following line. Use `all` in place of a +rule id to suppress every rule. + +```sql +-- sql-lint:disable-next-line trailing-comma +SELECT something, +``` + +### Block suppression + +A `disable-block ` directive on the line that opens a `/* */` block +comment suppresses the rule for every line in that block. + +For the common case โ€” commenting out a chunk of code without the linter +demanding ` * ` prefixes (or other comment styling) on each line โ€” use the +shorthand `EXCLUDED CODE` on the opening line. It is an alias for +`sql-lint:disable-block all`: + +```sql +/* EXCLUDED CODE +SELECT not_ready_yet( + foo + , bar +); +*/ +``` + +Text may follow it (e.g. `/* EXCLUDED CODE โ€” disabled until issue #123`). + +## Adding New Rules + +Add a subroutine to `bin/sql-lint` and register it in the `@rules` array: + +```perl +sub check_my_rule { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'CODE'; + my $cc = code_content($l->{text}); + next unless $cc =~ /pattern/; + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'my-rule', + 'description of the violation'); + } + return @out; +} +``` + +Each line in `$lines` is a hashref with `lineno`, `state` (`CODE` or +`COMMENT`), and `text`. The `code_content()` helper strips strings, +comments, and `/*` from a line. `maybe_finding()` handles suppression +automatically. + +Then add test fixtures and/or inline tests: + +- **Fixtures** (`test/fixtures/`): use `.good.sql` for clean files, + `.bad.sql` for files with violations. Bad fixtures need a + `-- expect-findings: N` header. No test code changes required. +- **Inline tests** (`test/02-scanner.t`): for specific edge cases, + regression tests, and assertions about finding content. + +## Testing + +Tests use Perl's `Test::More` + `prove` to test the linter itself (not +the extension code โ€” that's what `make lint` is for): + +```bash +make -C lint test # all linters (currently just sql) +make -C lint/sql test # sql linter only +``` + +## Design + +See [DESIGN.md](DESIGN.md) for architecture decisions and rationale. + +## Future + +The `lint/` directory is designed to host linters for other languages +(e.g. PL/pgSQL, bash). Each would be a sibling to `sql/` with its own +`Makefile` and test suite. diff --git a/lint/sql/bin/sql-lint b/lint/sql/bin/sql-lint new file mode 100755 index 0000000..22216c2 --- /dev/null +++ b/lint/sql/bin/sql-lint @@ -0,0 +1,525 @@ +#!/usr/bin/env perl +# +# sql-lint: style linter for PostgreSQL SQL files +# +# Usage: sql-lint [OPTIONS] FILE... +# sql-lint [OPTIONS] DIRECTORY... +# +# Checks SQL files against the project's coding style rules and reports +# violations. Directories are searched recursively for *.sql and *.sql.in +# files. +# +# Options: +# -h, --help Show this help message +# -q, --quiet Suppress summary line +# --scan Dump scanner output and exit (for debugging/testing) +# +# Exit codes: +# 0 No findings +# 1-10 Reserved for generic errors (2 = usage error) +# 11+ Lint findings: exit_code - 10 = number of findings +# (exit codes are capped at 255 by POSIX) + +use strict; +use warnings; +use File::Find (); +use Getopt::Long qw(:config no_ignore_case bundling); + +# ============================================================ +# Scanner โ€” labels each line CODE or COMMENT +# +# Single character-by-character pass over each line, maintaining state +# across lines. Tracks block comment depth (/* ... */, supports +# PostgreSQL's nested comments), single-quote strings (to prevent /* +# inside '...' from opening a comment), and line comments (-- to EOL). +# +# Each line is labeled CODE or COMMENT based on comment depth at the +# *start* of that line, then depth is updated for the next line. +# +# Nested /* is real nesting: PostgreSQL treats /* ... /* ... */ ... */ +# as nested block comments; a /* inside an existing block comment +# increments depth and requires a matching */. +# +# Dollar-quoted blocks ($$...$$) are intentionally not tracked โ€” their +# content is usually PL/pgSQL that should be linted. +# ============================================================ + +sub scan_file { + my ($filename) = @_; + open my $fh, '<', $filename or die "ERROR: cannot open $filename: $!\n"; + my @lines; + my $comment_depth = 0; + # Must persist across lines (like comment_depth): a single-quoted literal + # may span multiple lines, and -- or /* inside it must not start a comment. + my $in_string = 0; + # Line number of the /* that opened the block comment we are currently + # inside (the outermost one). Lets rules honor a block-level suppression + # directive placed on the opening line. Undef when not in a comment. + my $open_lineno; + + while (my $raw = <$fh>) { + chomp $raw; + # State is determined at line entry โ€” before scanning this line's content. + push @lines, { + lineno => $., + state => $comment_depth > 0 ? 'COMMENT' : 'CODE', + in_string => $in_string, + text => $raw, + ($comment_depth > 0 ? (block_open => $open_lineno) : ()), + }; + + # Character-by-character scan to update comment_depth for the NEXT line. + # Tracks nested /* */ and skips /* inside strings and -- comments. + my $len = length $raw; + my $i = 0; + + while ($i < $len) { + my $c = substr $raw, $i, 1; + my $c2 = $i + 1 < $len ? substr($raw, $i, 2) : ''; + + if ($comment_depth > 0) { + if ($c2 eq '*/') { $comment_depth--; $i += 2; next } # close block + if ($c2 eq '/*') { $comment_depth++; $i += 2; next } # nested open + $i++; next; + } + if ($in_string) { + if ($c2 eq "''") { $i += 2; next } # escaped single-quote + $in_string = 0 if $c eq "'"; # end of string + $i++; next; + } + # CODE context โ€” not inside comment or string + last if $c2 eq '--'; # line comment; rest is ignored + if ($c2 eq '/*') { $comment_depth++; $open_lineno = $.; $i += 2; next } # open block comment + if ($c eq "'") { $in_string = 1; $i++; next } # open string literal + # Dollar-quoted blocks ($$...$$) are intentionally not tracked. + # Their content is usually PL/pgSQL that should be linted. + $i++; + } + } + close $fh; + return \@lines; +} + +# ============================================================ +# Helpers +# ============================================================ + +# Strip single-quoted string literals (handles '' escapes). +# When $in_string is true, the line starts inside a multi-line string โ€” +# strip from the beginning through the closing quote. +sub strip_strings { + my ($text, $in_string) = @_; + my $t = $text; + if ($in_string) { + # Strip leading string continuation through closing quote + $t =~ s/^[^']*'// or return ''; # no close on this line โ€” all string + } + $t =~ s/'(?:''|[^'])*'//g; # complete single-line strings + $t =~ s/'.*$//; # unclosed trailing string (multi-line open) + return $t; +} + +# Like strip_strings, but replaces each string literal with a single sentinel +# character ('x') instead of removing it. Used where the *position* of code +# relative to a string matters (e.g. trailing-comma): removing the string would +# turn a correct leading-comma item like `, 'desc'` into a bare `,`, and turn a +# genuine trailing comma after a string like `'desc',` into the same bare `,` โ€” +# the two become indistinguishable. The sentinel preserves which side the comma +# is on. +sub blank_strings { + my ($text, $in_string) = @_; + my $t = $text; + if ($in_string) { + $t =~ s/^[^']*'/x/ or return 'x'; # continuation: close -> sentinel; else all string + } + $t =~ s/'(?:''|[^'])*'/x/g; # complete single-line strings + $t =~ s/'.*$/x/; # unclosed trailing string (multi-line open) + return $t; +} + +# Return the "code" portion of a line โ€” strings, trailing -- comment, +# and inline /* ... */ comments removed. +sub code_content { + my ($text, $in_string) = @_; + my $c = strip_strings($text, $in_string); + $c =~ s|/\*.*?\*/||g; # strip complete inline /* ... */ comments first + $c =~ s|/\*.*$||; # strip unclosed /* to end of line + $c =~ s/\s*--.*$//; # strip trailing line comment last (-- inside /* */ already gone) + return $c; +} + +# Like code_content but keeps a sentinel for string literals (see blank_strings). +sub code_skeleton { + my ($text, $in_string) = @_; + my $c = blank_strings($text, $in_string); + $c =~ s|/\*.*?\*/||g; + $c =~ s|/\*.*$||; + $c =~ s/\s*--.*$//; + return $c; +} + +# True if the finding is suppressed by an inline pragma. +sub is_suppressed { + my ($lines, $lineno, $rule_id) = @_; + my $idx = $lineno - 1; + return 0 if $idx < 0 || $idx > $#$lines; + + my $text = $lines->[$idx]{text}; + # Same-line directive. Matches `sql-lint:disable`, `disable-line`, or + # `disable-block` (the optional `-line`/`-block` suffix), followed by either + # this rule's id or the literal `all`. \Q...\E escapes any regex-special + # chars in $rule_id; the trailing \b stops `all` (or a rule id) from also + # matching when it is only a prefix of a longer following word. + return 1 if $text =~ /sql-lint:disable(?:-line|-block)?\s+(?:\Q$rule_id\E|all)\b/; + # `/* EXCLUDED CODE` marks the start of a commented-out code block โ€” a + # friendly alias for `sql-lint:disable-block all`. Matching here covers the + # opener line itself (e.g. its comment-opening finding). + return 1 if $text =~ m{/\*\s*EXCLUDED CODE\b}; + + if ($idx > 0) { + my $prev = $lines->[$idx - 1]{text}; + return 1 if $prev =~ /sql-lint:disable-next-line\s+(?:\Q$rule_id\E|all)\b/; + } + + # Block-level: a directive on the /* that opened the enclosing block comment + # suppresses the rule for every line in the block. This lets a /* */ block be + # used to comment out code without the linter demanding a " * " prefix (or + # other comment styling) on each line. `EXCLUDED CODE` is the short alias. + my $open = $lines->[$idx]{block_open}; + if (defined $open) { + my $open_text = $lines->[$open - 1]{text}; + # Same rule-id/`all` matching as the same-line directive above (see that + # comment for the regex breakdown), but only the `disable-block` form + # counts on the opening line. + return 1 if $open_text =~ /sql-lint:disable-block\s+(?:\Q$rule_id\E|all)\b/; + return 1 if $open_text =~ m{/\*\s*EXCLUDED CODE\b}; + } + return 0; +} + +# Convenience: push a finding unless suppressed. +sub maybe_finding { + my ($out, $lines, $file, $lineno, $rule, $msg) = @_; + return if is_suppressed($lines, $lineno, $rule); + push @$out, { file => $file, line => $lineno, rule => $rule, message => $msg }; +} + +# ============================================================ +# Rules +# +# Each rule is a subroutine receiving ($file, $lines) and returning a +# list of finding hashrefs. Use maybe_finding() for suppression checks. +# Rules examining comment formatting check COMMENT-state lines; all +# others check CODE-state lines only. +# ============================================================ + +sub check_stacked_dashes { + my ($file, $lines) = @_; + my @out; + my ($start, $end) = (0, 0); + + my $flush = sub { + return unless $start && ($end - $start) >= 1; + my $n = $end - $start + 1; + maybe_finding(\@out, $lines, $file, $start, 'comment-stacked-dashes', + "$n consecutive -- comment lines ($start-$end); use /* */ for multi-line comments"); + }; + + for my $l (@$lines) { + if ($l->{state} eq 'CODE' && $l->{text} =~ /^\s*--/) { # line-comment line + $start ||= $l->{lineno}; + $end = $l->{lineno}; + } else { + $flush->(); + ($start, $end) = (0, 0); + } + } + $flush->(); + return @out; +} + +sub check_comment_opening { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'CODE'; + next if $l->{text} =~ /^\s*--/; # skip line-comment lines + my $cc = strip_strings($l->{text}, $l->{in_string}); + # Strip /* ... */ before -- (same reason as code_content: -- inside + # a block comment would eat the closing */ if stripped first). + $cc =~ s|/\*.*?\*/||g; + $cc =~ s/\s*--.*$//; + next unless $cc =~ m|/\*|; # must contain a block comment opener + (my $after = $cc) =~ s|.*?/\*||; # text after the /* + next unless $after =~ /\S/; # something non-space follows /* + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-opening', + 'text after opening /*; put /* on its own line'); + } + return @out; +} + +sub check_single_line_block_comment { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'CODE'; + next if $l->{text} =~ /^\s*--/; # skip line-comment lines + my $cc = strip_strings($l->{text}, $l->{in_string}); + # NB: stripping -- before /* */ can eat a closing */ when -- appears + # inside a block comment (e.g. /* hint -- note */). That causes a + # false negative, but the line is already a violation regardless. + $cc =~ s/\s*--.*$//; + next unless $cc =~ m|/\*| && $cc =~ m|\*/|; # has both /* and */ + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-single-line', + 'single-line /* */ comment; use -- for single-line comments'); + } + return @out; +} + +sub check_comment_line_prefix { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'COMMENT'; # only inside /* */ blocks + next if $l->{text} =~ m|\*/|; # closing line โ€” other rule handles + (my $s = $l->{text}) =~ s/^\s*//; # strip leading whitespace + next if $s =~ /^\* / || $s eq '*'; # " * text" or bare " *" + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-line-prefix', + 'comment line must start with " * " (space-star-space)'); + } + return @out; +} + +sub check_comment_padding { + my ($file, $lines) = @_; + my @out; + for my $i (0 .. $#$lines) { + my $l = $lines->[$i]; + next unless $l->{state} eq 'COMMENT'; + next unless $l->{text} =~ /^\s*\*\s*$/; # bare * line + + # Leading: previous line opened the comment (CODE state) + if ($i > 0 && $lines->[$i-1]{state} eq 'CODE') { + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-padding', + 'unnecessary blank line at start of block comment'); + next; + } + + # Trailing: next line closes the comment + if ($i < $#$lines && $lines->[$i+1]{text} =~ /\*\//) { + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-padding', + 'unnecessary blank line at end of block comment'); + } + } + return @out; +} + +sub check_comment_closing { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'COMMENT'; # only inside /* */ blocks + next unless $l->{text} =~ m|\*/|; # must contain closing */ + next if $l->{text} =~ m|^\s*\*/\s*$|; # */ alone on its line is fine + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-closing', + 'closing */ must be alone on its own line'); + } + return @out; +} + +sub check_trailing_comma { + my ($file, $lines) = @_; + my @out; + for my $l (@$lines) { + next unless $l->{state} eq 'CODE'; + next if $l->{text} =~ /^\s*--/; # skip line-comment lines + # Keep a sentinel for strings: a leading-comma item whose payload is a + # string (e.g. `, 'desc'`) must not be mistaken for a trailing comma, + # while a genuine trailing comma after a string (e.g. `'desc',`) must + # still be caught. code_content (which deletes strings) collapses both + # to a bare `,`; code_skeleton keeps the comma on the correct side. + my $cc = code_skeleton($l->{text}, $l->{in_string}); + $cc =~ s/\s+$//; # trim trailing whitespace + next unless $cc =~ /,$/; # ends with comma after stripping + # The comma must follow an item to be "trailing". A line that reduces to + # a bare leading comma is not a violation โ€” e.g. a leading comma alone on + # its line introducing a multi-line item (the comma, then the item's + # first line indented below it). Pure string items like `, 'x'` keep + # their item via the sentinel above, so they never reach this point. + (my $before = $cc) =~ s/,\s*$//; + next unless $before =~ /\S/; + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'trailing-comma', + 'trailing comma; use leading comma style'); + } + return @out; +} + +sub check_prefer_short_type { + my ($file, $lines) = @_; + my @out; + + # Multi-word type aliases โ€” checked via regex, before single-word + my @multi = ( + [ qr/character\s+varying/i, 'varchar' ], + [ qr/timestamp\s+without\s+time\s+zone/i, 'timestamp' ], + [ qr/timestamp\s+with\s+time\s+zone/i, 'timestamptz' ], + [ qr/time\s+without\s+time\s+zone/i, 'time' ], + [ qr/time\s+with\s+time\s+zone/i, 'timetz' ], + ); + + # Single-word type aliases โ€” checked via \bword\b match + my @single = ( + [ 'integer', 'int' ], + [ 'int4', 'int' ], + [ 'int2', 'smallint' ], + [ 'int8', 'bigint' ], + [ 'float4', 'real' ], + [ 'float8', 'double precision' ], + [ 'bool', 'boolean' ], + [ 'decimal', 'numeric' ], + ); + + for my $l (@$lines) { + next unless $l->{state} eq 'CODE'; + next if $l->{text} =~ /^\s*--/; # skip line-comment lines + my $cc = lc code_content($l->{text}, $l->{in_string}); + + for my $m (@multi) { + if ($cc =~ $m->[0]) { + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'prefer-short-type', + "use \"$m->[1]\" instead of \"$&\""); + } + } + for my $s (@single) { + my ($word, $sug) = @$s; + if ($cc =~ /\b\Q$word\E\b/) { + maybe_finding(\@out, $lines, $file, $l->{lineno}, 'prefer-short-type', + "use \"$sug\" instead of \"$word\""); + } + } + } + return @out; +} + +# ============================================================ +# Rule registry +# ============================================================ + +my @rules = ( + { id => 'comment-closing', check => \&check_comment_closing }, + { id => 'comment-line-prefix', check => \&check_comment_line_prefix }, + { id => 'comment-opening', check => \&check_comment_opening }, + { id => 'comment-padding', check => \&check_comment_padding }, + { id => 'comment-single-line', check => \&check_single_line_block_comment }, + { id => 'comment-stacked-dashes', check => \&check_stacked_dashes }, + { id => 'prefer-short-type', check => \&check_prefer_short_type }, + { id => 'trailing-comma', check => \&check_trailing_comma }, +); + +# A file whose first line marks it as generated (both "GENERATED" and "DO NOT +# EDIT", matched independently so either word order works, e.g. "GENERATED +# FILE! DO NOT EDIT!") is a build artifact (e.g. a version-specific install +# script concatenated from a hand-edited .sql/.sql.in source that is linted +# on its own), so skip linting it. +sub file_is_generated { + my ($file) = @_; + open my $fh, '<', $file or return 0; + my $first = <$fh>; + close $fh; + return defined($first) && $first =~ /DO NOT EDIT/i && $first =~ /GENERATED/i; +} + +# ============================================================ +# Main +# ============================================================ + +my ($help, $quiet, $scan_mode); +GetOptions( + 'h|help' => \$help, + 'q|quiet' => \$quiet, + 'scan' => \$scan_mode, +) or exit 2; + +if ($help) { + # Extract usage from this script's header comment (lines 3 through + # the first non-comment line, with the leading "# " prefix stripped). + open my $fh, '<', $0 or die; + while (<$fh>) { + last unless /^#/; + next if $. < 3; + s/^# ?//; + print; + } + exit 0; +} + +unless (@ARGV) { + warn "ERROR: no files specified\n"; + exit 2; +} + +# Expand directories to *.sql and *.sql.in files (cat_tools authors the +# hand-maintained SQL source as a .sql.in template expanded at build time), +# excluding dependency dirs. +my @files; +for my $arg (@ARGV) { + if (-d $arg) { + File::Find::find({ + wanted => sub { + return unless -f && /\.sql(?:\.in)?$/; + my $p = $File::Find::name; + return if $p =~ m{/(?:deps|docker/deps|pgxntool|\.claude/worktrees)/}; + push @files, $p; + }, + no_chdir => 1, + }, $arg); + } elsif (-f $arg) { + push @files, $arg; + } else { + warn "WARNING: $arg: no such file or directory\n"; + } +} +@files = sort @files; + +unless (@files) { + warn "ERROR: no SQL files found\n"; + exit 2; +} + +# --scan: dump annotated lines and exit (for debugging and scanner tests). +if ($scan_mode) { + for my $file (@files) { + my $lines = scan_file($file); + printf "%d|%s|%s|%s\n", $_->{lineno}, $_->{state}, + ($_->{in_string} ? 'S' : '.'), $_->{text} for @$lines; + } + exit 0; +} + +# Run all rules on all files. +my @findings; +for my $file (@files) { + next if file_is_generated($file); + my $lines = scan_file($file); + for my $rule (@rules) { + push @findings, $rule->{check}->($file, $lines); + } +} + +# Sort by file, then line number. +@findings = sort { $a->{file} cmp $b->{file} || $a->{line} <=> $b->{line} } @findings; + +# Display. +printf "%s:%d: [%s] %s\n", $_->{file}, $_->{line}, $_->{rule}, $_->{message} + for @findings; + +# Summary and exit code. +my $count = scalar @findings; +if (!$quiet && $count > 0) { + printf STDERR "\n%d finding(s).\n", $count; +} +if ($count > 0) { + my $code = $count + 10; + $code = 255 if $code > 255; + exit $code; +} diff --git a/lint/sql/test/01-fixtures.t b/lint/sql/test/01-fixtures.t new file mode 100644 index 0000000..42218d8 --- /dev/null +++ b/lint/sql/test/01-fixtures.t @@ -0,0 +1,72 @@ +#!/usr/bin/env perl +# +# Data-driven fixture tests for sql-lint. +# +# Good fixtures (*.good.sql): must produce zero findings. +# Bad fixtures (*.bad.sql): must have a "-- expect-findings: N" header +# and produce exactly N findings. +# +# Adding a new fixture requires NO changes to this file. + +use strict; +use warnings; +use Test::More; +use File::Basename; +use File::Spec; + +my $test_dir = dirname(__FILE__); +my $lint = File::Spec->catfile($test_dir, '..', 'bin', 'sql-lint'); + +# -- Good fixtures: zero findings each ---------------------------------------- + +my @good = sort glob("$test_dir/fixtures/*.good.sql"); +ok(@good > 0, 'found good fixtures') or BAIL_OUT('no good fixtures'); + +subtest 'good fixtures produce zero findings' => sub { + plan tests => scalar @good; + for my $file (@good) { + my $name = basename($file); + my $output = `$lint -q "$file" 2>&1`; + my $rc = $? >> 8; + is($rc, 0, "$name: clean") or diag($output); + } +}; + +# -- Bad fixtures: expected finding counts from header ------------------------- + +my @bad = sort glob("$test_dir/fixtures/*.bad.sql"); +ok(@bad > 0, 'found bad fixtures') or BAIL_OUT('no bad fixtures'); + +subtest 'bad fixtures produce expected findings' => sub { + plan tests => scalar @bad * 2; + for my $file (@bad) { + my $name = basename($file); + my $expected = parse_expect($file); + + if (!defined $expected) { + fail("$name: missing '-- expect-findings: N' header"); + fail("$name: (skipped count check)"); + next; + } + + my $output = `$lint -q "$file" 2>&1`; + my $rc = $? >> 8; + isnt($rc, 0, "$name: non-zero exit"); + + my @lines = grep { /\S/ } split /\n/, $output; + is(scalar @lines, $expected, "$name: $expected finding(s)") + or diag($output); + } +}; + +done_testing(); + +sub parse_expect { + my ($file) = @_; + open my $fh, '<', $file or return undef; + while (<$fh>) { + return $1 if /^--\s*expect-findings:\s*(\d+)/; + last unless /^--/; + } + return undef; +} diff --git a/lint/sql/test/02-scanner.t b/lint/sql/test/02-scanner.t new file mode 100644 index 0000000..df7b5fe --- /dev/null +++ b/lint/sql/test/02-scanner.t @@ -0,0 +1,287 @@ +#!/usr/bin/env perl +# +# Scanner behavior and regression tests for sql-lint. +# +# Use this file for: +# - Scanner-specific edge cases (state transitions, nesting) +# - Regression tests for specific bugs (1-5 line SQL snippets) +# - Behavior assertions on finding content (not just count) +# +# Use fixtures (01-fixtures.t) for whole-file patterns instead. + +use strict; +use warnings; +use Test::More; +use File::Basename; +use File::Spec; +use File::Temp qw(tempfile tempdir); + +my $test_dir = dirname(__FILE__); +my $lint = File::Spec->catfile($test_dir, '..', 'bin', 'sql-lint'); + +# -- Helpers ------------------------------------------------------------------- + +sub lint_string { + my ($sql) = @_; + my ($fh, $tmp) = tempfile(SUFFIX => '.sql', UNLINK => 1); + print $fh $sql; + close $fh; + my $output = `$lint -q "$tmp" 2>&1`; + my $rc = $? >> 8; + return ($rc, $output); +} + +# Write $sql to $filename inside a fresh temp directory, then lint the +# directory itself (not the file directly) โ€” for asserting which extensions +# the directory walk picks up. +sub lint_dir_with_file { + my ($filename, $sql) = @_; + my $dir = tempdir(CLEANUP => 1); + my $path = File::Spec->catfile($dir, $filename); + open my $fh, '>', $path or die "cannot write $path: $!"; + print $fh $sql; + close $fh; + my $output = `$lint -q "$dir" 2>&1`; + my $rc = $? >> 8; + return ($rc, $output); +} + +sub scan_string { + my ($sql) = @_; + my ($fh, $tmp) = tempfile(SUFFIX => '.sql', UNLINK => 1); + print $fh $sql; + close $fh; + return `$lint --scan "$tmp" 2>&1`; +} + +# -- Scanner tests ------------------------------------------------------------- + +subtest 'scanner: CODE lines outside comments' => sub { + my $out = scan_string("SELECT 1;\n"); + like($out, qr/^1\|CODE\|\.\|SELECT 1;$/, 'basic CODE line'); +}; + +subtest 'scanner: block comment labeled COMMENT' => sub { + my $out = scan_string("/*\n * comment\n */\nSELECT 1;\n"); + my @lines = split /\n/, $out; + like($lines[0], qr/\|CODE\|.\|/, 'opening /* is CODE'); + like($lines[1], qr/\|COMMENT\|.\|/, 'body is COMMENT'); + like($lines[2], qr/\|COMMENT\|.\|/, 'closing */ is COMMENT'); + like($lines[3], qr/\|CODE\|.\|/, 'after close is CODE'); +}; + +subtest 'scanner: nested block comments' => sub { + my $out = scan_string("/* outer\n /* inner */\nstill outer\n*/\nSELECT 1;\n"); + my @lines = split /\n/, $out; + like($lines[2], qr/\|COMMENT\|.\|/, 'inner */ only closes inner โ€” still in outer'); + like($lines[3], qr/\|COMMENT\|.\|/, 'outer closing */ is COMMENT'); + like($lines[4], qr/\|CODE\|.\|/, 'after outer close is CODE'); +}; + +subtest 'scanner: /* inside string is not a comment' => sub { + my $out = scan_string("SELECT '/* not a comment */';\nSELECT 2;\n"); + my @lines = split /\n/, $out; + like($lines[0], qr/\|CODE\|.\|/, 'line with /* in string is CODE'); + like($lines[1], qr/\|CODE\|.\|/, 'next line is still CODE'); +}; + +subtest 'scanner: -- prevents /* from opening block' => sub { + my $out = scan_string("SELECT 1; -- /* not a block comment\nSELECT 2;\n"); + my @lines = split /\n/, $out; + like($lines[1], qr/\|CODE\|.\|/, 'next line is CODE (/* was in line comment)'); +}; + +subtest 'scanner: /* in comment body increments depth' => sub { + # This is PostgreSQL-correct: /* inside /* */ is nested. + # The single */ only closes the inner one. + my $out = scan_string("/*\n * mentions /* in prose\n */\nSELECT 1;\n"); + my @lines = split /\n/, $out; + like($lines[3], qr/\|COMMENT\|.\|/, + 'SELECT after */ is still COMMENT โ€” nested /* needs its own */'); +}; + +# -- Regression tests ---------------------------------------------------------- + +subtest 'comma inside single-quoted string is not flagged' => sub { + my ($rc, $out) = lint_string("SELECT 'hello,';\n"); + is($rc, 0, 'exit 0'); +}; + +subtest 'pure -- comment line with comma not flagged as trailing-comma' => sub { + my ($rc, $out) = lint_string("-- SELECT a,\n-- SELECT b,\nSELECT 1;\n"); + unlike($out, qr/trailing-comma/, 'no trailing-comma finding'); +}; + +subtest '/* inside single-quoted string not flagged as comment-opening' => sub { + my ($rc, $out) = lint_string("SELECT '/* not a comment';\n"); + is($rc, 0, 'exit 0'); +}; + +subtest 'comma after /* on same line not flagged as trailing-comma' => sub { + my ($rc, $out) = lint_string("/* text,\n */\nSELECT 1;\n"); + unlike($out, qr/trailing-comma/, 'no trailing-comma finding'); +}; + +subtest '/* */ inside -- comment not flagged as comment-single-line' => sub { + my ($rc, $out) = lint_string("-- Use /* */ for multi-line comments.\nSELECT 1;\n"); + unlike($out, qr/comment-single-line/, 'no comment-single-line finding'); +}; + +subtest '/* */ inside string not flagged as comment-single-line' => sub { + my ($rc, $out) = lint_string("SELECT '/* not a comment */';\n"); + unlike($out, qr/comment-single-line/, 'no comment-single-line finding'); +}; + +subtest 'properly formatted block comment is clean' => sub { + my ($rc, $out) = lint_string("/*\n * First line.\n *\n * Third line.\n */\nSELECT 1;\n"); + unlike($out, qr/comment-line-prefix/, 'no comment-line-prefix finding'); +}; + +subtest 'indented block comment is clean' => sub { + my $sql = <<'SQL'; +CREATE FUNCTION f() RETURNS void LANGUAGE plpgsql AS $body$ +BEGIN + /* + * Indented block comment. + */ + RAISE NOTICE 'hi'; +END; +$body$; +SQL + my ($rc, $out) = lint_string($sql); + unlike($out, qr/comment-line-prefix/, 'no comment-line-prefix finding'); + unlike($out, qr/comment-closing/, 'no comment-closing finding'); +}; + +subtest 'closing */ alone on its line is clean' => sub { + my ($rc, $out) = lint_string("/*\n * Comment.\n */\nSELECT 1;\n"); + unlike($out, qr/comment-closing/, 'no comment-closing finding'); +}; + +subtest 'findings are sorted by line number' => sub { + my ($rc, $out) = lint_string("-- line one\n-- line two\n\n/* bad opening text\n */\n\nSELECT a,\n"); + my @nums = $out =~ /:(\d+):/g; + my @sorted = sort { $a <=> $b } @nums; + is_deeply(\@nums, \@sorted, 'findings in line order'); +}; + +subtest 'empty file produces no findings' => sub { + my ($rc, $out) = lint_string(''); + is($rc, 0, 'exit 0'); +}; + +subtest 'exit code encodes finding count' => sub { + my ($rc, $out) = lint_string("-- line1\n-- line2\n"); + is($rc, 11, 'exit 11 = 1 finding + 10'); +}; + +subtest '-- /* line not flagged as comment-opening' => sub { + my ($rc, $out) = lint_string("-- /* this is a line comment\nSELECT 1;\n"); + unlike($out, qr/comment-opening/, 'no comment-opening finding'); +}; + +subtest 'code after inline /* -- */ comment is not lost' => sub { + my ($rc, $out) = lint_string("CREATE TABLE t(a /* flag -- TODO */ integer);\n"); + like($out, qr/prefer-short-type/, 'integer after inline comment is still linted'); +}; + +subtest 'verbose type suggestions are correct' => sub { + my ($rc, $out) = lint_string("CREATE TABLE t(a integer, b bool, c int2);\n"); + like($out, qr/"int" instead of "integer"/, 'integer -> int'); + like($out, qr/"boolean" instead of "bool"/, 'bool -> boolean'); + like($out, qr/"smallint" instead of "int2"/, 'int2 -> smallint'); +}; + +subtest 'inline suppression prevents finding' => sub { + my ($rc, $out) = lint_string( + "SELECT a, -- sql-lint:disable trailing-comma\n"); + is($rc, 0, 'suppressed finding exits 0'); +}; + +subtest 'block suppression: disable-block and EXCLUDED CODE alias' => sub { + # Unprefixed code lines inside a /* */ block trip comment-line-prefix. + my $body = "raw line, not prefixed\nSELECT not_ready(\n foo\n , bar\n);\n"; + + my ($rc_bare) = lint_string("/*\n$body*/\nSELECT 1;\n"); + isnt($rc_bare, 0, 'plain /* */ block flags the unprefixed lines'); + + my ($rc_db) = lint_string("/* sql-lint:disable-block all\n$body*/\nSELECT 1;\n"); + is($rc_db, 0, 'sql-lint:disable-block all suppresses the whole block'); + + my ($rc_ex) = lint_string("/* EXCLUDED CODE\n$body*/\nSELECT 1;\n"); + is($rc_ex, 0, 'EXCLUDED CODE alias suppresses the whole block'); + + my ($rc_ex2) = lint_string("/* EXCLUDED CODE: disabled until #123\n$body*/\nSELECT 1;\n"); + is($rc_ex2, 0, 'EXCLUDED CODE with a trailing description still suppresses'); +}; + +subtest 'multi-line single-quoted string keeps state across lines' => sub { + my $out = scan_string("SELECT 'start\n/* inside the string\nend';\nSELECT 2;\n"); + my @lines = split /\n/, $out; + like($lines[1], qr/\|CODE\|S\|/, 'continuation line inside string has in_string=S'); + like($lines[3], qr/\|CODE\|\.\|/, 'code after string close is CODE with no in_string'); +}; + +subtest 'multi-line string does not cause trailing-comma false positive' => sub { + my ($rc, $out) = lint_string("SELECT 'hello,\nworld';\n"); + unlike($out, qr/trailing-comma/, 'comma inside multi-line string not flagged'); +}; + +subtest 'multi-line string continuation line not checked for types' => sub { + my ($rc, $out) = lint_string("SELECT 'integer\nbool';\n"); + unlike($out, qr/prefer-short-type/, 'type name inside multi-line string not flagged'); +}; + +subtest 'trailing comma before -- with no leading space is flagged' => sub { + # code_content must strip the line comment even without whitespace before --, + # matching the scanner which treats -- as a comment start unconditionally. + my ($rc, $out) = lint_string("SELECT a,--comment\nSELECT 1;\n"); + like($out, qr/trailing-comma/, 'trailing comma detected when -- has no leading space'); +}; + +subtest '-- with no leading space hides a following /* (no false comment-opening)' => sub { + # `1--x /* y`: -- starts a line comment, so the /* is inside it and must not + # be treated as a block-comment opener. The per-rule -- stripping must match + # the scanner (no leading-whitespace requirement). + my ($rc, $out) = lint_string("SELECT 1--x /* y\nSELECT 2;\n"); + unlike($out, qr/comment-opening/, 'no false comment-opening when -- has no leading space'); +}; + +subtest '-- with no leading space hides a following /* */ (no false comment-single-line)' => sub { + my ($rc, $out) = lint_string("SELECT 1--x /* c */\nSELECT 2;\n"); + unlike($out, qr/comment-single-line/, 'no false comment-single-line when -- has no leading space'); +}; + +# -- Edge cases ---------------------------------------------------------------- + +subtest 'no arguments prints usage and exits 2' => sub { + my $out = `$lint 2>&1`; + my $rc = $? >> 8; + is($rc, 2, 'exit 2 for no args'); +}; + +subtest 'nonexistent file exits 2' => sub { + my $out = `$lint -q /nonexistent/file.sql 2>&1`; + my $rc = $? >> 8; + is($rc, 2, 'exit 2 for missing file'); +}; + +subtest 'directory scanning finds files' => sub { + my ($rc, $out) = lint_string("SELECT 1;\n"); + is($rc, 0, 'clean file via directory'); +}; + +subtest 'directory scanning finds .sql.in files' => sub { + # cat_tools authors the hand-maintained SQL source as a .sql.in template + # expanded at build time; the directory walk must not skip it. + my ($rc, $out) = lint_dir_with_file('example.sql.in', "SELECT a,\n"); + like($out, qr/trailing-comma/, '.sql.in file was scanned and linted'); +}; + +subtest '.sql.in generated-file marker still skips the file' => sub { + my ($rc, $out) = lint_dir_with_file('example.sql.in', + "-- GENERATED FILE! DO NOT EDIT! See source.sql.in\nSELECT a,\n"); + is($rc, 0, 'marker (GENERATED ... DO NOT EDIT, either order) skips the file'); +}; + +done_testing(); diff --git a/lint/sql/test/fixtures/comment-closing.bad.sql b/lint/sql/test/fixtures/comment-closing.bad.sql new file mode 100644 index 0000000..17fd7dd --- /dev/null +++ b/lint/sql/test/fixtures/comment-closing.bad.sql @@ -0,0 +1,7 @@ +-- expect-findings: 2 +/* + * Text before the closing marker. */ + +/* + * Another comment + * with text before closing */ diff --git a/lint/sql/test/fixtures/comment-closing.good.sql b/lint/sql/test/fixtures/comment-closing.good.sql new file mode 100644 index 0000000..c2e5646 --- /dev/null +++ b/lint/sql/test/fixtures/comment-closing.good.sql @@ -0,0 +1,7 @@ +/* + * Closing marker is on its own line. + */ + +/* + * Another properly closed comment. + */ diff --git a/lint/sql/test/fixtures/comment-line-prefix.bad.sql b/lint/sql/test/fixtures/comment-line-prefix.bad.sql new file mode 100644 index 0000000..428fda6 --- /dev/null +++ b/lint/sql/test/fixtures/comment-line-prefix.bad.sql @@ -0,0 +1,6 @@ +-- expect-findings: 2 +/* + Missing the star prefix on this line. + * This line is fine. +also missing the star prefix. + */ diff --git a/lint/sql/test/fixtures/comment-line-prefix.good.sql b/lint/sql/test/fixtures/comment-line-prefix.good.sql new file mode 100644 index 0000000..9b87003 --- /dev/null +++ b/lint/sql/test/fixtures/comment-line-prefix.good.sql @@ -0,0 +1,20 @@ +/* + * Every content line starts with " * ". + * Including this one. + */ + +/* sql-lint:disable-block all +A disable-block directive on the opening line exempts the whole block, so code +commented out like this needs no " * " prefixes on each line. +SELECT not_ready_yet( + foo + , bar +); +*/ + +/* EXCLUDED CODE โ€” the friendly alias for disable-block all, for commented-out code +SELECT also_not_ready( + foo + , bar +); +*/ diff --git a/lint/sql/test/fixtures/comment-opening.bad.sql b/lint/sql/test/fixtures/comment-opening.bad.sql new file mode 100644 index 0000000..74259ac --- /dev/null +++ b/lint/sql/test/fixtures/comment-opening.bad.sql @@ -0,0 +1,7 @@ +-- expect-findings: 2 +/* This text should not be on the opening line + * of a multi-line comment. + */ + +/* Also bad: text after the opening + */ diff --git a/lint/sql/test/fixtures/comment-opening.good.sql b/lint/sql/test/fixtures/comment-opening.good.sql new file mode 100644 index 0000000..b703aff --- /dev/null +++ b/lint/sql/test/fixtures/comment-opening.good.sql @@ -0,0 +1,9 @@ +/* + * Opening marker is alone on its line โ€” correct. + */ + +/* + * Another properly opened block comment. + */ + +-- /* this is a line comment, not a block comment opening diff --git a/lint/sql/test/fixtures/comment-padding.bad.sql b/lint/sql/test/fixtures/comment-padding.bad.sql new file mode 100644 index 0000000..f62ff6b --- /dev/null +++ b/lint/sql/test/fixtures/comment-padding.bad.sql @@ -0,0 +1,10 @@ +-- expect-findings: 2 +/* + * + * Leading blank line above is unnecessary. + */ + +/* + * Trailing blank line below is unnecessary. + * + */ diff --git a/lint/sql/test/fixtures/comment-padding.good.sql b/lint/sql/test/fixtures/comment-padding.good.sql new file mode 100644 index 0000000..428528e --- /dev/null +++ b/lint/sql/test/fixtures/comment-padding.good.sql @@ -0,0 +1,9 @@ +/* + * No padding lines here. + */ + +/* + * Interior blank lines are fine. + * + * Like the one above. + */ diff --git a/lint/sql/test/fixtures/comment-single-line.bad.sql b/lint/sql/test/fixtures/comment-single-line.bad.sql new file mode 100644 index 0000000..9e1e56a --- /dev/null +++ b/lint/sql/test/fixtures/comment-single-line.bad.sql @@ -0,0 +1,4 @@ +-- expect-findings: 2 +/* This is a single-line block comment. */ + +/* Also caught: inline with code */ SELECT 1; diff --git a/lint/sql/test/fixtures/comment-single-line.good.sql b/lint/sql/test/fixtures/comment-single-line.good.sql new file mode 100644 index 0000000..e946f34 --- /dev/null +++ b/lint/sql/test/fixtures/comment-single-line.good.sql @@ -0,0 +1,7 @@ +-- Single-line comments are fine. + +/* + * Multi-line block comments are fine. + */ + +/* suppressed single-line block comment */ -- sql-lint:disable comment-single-line diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql b/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql new file mode 100644 index 0000000..8caaa7a --- /dev/null +++ b/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql @@ -0,0 +1,7 @@ +-- expect-findings: 2 +-- This is a multi-line comment +-- written with stacked dash lines. +-- It should use /* */ instead. + +-- This is another +-- stacked comment. diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.good.sql b/lint/sql/test/fixtures/comment-stacked-dashes.good.sql new file mode 100644 index 0000000..757b2de --- /dev/null +++ b/lint/sql/test/fixtures/comment-stacked-dashes.good.sql @@ -0,0 +1,17 @@ +-- A single-line comment is fine. + +-- Another single-line comment, separated by a blank line. + +-- /* this is a line comment, not a block comment opening + +--/* no space variant + +-- This is stacked -- sql-lint:disable comment-stacked-dashes +-- but suppressed. + +/* + * This block comment contains lines that look like stacked dashes: + * -- line one + * -- line two + * But they are inside a block comment and should NOT be flagged. + */ diff --git a/lint/sql/test/fixtures/generated-file-skipped.good.sql b/lint/sql/test/fixtures/generated-file-skipped.good.sql new file mode 100644 index 0000000..187f5b4 --- /dev/null +++ b/lint/sql/test/fixtures/generated-file-skipped.good.sql @@ -0,0 +1,5 @@ +/* DO NOT EDIT - AUTO-GENERATED FILE */ +/* This single-line block comment would trip comment-single-line, and the stacked +-- dashes below would trip comment-stacked-dashes, but a file whose first line is +-- the AUTO-GENERATED marker is skipped entirely, so this fixture yields zero +-- findings. */ diff --git a/lint/sql/test/fixtures/prefer-short-type.bad.sql b/lint/sql/test/fixtures/prefer-short-type.bad.sql new file mode 100644 index 0000000..0a403d1 --- /dev/null +++ b/lint/sql/test/fixtures/prefer-short-type.bad.sql @@ -0,0 +1,12 @@ +-- expect-findings: 9 +CREATE TABLE long_types( + id integer NOT NULL PRIMARY KEY + , flag bool NOT NULL + , name character varying(100) NOT NULL + , amount decimal NOT NULL + , score float8 + , small_val int2 + , big_val int8 + , approx float4 + , internal int4 +); diff --git a/lint/sql/test/fixtures/prefer-short-type.good.sql b/lint/sql/test/fixtures/prefer-short-type.good.sql new file mode 100644 index 0000000..8d84667 --- /dev/null +++ b/lint/sql/test/fixtures/prefer-short-type.good.sql @@ -0,0 +1,19 @@ +CREATE TABLE short_types( + id int NOT NULL PRIMARY KEY + , flag boolean NOT NULL + , name varchar(100) NOT NULL + , label text NOT NULL + , amount numeric NOT NULL + , score double precision + , small_val smallint + , big_val bigint + , approx real + , ts timestamp + , tstz timestamptz +); +/* + * Type names inside identifiers should not be flagged. + * "is_real_value", "integer_count", "boolean_flag" are identifiers, not types. + */ +CREATE FUNCTION is_real_value(integer_count int, boolean_flag boolean) RETURNS boolean +LANGUAGE sql AS $$SELECT true$$; diff --git a/lint/sql/test/fixtures/trailing-comma.bad.sql b/lint/sql/test/fixtures/trailing-comma.bad.sql new file mode 100644 index 0000000..d20b727 --- /dev/null +++ b/lint/sql/test/fixtures/trailing-comma.bad.sql @@ -0,0 +1,30 @@ +-- expect-findings: 7 +SELECT + column1, + column2, + column3 +FROM some_table +; + +CREATE TABLE bad_table( + id serial NOT NULL PRIMARY KEY, + name text NOT NULL, + status text +); + +INSERT INTO t(a, b, c) +VALUES ( + 1, + 2, + 3 +); + +/* + * A genuine trailing comma after a lone string literal must still be caught + * (the sentinel keeps the comma on the trailing side after string-stripping). + */ +INSERT INTO t(a) +VALUES ( + 'x', + 'y' +); diff --git a/lint/sql/test/fixtures/trailing-comma.good.sql b/lint/sql/test/fixtures/trailing-comma.good.sql new file mode 100644 index 0000000..405a6d6 --- /dev/null +++ b/lint/sql/test/fixtures/trailing-comma.good.sql @@ -0,0 +1,46 @@ +/* + * Lines that are pure -- comments should not trigger trailing-comma, + * even if the comment text contains a comma. + */ + +-- This is a comment with a trailing comma, + +-- And another one with a comma, + +SELECT column1 -- inline comment + , column2 -- another inline comment + , column3 +FROM some_table +; + +SELECT + column1, -- sql-lint:disable trailing-comma + column2 +; + +/* + * Leading-comma items whose entire payload is a string literal are correct + * leading-comma style and must NOT be flagged. (Regression: stripping the + * string used to leave a bare leading comma that looked like a trailing one.) + */ +SELECT is( + 1 + , 'a description' + , 'another, with an embedded comma' +); + +CALL pg_temp.routine( + 'FUNCTION' + , 'clean_routine_args' + , 'args text' +); + +-- A leading comma may sit alone on its line when it introduces a multi-line item. +SELECT row_eq( + 'SELECT ' || call + , row(out) + , + 'SELECT ' || call + || ' should return ' + || coalesce(out::text, 'NULL') +); diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index 9d66d63..f9ff3c7 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -194,7 +194,7 @@ BEGIN */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' /* invalid_authorization_specification */ + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('current_user is %s, session_user is %s', current_user, session_user) , HINT = 'Helper functions must not be called from SECURITY DEFINER context.'; @@ -250,7 +250,7 @@ BEGIN */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' /* invalid_authorization_specification */ + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('API function %s must not be called from a SECURITY DEFINER function', api_function_name) , HINT = 'We detect SECURITY DEFINER context by comparing current_user and session_user, which can cause false positives if SET ROLE is used'; @@ -775,8 +775,10 @@ CREATE TYPE cat_tools.object_type AS ENUM( , 'materialized view column' , 'composite type column' , 'foreign table column' - -- pg_constraint - -- NOTE: a domain itself is considered to be a type + /* + * pg_constraint + * NOTE: a domain itself is considered to be a type + */ , 'domain constraint', 'table constraint' -- pg_proc , 'aggregate', 'function' @@ -936,8 +938,10 @@ SELECT ( WHEN object_type::text LIKE '% column' THEN 'pg_attribute' ELSE CASE object_type - -- Unusual cases - -- s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ + /* + * Unusual cases + * s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ + */ WHEN 'default value' THEN 'pg_attrdef' WHEN 'large object' THEN 'pg_largeobject' WHEN 'operator class' THEN 'pg_opclass' @@ -1285,38 +1289,38 @@ $$ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS - SELECT n1.nspname AS fk_schema_name, - c1.relname AS fk_table_name, - k1.conname AS fk_constraint_name, - c1.oid AS fk_table_oid, - _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns, - n2.nspname AS pk_schema_name, - c2.relname AS pk_table_name, - k2.conname AS pk_constraint_name, - c2.oid AS pk_table_oid, - ci.relname AS pk_index_name, - _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns, - CASE k1.confmatchtype WHEN 'f' THEN 'FULL' + SELECT n1.nspname AS fk_schema_name + , c1.relname AS fk_table_name + , k1.conname AS fk_constraint_name + , c1.oid AS fk_table_oid + , _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns + , n2.nspname AS pk_schema_name + , c2.relname AS pk_table_name + , k2.conname AS pk_constraint_name + , c2.oid AS pk_table_oid + , ci.relname AS pk_index_name + , _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns + , CASE k1.confmatchtype WHEN 'f' THEN 'FULL' WHEN 'p' THEN 'PARTIAL' WHEN 'u' THEN 'NONE' else null - END AS match_type, - CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ + END AS match_type + , CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' else null - END AS on_delete, - CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' + END AS on_delete + , CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' ELSE NULL - END AS on_update, - k1.condeferrable AS is_deferrable, -- @generated@ - k1.condeferred AS is_deferred + END AS on_update + , k1.condeferrable AS is_deferrable -- @generated@ + , k1.condeferred AS is_deferred FROM pg_catalog.pg_constraint k1 JOIN pg_catalog.pg_namespace n1 ON (n1.oid = k1.connamespace) JOIN pg_catalog.pg_class c1 ON (c1.oid = k1.conrelid) diff --git a/sql/cat_tools.sql.in b/sql/cat_tools.sql.in index 9d66d63..f9ff3c7 100644 --- a/sql/cat_tools.sql.in +++ b/sql/cat_tools.sql.in @@ -194,7 +194,7 @@ BEGIN */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' /* invalid_authorization_specification */ + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('current_user is %s, session_user is %s', current_user, session_user) , HINT = 'Helper functions must not be called from SECURITY DEFINER context.'; @@ -250,7 +250,7 @@ BEGIN */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' /* invalid_authorization_specification */ + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('API function %s must not be called from a SECURITY DEFINER function', api_function_name) , HINT = 'We detect SECURITY DEFINER context by comparing current_user and session_user, which can cause false positives if SET ROLE is used'; @@ -775,8 +775,10 @@ CREATE TYPE cat_tools.object_type AS ENUM( , 'materialized view column' , 'composite type column' , 'foreign table column' - -- pg_constraint - -- NOTE: a domain itself is considered to be a type + /* + * pg_constraint + * NOTE: a domain itself is considered to be a type + */ , 'domain constraint', 'table constraint' -- pg_proc , 'aggregate', 'function' @@ -936,8 +938,10 @@ SELECT ( WHEN object_type::text LIKE '% column' THEN 'pg_attribute' ELSE CASE object_type - -- Unusual cases - -- s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ + /* + * Unusual cases + * s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ + */ WHEN 'default value' THEN 'pg_attrdef' WHEN 'large object' THEN 'pg_largeobject' WHEN 'operator class' THEN 'pg_opclass' @@ -1285,38 +1289,38 @@ $$ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS - SELECT n1.nspname AS fk_schema_name, - c1.relname AS fk_table_name, - k1.conname AS fk_constraint_name, - c1.oid AS fk_table_oid, - _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns, - n2.nspname AS pk_schema_name, - c2.relname AS pk_table_name, - k2.conname AS pk_constraint_name, - c2.oid AS pk_table_oid, - ci.relname AS pk_index_name, - _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns, - CASE k1.confmatchtype WHEN 'f' THEN 'FULL' + SELECT n1.nspname AS fk_schema_name + , c1.relname AS fk_table_name + , k1.conname AS fk_constraint_name + , c1.oid AS fk_table_oid + , _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns + , n2.nspname AS pk_schema_name + , c2.relname AS pk_table_name + , k2.conname AS pk_constraint_name + , c2.oid AS pk_table_oid + , ci.relname AS pk_index_name + , _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns + , CASE k1.confmatchtype WHEN 'f' THEN 'FULL' WHEN 'p' THEN 'PARTIAL' WHEN 'u' THEN 'NONE' else null - END AS match_type, - CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ + END AS match_type + , CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' else null - END AS on_delete, - CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' + END AS on_delete + , CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' ELSE NULL - END AS on_update, - k1.condeferrable AS is_deferrable, -- @generated@ - k1.condeferred AS is_deferred + END AS on_update + , k1.condeferrable AS is_deferrable -- @generated@ + , k1.condeferred AS is_deferred FROM pg_catalog.pg_constraint k1 JOIN pg_catalog.pg_namespace n1 ON (n1.oid = k1.connamespace) JOIN pg_catalog.pg_class c1 ON (c1.oid = k1.conrelid) diff --git a/test/sql/pg_depends.sql b/test/sql/pg_depends.sql index ff149f7..e1c411f 100644 --- a/test/sql/pg_depends.sql +++ b/test/sql/pg_depends.sql @@ -11,7 +11,7 @@ CREATE TEMP VIEW views AS ; GRANT SELECT ON views TO public; -/* +/* EXCLUDED CODE CREATE TEMP VIEW func_calls AS SELECT * FROM (VALUES ('pg_attribute__get'::name, $$'pg_class', 'relname'$$::text) From d103c936c6abc7235db0ba75e5c9c0f8dee75c8a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 16:25:38 -0500 Subject: [PATCH 02/11] Address review feedback on sql-lint port - lint/sql/test/fixtures/prefer-short-type.{bad,good}.sql: rename the id column to table_name_id. - comment-stacked-dashes: allow up to 2 consecutive -- lines; only 3+ must become a /* */ block. Updated fixtures/docs and reverted the two cat_tools.sql.in spots that were converted to /* */ under the old threshold back to -- (both are exactly 2 lines). - Added a new region-suppression pair, disable-block/enable-block on their own -- lines, for silencing every rule across a chunk of real code (as opposed to the existing /* */-anchored disable-block, which only applies inside a comment and auto-closes at the comment's own */). Used it to wrap the borrowed pg_all_foreign_keys view in cat_tools.sql.in, which is also reverted back to its original trailing-comma form -- that borrow was intentionally left unreformatted. - Dropped the sed-generation-artifact regex comment in cat_tools.sql.in (object_type case expression); it was a personal note on how the WHEN clauses were originally generated, not documentation worth keeping. - test/sql/pg_depends.sql: removed the commented-out func_calls view instead of marking it EXCLUDED CODE. - sql.mk: version-specific install/update scripts (sql/cat_tools--X.Y.Z.sql) now get an extra "VERSIONED FILE!" tag ahead of the existing GENERATED FILE marker, so it's obvious at a glance that a file is a frozen snapshot that must never be hand-edited -- distinct from cat_tools.sql itself. The substitution is now anchored to whole-line @generated@ markers rather than a bare substring match, which incidentally also stops it from clobbering the handful of @generated@ occurrences that are trailing annotations on real code lines in cat_tools.sql.in (e.g. on the confdeltype CASE expression) rather than standalone section-divider markers. make -C lint test, make lint, make install, and make test all verified green after these changes. --- lint/sql/README.md | 27 ++++++++-- lint/sql/bin/sql-lint | 37 +++++++++++++- lint/sql/test/02-scanner.t | 22 +++++++- .../fixtures/comment-stacked-dashes.bad.sql | 3 +- .../fixtures/comment-stacked-dashes.good.sql | 8 ++- .../test/fixtures/prefer-short-type.bad.sql | 2 +- .../test/fixtures/prefer-short-type.good.sql | 2 +- sql.mk | 28 +++++++++- sql/cat_tools--0.3.0.sql.in | 51 +++++++++---------- sql/cat_tools.sql.in | 51 +++++++++---------- test/sql/pg_depends.sql | 9 ---- 11 files changed, 164 insertions(+), 76 deletions(-) diff --git a/lint/sql/README.md b/lint/sql/README.md index 3342ffa..909d436 100644 --- a/lint/sql/README.md +++ b/lint/sql/README.md @@ -115,15 +115,20 @@ single-line comments. ### comment-stacked-dashes -Multi-line comments must use `/* */` syntax, not consecutive `--` lines. +A run of 3 or more consecutive `--` lines must use `/* */` syntax instead. +Up to 2 consecutive `--` lines is fine โ€” a short remark reads fine as `--`. ```sql -- Bad: stacked dashes --- spanning multiple lines +-- spanning three +-- or more lines + +-- Good: 2 lines is fine +-- as a short remark /* * Good: block comment - * spanning multiple lines + * for anything longer */ ``` @@ -204,6 +209,22 @@ SELECT not_ready_yet( Text may follow it (e.g. `/* EXCLUDED CODE โ€” disabled until issue #123`). +### Region suppression (real code, not a comment) + +To suppress a chunk of actual code โ€” e.g. borrowed code deliberately left +unreformatted โ€” wrap it in `disable-block ` / `enable-block` +directives on their own `--` lines. Unlike the `/* */`-anchored form above, +this pair works outside a comment and is closed explicitly rather than by a +comment's own `*/` (if never closed, it runs to end of file): + +```sql +-- sql-lint:disable-block all +SELECT n1.nspname AS fk_schema_name, + c1.relname AS fk_table_name + FROM pg_catalog.pg_constraint k1 +-- sql-lint:enable-block +``` + ## Adding New Rules Add a subroutine to `bin/sql-lint` and register it in the `@rules` array: diff --git a/lint/sql/bin/sql-lint b/lint/sql/bin/sql-lint index 22216c2..e9e926a 100755 --- a/lint/sql/bin/sql-lint +++ b/lint/sql/bin/sql-lint @@ -158,13 +158,43 @@ sub code_skeleton { return $c; } +# Mark each line as inside a `sql-lint:disable-block ... enable-block` CODE +# region โ€” as opposed to the /* */-anchored disable-block above (auto-scoped +# by the comment's own close, untouched by this). This form works on real +# code, explicitly closed by `sql-lint:enable-block` (or open to EOF if never +# closed), so a whole chunk โ€” e.g. borrowed code deliberately left +# unreformatted โ€” can be silenced without wrapping it in a comment. Nested +# regions are tracked as a stack, so an inner region closes before an outer +# one. Stores results directly on each line hashref (region_all / a +# region_rules set) for is_suppressed() to check. +sub annotate_region_suppressions { + my ($lines) = @_; + my @stack; + for my $l (@$lines) { + if ($l->{text} =~ /sql-lint:disable-block\s+(\S+)/) { + push @stack, $1; + } + for my $r (@stack) { + if ($r eq 'all') { $l->{region_all} = 1 } + else { $l->{region_rules}{$r} = 1 } + } + if ($l->{text} =~ /sql-lint:enable-block\b/ && @stack) { + pop @stack; + } + } +} + # True if the finding is suppressed by an inline pragma. sub is_suppressed { my ($lines, $lineno, $rule_id) = @_; my $idx = $lineno - 1; return 0 if $idx < 0 || $idx > $#$lines; - my $text = $lines->[$idx]{text}; + my $l = $lines->[$idx]; + return 1 if $l->{region_all}; + return 1 if $l->{region_rules} && $l->{region_rules}{$rule_id}; + + my $text = $l->{text}; # Same-line directive. Matches `sql-lint:disable`, `disable-line`, or # `disable-block` (the optional `-line`/`-block` suffix), followed by either # this rule's id or the literal `all`. \Q...\E escapes any regex-special @@ -218,8 +248,10 @@ sub check_stacked_dashes { my @out; my ($start, $end) = (0, 0); + # Up to 2 consecutive -- lines is allowed (a short two-line remark reads + # fine as --); only 3+ must become a /* */ block. my $flush = sub { - return unless $start && ($end - $start) >= 1; + return unless $start && ($end - $start) >= 2; my $n = $end - $start + 1; maybe_finding(\@out, $lines, $file, $start, 'comment-stacked-dashes', "$n consecutive -- comment lines ($start-$end); use /* */ for multi-line comments"); @@ -501,6 +533,7 @@ my @findings; for my $file (@files) { next if file_is_generated($file); my $lines = scan_file($file); + annotate_region_suppressions($lines); for my $rule (@rules) { push @findings, $rule->{check}->($file, $lines); } diff --git a/lint/sql/test/02-scanner.t b/lint/sql/test/02-scanner.t index df7b5fe..fbd8a8d 100644 --- a/lint/sql/test/02-scanner.t +++ b/lint/sql/test/02-scanner.t @@ -171,7 +171,7 @@ subtest 'empty file produces no findings' => sub { }; subtest 'exit code encodes finding count' => sub { - my ($rc, $out) = lint_string("-- line1\n-- line2\n"); + my ($rc, $out) = lint_string("SELECT a,\n"); is($rc, 11, 'exit 11 = 1 finding + 10'); }; @@ -215,6 +215,26 @@ subtest 'block suppression: disable-block and EXCLUDED CODE alias' => sub { is($rc_ex2, 0, 'EXCLUDED CODE with a trailing description still suppresses'); }; +subtest 'region suppression: disable-block/enable-block around real code' => sub { + # Unlike the /* */-anchored disable-block above, this pair works on real + # (uncommented) code and is closed explicitly rather than by a comment's + # own */ -- e.g. for a chunk of borrowed code left deliberately + # unreformatted. + my ($rc_all, $out_all) = lint_string( + "-- sql-lint:disable-block all\nSELECT a,\nSELECT b integer,\n-- sql-lint:enable-block\nSELECT c,\n"); + isnt($rc_all, 0, 'finding after enable-block still flagged'); + unlike($out_all, qr/:2:|:3:/, 'no findings inside the disable-block/enable-block region'); + like($out_all, qr/:5:/, 'finding on line 5 (after enable-block) still reported'); + + my (undef, $out_rule) = lint_string( + "-- sql-lint:disable-block trailing-comma\nSELECT a integer,\n-- sql-lint:enable-block\n"); + like($out_rule, qr/prefer-short-type/, 'disabling one rule does not suppress other rules in the region'); + unlike($out_rule, qr/trailing-comma/, 'disabling one rule does suppress that rule in the region'); + + my ($rc_unclosed) = lint_string("-- sql-lint:disable-block all\nSELECT a,\nSELECT b,\n"); + is($rc_unclosed, 0, 'unclosed disable-block suppresses through EOF'); +}; + subtest 'multi-line single-quoted string keeps state across lines' => sub { my $out = scan_string("SELECT 'start\n/* inside the string\nend';\nSELECT 2;\n"); my @lines = split /\n/, $out; diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql b/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql index 8caaa7a..9b72b35 100644 --- a/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql +++ b/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql @@ -4,4 +4,5 @@ -- It should use /* */ instead. -- This is another --- stacked comment. +-- stacked comment +-- spanning three lines. diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.good.sql b/lint/sql/test/fixtures/comment-stacked-dashes.good.sql index 757b2de..6d14e02 100644 --- a/lint/sql/test/fixtures/comment-stacked-dashes.good.sql +++ b/lint/sql/test/fixtures/comment-stacked-dashes.good.sql @@ -2,12 +2,16 @@ -- Another single-line comment, separated by a blank line. +-- Two consecutive -- lines are also fine; +-- only a run of 3+ must become a /* */ block. + -- /* this is a line comment, not a block comment opening --/* no space variant --- This is stacked -- sql-lint:disable comment-stacked-dashes --- but suppressed. +-- This is a stacked comment -- sql-lint:disable comment-stacked-dashes +-- of three lines that would otherwise +-- be flagged, but is suppressed. /* * This block comment contains lines that look like stacked dashes: diff --git a/lint/sql/test/fixtures/prefer-short-type.bad.sql b/lint/sql/test/fixtures/prefer-short-type.bad.sql index 0a403d1..84db8a4 100644 --- a/lint/sql/test/fixtures/prefer-short-type.bad.sql +++ b/lint/sql/test/fixtures/prefer-short-type.bad.sql @@ -1,6 +1,6 @@ -- expect-findings: 9 CREATE TABLE long_types( - id integer NOT NULL PRIMARY KEY + long_types_id integer NOT NULL PRIMARY KEY , flag bool NOT NULL , name character varying(100) NOT NULL , amount decimal NOT NULL diff --git a/lint/sql/test/fixtures/prefer-short-type.good.sql b/lint/sql/test/fixtures/prefer-short-type.good.sql index 8d84667..1d83749 100644 --- a/lint/sql/test/fixtures/prefer-short-type.good.sql +++ b/lint/sql/test/fixtures/prefer-short-type.good.sql @@ -1,5 +1,5 @@ CREATE TABLE short_types( - id int NOT NULL PRIMARY KEY + short_types_id int NOT NULL PRIMARY KEY , flag boolean NOT NULL , name varchar(100) NOT NULL , label text NOT NULL diff --git a/sql.mk b/sql.mk index bd079ef..3154c2c 100644 --- a/sql.mk +++ b/sql.mk @@ -201,8 +201,25 @@ endef # The recipe builds $@.tmp then moves it into place atomically. # TODO: refactor the version handling into a function. +# +# The boundary/divider @generated@ markers (added by the echo's above, plus +# any standalone ones already in the source used as visual section dividers) +# are matched anchored to the WHOLE line -- not a plain substring -- so an +# @generated@ that coincidentally appears as a trailing annotation on a real +# code line (there are a few in cat_tools.sql.in) is left untouched instead +# of being clobbered into generic marker text. $* (the %-match) tells apart a +# version-specific file (stem contains --, e.g. cat_tools--0.2.2) from the +# master (stem is just cat_tools): version files get an extra "VERSIONED +# FILE!" tag up front so it's obvious at a glance that the file is a frozen +# snapshot that must never be hand-edited. sql/%.sql: sql/%.sql.in pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | awk -v src='$<' -v stem='$*' '\ + /^@generated@$$/ { \ + if (stem ~ /--/) print "-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See " src; \ + else print "-- GENERATED FILE! DO NOT EDIT! See " src; \ + next \ + } \ + { print }' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ @@ -216,7 +233,14 @@ $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control # cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with # version-conditional substitutions applied. GNU Make's "overriding recipe" # warning for this target is expected. +# +# $* is not meaningful for this static (non-pattern) rule, so unlike the +# pattern rule above this always tags with "VERSIONED FILE!" -- everything +# built here is, by definition (see EXTENSION_VERSION_FILES above), a +# version-specific file. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | awk -v src='$<' '\ + /^@generated@$$/ { print "-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See " src; next } \ + { print }' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index f9ff3c7..30b8020 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -775,10 +775,8 @@ CREATE TYPE cat_tools.object_type AS ENUM( , 'materialized view column' , 'composite type column' , 'foreign table column' - /* - * pg_constraint - * NOTE: a domain itself is considered to be a type - */ + -- pg_constraint + -- NOTE: a domain itself is considered to be a type , 'domain constraint', 'table constraint' -- pg_proc , 'aggregate', 'function' @@ -938,10 +936,7 @@ SELECT ( WHEN object_type::text LIKE '% column' THEN 'pg_attribute' ELSE CASE object_type - /* - * Unusual cases - * s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ - */ + -- Unusual cases WHEN 'default value' THEN 'pg_attrdef' WHEN 'large object' THEN 'pg_largeobject' WHEN 'operator class' THEN 'pg_opclass' @@ -1287,40 +1282,41 @@ $$ @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ +-- sql-lint:disable-block all CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS - SELECT n1.nspname AS fk_schema_name - , c1.relname AS fk_table_name - , k1.conname AS fk_constraint_name - , c1.oid AS fk_table_oid - , _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns - , n2.nspname AS pk_schema_name - , c2.relname AS pk_table_name - , k2.conname AS pk_constraint_name - , c2.oid AS pk_table_oid - , ci.relname AS pk_index_name - , _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns - , CASE k1.confmatchtype WHEN 'f' THEN 'FULL' + SELECT n1.nspname AS fk_schema_name, + c1.relname AS fk_table_name, + k1.conname AS fk_constraint_name, + c1.oid AS fk_table_oid, + _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns, + n2.nspname AS pk_schema_name, + c2.relname AS pk_table_name, + k2.conname AS pk_constraint_name, + c2.oid AS pk_table_oid, + ci.relname AS pk_index_name, + _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns, + CASE k1.confmatchtype WHEN 'f' THEN 'FULL' WHEN 'p' THEN 'PARTIAL' WHEN 'u' THEN 'NONE' else null - END AS match_type - , CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ + END AS match_type, + CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' else null - END AS on_delete - , CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' + END AS on_delete, + CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' ELSE NULL - END AS on_update - , k1.condeferrable AS is_deferrable -- @generated@ - , k1.condeferred AS is_deferred + END AS on_update, + k1.condeferrable AS is_deferrable, -- @generated@ + k1.condeferred AS is_deferred FROM pg_catalog.pg_constraint k1 JOIN pg_catalog.pg_namespace n1 ON (n1.oid = k1.connamespace) JOIN pg_catalog.pg_class c1 ON (c1.oid = k1.conrelid) @@ -1352,6 +1348,7 @@ AS AND k1.contype = 'f' AND _cat_tools._pg_sv_table_accessible(n1.oid, c1.oid) ; +-- sql-lint:enable-block GRANT SELECT ON cat_tools.pg_all_foreign_keys TO cat_tools__usage; @generated@ diff --git a/sql/cat_tools.sql.in b/sql/cat_tools.sql.in index f9ff3c7..30b8020 100644 --- a/sql/cat_tools.sql.in +++ b/sql/cat_tools.sql.in @@ -775,10 +775,8 @@ CREATE TYPE cat_tools.object_type AS ENUM( , 'materialized view column' , 'composite type column' , 'foreign table column' - /* - * pg_constraint - * NOTE: a domain itself is considered to be a type - */ + -- pg_constraint + -- NOTE: a domain itself is considered to be a type , 'domain constraint', 'table constraint' -- pg_proc , 'aggregate', 'function' @@ -938,10 +936,7 @@ SELECT ( WHEN object_type::text LIKE '% column' THEN 'pg_attribute' ELSE CASE object_type - /* - * Unusual cases - * s/, \(.\{-}\) -- \(.*\)/ WHEN \1 THEN '\2'/ - */ + -- Unusual cases WHEN 'default value' THEN 'pg_attrdef' WHEN 'large object' THEN 'pg_largeobject' WHEN 'operator class' THEN 'pg_opclass' @@ -1287,40 +1282,41 @@ $$ @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ +-- sql-lint:disable-block all CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS - SELECT n1.nspname AS fk_schema_name - , c1.relname AS fk_table_name - , k1.conname AS fk_constraint_name - , c1.oid AS fk_table_oid - , _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns - , n2.nspname AS pk_schema_name - , c2.relname AS pk_table_name - , k2.conname AS pk_constraint_name - , c2.oid AS pk_table_oid - , ci.relname AS pk_index_name - , _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns - , CASE k1.confmatchtype WHEN 'f' THEN 'FULL' + SELECT n1.nspname AS fk_schema_name, + c1.relname AS fk_table_name, + k1.conname AS fk_constraint_name, + c1.oid AS fk_table_oid, + _cat_tools._pg_sv_column_array(k1.conrelid,k1.conkey) AS fk_columns, + n2.nspname AS pk_schema_name, + c2.relname AS pk_table_name, + k2.conname AS pk_constraint_name, + c2.oid AS pk_table_oid, + ci.relname AS pk_index_name, + _cat_tools._pg_sv_column_array(k1.confrelid,k1.confkey) AS pk_columns, + CASE k1.confmatchtype WHEN 'f' THEN 'FULL' WHEN 'p' THEN 'PARTIAL' WHEN 'u' THEN 'NONE' else null - END AS match_type - , CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ + END AS match_type, + CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' else null - END AS on_delete - , CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' + END AS on_delete, + CASE k1.confupdtype WHEN 'a' THEN 'NO ACTION' WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' WHEN 'r' THEN 'RESTRICT' ELSE NULL - END AS on_update - , k1.condeferrable AS is_deferrable -- @generated@ - , k1.condeferred AS is_deferred + END AS on_update, + k1.condeferrable AS is_deferrable, -- @generated@ + k1.condeferred AS is_deferred FROM pg_catalog.pg_constraint k1 JOIN pg_catalog.pg_namespace n1 ON (n1.oid = k1.connamespace) JOIN pg_catalog.pg_class c1 ON (c1.oid = k1.conrelid) @@ -1352,6 +1348,7 @@ AS AND k1.contype = 'f' AND _cat_tools._pg_sv_table_accessible(n1.oid, c1.oid) ; +-- sql-lint:enable-block GRANT SELECT ON cat_tools.pg_all_foreign_keys TO cat_tools__usage; @generated@ diff --git a/test/sql/pg_depends.sql b/test/sql/pg_depends.sql index e1c411f..d279917 100644 --- a/test/sql/pg_depends.sql +++ b/test/sql/pg_depends.sql @@ -11,15 +11,6 @@ CREATE TEMP VIEW views AS ; GRANT SELECT ON views TO public; -/* EXCLUDED CODE -CREATE TEMP VIEW func_calls AS - SELECT * FROM (VALUES - ('pg_attribute__get'::name, $$'pg_class', 'relname'$$::text) - ) v(fname, args) -; -GRANT SELECT ON func_calls TO public; -*/ - /* * The pg_depends tests are problematic, because bag_eq creates temp objects, * which then do not show up in pg_dep or the view. So we need to capture what From 9337febdd972144f27beeadd070f71e0a61a21e0 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 16:59:12 -0500 Subject: [PATCH 03/11] Vendor the linter as a submodule instead of copying it in-tree Replace the fully-copied lint/ directory with a git submodule at .vendor/linter, pointing at the new https://github.com/Postgres-Extensions/ linter -- the actual linter code, its tests, and its docs now live in exactly one place, shared across every Postgres-Extensions project, instead of a separate copy per consuming repo. The entire local footprint for consuming it is one small lint.mk: it self-initializes the submodule (git submodule update --init) if needed, then hands off to the vendored repo's own lint.mk. This means `make lint` works right after a plain `git clone` with no --recurse-submodules step, and the Makefile/CI don't need to know anything about the linter beyond that one hand-off -- everything else (rules, tests, README, LICENSE) lives in the vendored repo. CI's lint job now checks out the repo WITHOUT submodules and just runs `make lint` -- the same command a developer would run locally -- which is what actually proves the self-init in lint.mk works, rather than papering over it with a submodules: true checkout. The linter's own test suite (fixtures + scanner edge cases) moved to being that repo's own CI's responsibility, so the lint job here dropped its separate `make -C lint test` step. Verified make lint (after deinit-ing the submodule to simulate a fresh clone), make install, and make verify-results all green. --- .github/workflows/ci.yml | 16 +- .gitmodules | 3 + .vendor/linter | 1 + Makefile | 20 +- lint.mk | 11 + lint/Makefile | 8 - lint/lint.mk | 29 - lint/sql/DESIGN.md | 91 --- lint/sql/Makefile | 4 - lint/sql/README.md | 278 --------- lint/sql/bin/sql-lint | 558 ------------------ lint/sql/test/01-fixtures.t | 72 --- lint/sql/test/02-scanner.t | 307 ---------- .../sql/test/fixtures/comment-closing.bad.sql | 7 - .../test/fixtures/comment-closing.good.sql | 7 - .../test/fixtures/comment-line-prefix.bad.sql | 6 - .../fixtures/comment-line-prefix.good.sql | 20 - .../sql/test/fixtures/comment-opening.bad.sql | 7 - .../test/fixtures/comment-opening.good.sql | 9 - .../sql/test/fixtures/comment-padding.bad.sql | 10 - .../test/fixtures/comment-padding.good.sql | 9 - .../test/fixtures/comment-single-line.bad.sql | 4 - .../fixtures/comment-single-line.good.sql | 7 - .../fixtures/comment-stacked-dashes.bad.sql | 8 - .../fixtures/comment-stacked-dashes.good.sql | 21 - .../fixtures/generated-file-skipped.good.sql | 5 - .../test/fixtures/prefer-short-type.bad.sql | 12 - .../test/fixtures/prefer-short-type.good.sql | 19 - lint/sql/test/fixtures/trailing-comma.bad.sql | 30 - .../sql/test/fixtures/trailing-comma.good.sql | 46 -- 30 files changed, 35 insertions(+), 1590 deletions(-) create mode 100644 .gitmodules create mode 160000 .vendor/linter create mode 100644 lint.mk delete mode 100644 lint/Makefile delete mode 100644 lint/lint.mk delete mode 100644 lint/sql/DESIGN.md delete mode 100644 lint/sql/Makefile delete mode 100644 lint/sql/README.md delete mode 100755 lint/sql/bin/sql-lint delete mode 100644 lint/sql/test/01-fixtures.t delete mode 100644 lint/sql/test/02-scanner.t delete mode 100644 lint/sql/test/fixtures/comment-closing.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-closing.good.sql delete mode 100644 lint/sql/test/fixtures/comment-line-prefix.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-line-prefix.good.sql delete mode 100644 lint/sql/test/fixtures/comment-opening.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-opening.good.sql delete mode 100644 lint/sql/test/fixtures/comment-padding.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-padding.good.sql delete mode 100644 lint/sql/test/fixtures/comment-single-line.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-single-line.good.sql delete mode 100644 lint/sql/test/fixtures/comment-stacked-dashes.bad.sql delete mode 100644 lint/sql/test/fixtures/comment-stacked-dashes.good.sql delete mode 100644 lint/sql/test/fixtures/generated-file-skipped.good.sql delete mode 100644 lint/sql/test/fixtures/prefer-short-type.bad.sql delete mode 100644 lint/sql/test/fixtures/prefer-short-type.good.sql delete mode 100644 lint/sql/test/fixtures/trailing-comma.bad.sql delete mode 100644 lint/sql/test/fixtures/trailing-comma.good.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c03606d..2185e7c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,11 +211,15 @@ jobs: # (pgxntool marks installcheck `.IGNORE`), so failures would pass silently. make verify-results - # Style linter (see lint/sql/README.md). Runs the linter's own test suite - # first (fixtures + scanner edge cases), then lints the actively-maintained - # SQL (LINT_TARGETS in the root Makefile). No PostgreSQL needed -- sql-lint - # is a standalone Perl script -- so this doesn't use the pgxn-tools - # container. + # Style linter (https://github.com/Postgres-Extensions/linter, vendored at + # .vendor/linter). 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 (see its comment). 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. The + # linter's own test suite (fixtures + scanner edge cases) is that repo's + # own CI's job, not this one's. No PostgreSQL needed -- sql-lint is a + # standalone Perl script -- so this doesn't use the pgxn-tools container. lint: needs: [changes] if: needs.changes.outputs.docs_only != 'true' @@ -224,8 +228,6 @@ jobs: steps: - name: Check out the repo uses: actions/checkout@v6 - - name: Run the linter's own test suite - run: make -C lint test - 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..e8575e5 --- /dev/null +++ b/.vendor/linter @@ -0,0 +1 @@ +Subproject commit e8575e5c04b4ca28304d36d661f73650527876d5 diff --git a/Makefile b/Makefile index a6de719..5d894e6 100644 --- a/Makefile +++ b/Makefile @@ -93,13 +93,15 @@ $(DESTDIR)$(datadir)/extension/cat_tools--0.2.0.sql: clean_old_version: pgxn uninstall --unstable 'cat_tools=0.2.0' -# Style linter (see lint/sql/README.md). Scoped to the actively-maintained -# source rather than the default `sql/ test/` (lint/lint.mk): version-specific -# install/update scripts under sql/ are frozen once released (SQL file -# conventions rule 5 in CLAUDE.md โ€” never hand-edited again), so linting them -# would produce permanent, unfixable findings and make `make lint` unusable -# as a CI gate. Lint the current source instead; a version file still under -# active development can be linted directly, e.g. -# `lint/sql/bin/sql-lint sql/cat_tools--0.3.0.sql.in`. +# 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 the actively-maintained source rather than the default +# `sql/ test/`: version-specific install/update scripts under sql/ are +# frozen once released (SQL file conventions rule 5 in CLAUDE.md โ€” never +# hand-edited again), so linting them would produce permanent, unfixable +# findings and make `make lint` unusable as a CI gate. Lint the current +# source instead; a version file still under active development can be +# linted directly, e.g. +# `.vendor/linter/sql/bin/sql-lint sql/cat_tools--0.3.0.sql.in`. LINT_TARGETS = sql/cat_tools.sql.in sql/omit_column.sql test/ -include lint/lint.mk +include lint.mk 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/lint/Makefile b/lint/Makefile deleted file mode 100644 index 5c19f0a..0000000 --- a/lint/Makefile +++ /dev/null @@ -1,8 +0,0 @@ -# Parent Makefile for all linters. -# Add new linter subdirectories to LINTERS as they are created. - -LINTERS = sql - -.PHONY: test -test: - for linter in $(LINTERS); do $(MAKE) -C $$linter test || exit 1; done diff --git a/lint/lint.mk b/lint/lint.mk deleted file mode 100644 index ae90c99..0000000 --- a/lint/lint.mk +++ /dev/null @@ -1,29 +0,0 @@ -# lint.mk โ€” shared Make fragment that adds a `lint` target to any extension. -# -# Include from an extension Makefile (one level below the repo root): -# -# include ../lint/lint.mk -# -# Provides: -# make lint โ€” run sql-lint on all SQL we own under the extension dir (sql/ and -# test/, or LINT_TARGETS if set). sql-lint skips vendored trees -# (deps/, pgxntool/) and auto-generated files (e.g. -# sql/--.sql). -# -# Set LINT_TARGETS before this include to lint a different set of paths, e.g. -# to exclude frozen version-snapshot files that are never hand-edited: -# -# LINT_TARGETS = sql/cat_tools.sql.in sql/omit_column.sql test/ -# include lint/lint.mk - -# Resolve the repo root relative to this file's location (lint/ is one level -# below root, so go up one more). -_LINT_MK_DIR := $(dir $(lastword $(MAKEFILE_LIST))) -_REPO_ROOT := $(abspath $(_LINT_MK_DIR)/..) -SQL_LINT := $(_REPO_ROOT)/lint/sql/bin/sql-lint - -LINT_TARGETS ?= sql/ test/ - -.PHONY: lint -lint: - $(SQL_LINT) $(LINT_TARGETS) diff --git a/lint/sql/DESIGN.md b/lint/sql/DESIGN.md deleted file mode 100644 index 320ccc8..0000000 --- a/lint/sql/DESIGN.md +++ /dev/null @@ -1,91 +0,0 @@ -# sql-lint Design - -## Type Name Preferences - -Preferred names were determined by querying `pg_type` on PG17. All pairs are -true aliases (same OID). Preferences: - -- SQL standard names where shorter: `int` (not `integer`), `varchar` (not - `character varying`) -- SQL standard names where clearer: `smallint`/`bigint` (not `int2`/`int8`), - `boolean` (not `bool`), `real` (not `float4`), `double precision` (not - `float8`) -- Common short form: `numeric` (not `decimal`), `timestamp`/`timestamptz` - (not the verbose `without/with time zone` forms) - -NOT flagged: `text` vs `varchar` (semantically different โ€” `varchar` has an -optional length constraint). - -## Portability - -Two spots are kept generic rather than hardcoded to this project, since -build conventions for what counts as "generated" vary: - -- The directory walk in `bin/sql-lint` matches both `*.sql` and `*.sql.in` โ€” - cat_tools' hand-maintained SQL source is a `.sql.in` template expanded to - `.sql` at build time, not a plain `.sql` file. -- `file_is_generated()` matches any first line containing both "GENERATED" - and "DO NOT EDIT" (in either order), rather than one fixed marker string. - -`lint.mk` also exposes `LINT_TARGETS` (default `sql/ test/`) so `make lint` -can be scoped to specific paths โ€” e.g. to skip frozen version-snapshot files -that are tracked for update-path testing but, per project convention, are -never hand-edited again and so have no fixable findings. - -## Future Rules - -### Ready Now (line-level, no new infrastructure) - -| Rule | Complexity | Description | -|---|---|---| -| **coalesce-boolean** | ~15 lines | Flag `COALESCE(expr, true/false)` โ€” use `IS [NOT] TRUE` instead | -| **dollar-quote-naming** | ~15 lines | Flag bare `$$` and single-char `$x$` tags on multi-line blocks | - -### Needs Small Helper (~10 lines of infra) - -| Rule | Complexity | Description | -|---|---|---| -| **when-others-comment** | ~25 lines | `WHEN OTHERS` without explanatory comment on same/next line | -| **security-definer** | ~30 lines | `SECURITY DEFINER` without `SET search_path` nearby | - -### Needs Statement Tracking Infrastructure - -| Rule | Complexity | Description | -|---|---|---| -| **semicolon-placement** | ~50 lines + tracker | `;` on own line for multi-line statements | -| **comment-ordering** | ~80 lines + tracker + DDL extractor | `COMMENT ON` must follow its object | -| **variable-naming** | ~50 lines + dollar-quote awareness | PL/pgSQL `c_`/`v_`/`a_`/`r_` prefixes | - -The statement boundary tracker is the key infrastructure piece โ€” building it -unlocks semicolon-placement and comment-ordering. Variable-naming needs -separate dollar-quote block awareness. - -The `lint/` directory structure supports adding linters for other languages -in the future (e.g. `lint/plpgsql/`, `lint/bash/`). Each linter is a -subdirectory with its own `Makefile`; `lint/Makefile` aggregates them. - -## History - -### Language Choice - -The linter went through two iterations: - -1. **Initial: Bash + AWK** (7 files, ~530 lines, 3 languages). Shipped fast - but hit pain points: subshell variable loss, temp-file workarounds, awk - embedded inside bash single-quotes (`\047` for single-quote), and no path - to statement-level rules. - -2. **Current: single Perl script**. Same rules, same output โ€” but the scanner, - suppression, and all rules live in one file with real data structures. - -Why Perl over Go/Rust/Python: - -| | Perl | Go/Rust | Python | -|---|---|---|---| -| Text processing | Native strength | Adequate | Adequate | -| Available everywhere | Yes (ships with every Unix) | Needs build pipeline | Yes (3.6+) | -| Statement-level rules | Natural (slurp + split) | Natural | Natural | -| Docker footprint | 15 MB | N/A (static binary) | 50 MB | - -Go and Rust require a build/release pipeline that doesn't exist for this repo. -Python was considered but Perl is more concise for regex-heavy text processing. diff --git a/lint/sql/Makefile b/lint/sql/Makefile deleted file mode 100644 index 80404d9..0000000 --- a/lint/sql/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -# Run lint/sql tests locally (requires perl on $PATH). -.PHONY: test -test: - prove -v test/ diff --git a/lint/sql/README.md b/lint/sql/README.md deleted file mode 100644 index 909d436..0000000 --- a/lint/sql/README.md +++ /dev/null @@ -1,278 +0,0 @@ -# sql-lint - -Style linter for PostgreSQL SQL files. Enforces this project's SQL coding -conventions (block comments, leading commas, short type names โ€” see the -"SQL file conventions" and "Code Style" sections of `CLAUDE.md`). - -## Quick Start - -```bash -# Lint the project's SQL from the repo root (paths are set in Makefile's -# LINT_TARGETS; see lint/lint.mk) -make lint - -# Run the linter directly on specific files or directories -lint/sql/bin/sql-lint sql/cat_tools.sql.in test/ -``` - -## Usage - -``` -sql-lint [OPTIONS] FILE... -sql-lint [OPTIONS] DIRECTORY... -``` - -Files are checked against all enabled rules. Directories are searched -recursively for `*.sql` and `*.sql.in` files (excluding `deps/`, -`docker/deps/`, `pgxntool/`, and `.claude/worktrees/`). - -### Options - -| Flag | Description | -|------|-------------| -| `-h`, `--help` | Show help message | -| `-q`, `--quiet` | Suppress the summary line | - -### Exit Codes - -| Code | Meaning | -|------|---------| -| 0 | No findings | -| 1-10 | Reserved for generic errors (2 = usage error) | -| 11+ | Lint findings: `exit_code - 10` = number of findings | - -Note: exit codes are capped at 255 by POSIX, so 245+ findings all exit 255. - -## Rules - -All comment formatting rules share the `comment-` prefix so they sort -together in fixture listings. - -### comment-closing - -Block comments must end with `*/` alone on its own line. - -```sql -/* - * Bad: text before closing */ - -/* - * Good: closing on its own line. - */ -``` - -### comment-line-prefix - -Lines inside a block comment must start with ` * `. - -```sql -/* - Bad: missing the star prefix. - * Good: has the star prefix. - */ -``` - -### comment-opening - -Block comments must start with `/*` alone on the opening line. - -```sql -/* Bad: text on the opening line - * of a multi-line comment - */ - -/* - * Good: opening line has only /* - */ -``` - -### comment-padding - -Block comments must not have blank `*` lines immediately after `/*` -or immediately before `*/`. - -```sql -/* - * - * Bad: leading blank line above. - */ - -/* - * Good: content starts immediately. - */ -``` - -### comment-single-line - -Single-line `/* text */` comments are not allowed. Use `--` for -single-line comments. - -```sql -/* Bad: single-line block comment */ - --- Good: use a line comment instead -``` - -### comment-stacked-dashes - -A run of 3 or more consecutive `--` lines must use `/* */` syntax instead. -Up to 2 consecutive `--` lines is fine โ€” a short remark reads fine as `--`. - -```sql --- Bad: stacked dashes --- spanning three --- or more lines - --- Good: 2 lines is fine --- as a short remark - -/* - * Good: block comment - * for anything longer - */ -``` - -### trailing-comma - -Multi-line constructs must use leading commas (`, item`), not trailing -commas (`item,`). - -```sql --- Bad -SELECT - column1, - column2, - column3 - --- Good -SELECT - column1 - , column2 - , column3 -``` - -### prefer-short-type - -Use short-form type names where PostgreSQL provides them. `text` and -`varchar` are not interchangeable and are not flagged against each other. - -| Don't use | Use instead | -|-------------|-----------| -| `integer`, `int4` | `int` | -| `bool` | `boolean` | -| `character varying` | `varchar` | -| `float8` | `double precision` | -| `float4` | `real` | -| `int2` | `smallint` | -| `int8` | `bigint` | -| `decimal` | `numeric` | -| `timestamp without time zone` | `timestamp` | -| `timestamp with time zone` | `timestamptz` | -| `time without time zone` | `time` | -| `time with time zone` | `timetz` | - -## Inline Suppression - -Suppress a finding on a specific line: - -```sql -SELECT something, -- sql-lint:disable trailing-comma -``` - -`disable` and `disable-line` both apply to the line they appear on; -`disable-next-line` applies to the following line. Use `all` in place of a -rule id to suppress every rule. - -```sql --- sql-lint:disable-next-line trailing-comma -SELECT something, -``` - -### Block suppression - -A `disable-block ` directive on the line that opens a `/* */` block -comment suppresses the rule for every line in that block. - -For the common case โ€” commenting out a chunk of code without the linter -demanding ` * ` prefixes (or other comment styling) on each line โ€” use the -shorthand `EXCLUDED CODE` on the opening line. It is an alias for -`sql-lint:disable-block all`: - -```sql -/* EXCLUDED CODE -SELECT not_ready_yet( - foo - , bar -); -*/ -``` - -Text may follow it (e.g. `/* EXCLUDED CODE โ€” disabled until issue #123`). - -### Region suppression (real code, not a comment) - -To suppress a chunk of actual code โ€” e.g. borrowed code deliberately left -unreformatted โ€” wrap it in `disable-block ` / `enable-block` -directives on their own `--` lines. Unlike the `/* */`-anchored form above, -this pair works outside a comment and is closed explicitly rather than by a -comment's own `*/` (if never closed, it runs to end of file): - -```sql --- sql-lint:disable-block all -SELECT n1.nspname AS fk_schema_name, - c1.relname AS fk_table_name - FROM pg_catalog.pg_constraint k1 --- sql-lint:enable-block -``` - -## Adding New Rules - -Add a subroutine to `bin/sql-lint` and register it in the `@rules` array: - -```perl -sub check_my_rule { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'CODE'; - my $cc = code_content($l->{text}); - next unless $cc =~ /pattern/; - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'my-rule', - 'description of the violation'); - } - return @out; -} -``` - -Each line in `$lines` is a hashref with `lineno`, `state` (`CODE` or -`COMMENT`), and `text`. The `code_content()` helper strips strings, -comments, and `/*` from a line. `maybe_finding()` handles suppression -automatically. - -Then add test fixtures and/or inline tests: - -- **Fixtures** (`test/fixtures/`): use `.good.sql` for clean files, - `.bad.sql` for files with violations. Bad fixtures need a - `-- expect-findings: N` header. No test code changes required. -- **Inline tests** (`test/02-scanner.t`): for specific edge cases, - regression tests, and assertions about finding content. - -## Testing - -Tests use Perl's `Test::More` + `prove` to test the linter itself (not -the extension code โ€” that's what `make lint` is for): - -```bash -make -C lint test # all linters (currently just sql) -make -C lint/sql test # sql linter only -``` - -## Design - -See [DESIGN.md](DESIGN.md) for architecture decisions and rationale. - -## Future - -The `lint/` directory is designed to host linters for other languages -(e.g. PL/pgSQL, bash). Each would be a sibling to `sql/` with its own -`Makefile` and test suite. diff --git a/lint/sql/bin/sql-lint b/lint/sql/bin/sql-lint deleted file mode 100755 index e9e926a..0000000 --- a/lint/sql/bin/sql-lint +++ /dev/null @@ -1,558 +0,0 @@ -#!/usr/bin/env perl -# -# sql-lint: style linter for PostgreSQL SQL files -# -# Usage: sql-lint [OPTIONS] FILE... -# sql-lint [OPTIONS] DIRECTORY... -# -# Checks SQL files against the project's coding style rules and reports -# violations. Directories are searched recursively for *.sql and *.sql.in -# files. -# -# Options: -# -h, --help Show this help message -# -q, --quiet Suppress summary line -# --scan Dump scanner output and exit (for debugging/testing) -# -# Exit codes: -# 0 No findings -# 1-10 Reserved for generic errors (2 = usage error) -# 11+ Lint findings: exit_code - 10 = number of findings -# (exit codes are capped at 255 by POSIX) - -use strict; -use warnings; -use File::Find (); -use Getopt::Long qw(:config no_ignore_case bundling); - -# ============================================================ -# Scanner โ€” labels each line CODE or COMMENT -# -# Single character-by-character pass over each line, maintaining state -# across lines. Tracks block comment depth (/* ... */, supports -# PostgreSQL's nested comments), single-quote strings (to prevent /* -# inside '...' from opening a comment), and line comments (-- to EOL). -# -# Each line is labeled CODE or COMMENT based on comment depth at the -# *start* of that line, then depth is updated for the next line. -# -# Nested /* is real nesting: PostgreSQL treats /* ... /* ... */ ... */ -# as nested block comments; a /* inside an existing block comment -# increments depth and requires a matching */. -# -# Dollar-quoted blocks ($$...$$) are intentionally not tracked โ€” their -# content is usually PL/pgSQL that should be linted. -# ============================================================ - -sub scan_file { - my ($filename) = @_; - open my $fh, '<', $filename or die "ERROR: cannot open $filename: $!\n"; - my @lines; - my $comment_depth = 0; - # Must persist across lines (like comment_depth): a single-quoted literal - # may span multiple lines, and -- or /* inside it must not start a comment. - my $in_string = 0; - # Line number of the /* that opened the block comment we are currently - # inside (the outermost one). Lets rules honor a block-level suppression - # directive placed on the opening line. Undef when not in a comment. - my $open_lineno; - - while (my $raw = <$fh>) { - chomp $raw; - # State is determined at line entry โ€” before scanning this line's content. - push @lines, { - lineno => $., - state => $comment_depth > 0 ? 'COMMENT' : 'CODE', - in_string => $in_string, - text => $raw, - ($comment_depth > 0 ? (block_open => $open_lineno) : ()), - }; - - # Character-by-character scan to update comment_depth for the NEXT line. - # Tracks nested /* */ and skips /* inside strings and -- comments. - my $len = length $raw; - my $i = 0; - - while ($i < $len) { - my $c = substr $raw, $i, 1; - my $c2 = $i + 1 < $len ? substr($raw, $i, 2) : ''; - - if ($comment_depth > 0) { - if ($c2 eq '*/') { $comment_depth--; $i += 2; next } # close block - if ($c2 eq '/*') { $comment_depth++; $i += 2; next } # nested open - $i++; next; - } - if ($in_string) { - if ($c2 eq "''") { $i += 2; next } # escaped single-quote - $in_string = 0 if $c eq "'"; # end of string - $i++; next; - } - # CODE context โ€” not inside comment or string - last if $c2 eq '--'; # line comment; rest is ignored - if ($c2 eq '/*') { $comment_depth++; $open_lineno = $.; $i += 2; next } # open block comment - if ($c eq "'") { $in_string = 1; $i++; next } # open string literal - # Dollar-quoted blocks ($$...$$) are intentionally not tracked. - # Their content is usually PL/pgSQL that should be linted. - $i++; - } - } - close $fh; - return \@lines; -} - -# ============================================================ -# Helpers -# ============================================================ - -# Strip single-quoted string literals (handles '' escapes). -# When $in_string is true, the line starts inside a multi-line string โ€” -# strip from the beginning through the closing quote. -sub strip_strings { - my ($text, $in_string) = @_; - my $t = $text; - if ($in_string) { - # Strip leading string continuation through closing quote - $t =~ s/^[^']*'// or return ''; # no close on this line โ€” all string - } - $t =~ s/'(?:''|[^'])*'//g; # complete single-line strings - $t =~ s/'.*$//; # unclosed trailing string (multi-line open) - return $t; -} - -# Like strip_strings, but replaces each string literal with a single sentinel -# character ('x') instead of removing it. Used where the *position* of code -# relative to a string matters (e.g. trailing-comma): removing the string would -# turn a correct leading-comma item like `, 'desc'` into a bare `,`, and turn a -# genuine trailing comma after a string like `'desc',` into the same bare `,` โ€” -# the two become indistinguishable. The sentinel preserves which side the comma -# is on. -sub blank_strings { - my ($text, $in_string) = @_; - my $t = $text; - if ($in_string) { - $t =~ s/^[^']*'/x/ or return 'x'; # continuation: close -> sentinel; else all string - } - $t =~ s/'(?:''|[^'])*'/x/g; # complete single-line strings - $t =~ s/'.*$/x/; # unclosed trailing string (multi-line open) - return $t; -} - -# Return the "code" portion of a line โ€” strings, trailing -- comment, -# and inline /* ... */ comments removed. -sub code_content { - my ($text, $in_string) = @_; - my $c = strip_strings($text, $in_string); - $c =~ s|/\*.*?\*/||g; # strip complete inline /* ... */ comments first - $c =~ s|/\*.*$||; # strip unclosed /* to end of line - $c =~ s/\s*--.*$//; # strip trailing line comment last (-- inside /* */ already gone) - return $c; -} - -# Like code_content but keeps a sentinel for string literals (see blank_strings). -sub code_skeleton { - my ($text, $in_string) = @_; - my $c = blank_strings($text, $in_string); - $c =~ s|/\*.*?\*/||g; - $c =~ s|/\*.*$||; - $c =~ s/\s*--.*$//; - return $c; -} - -# Mark each line as inside a `sql-lint:disable-block ... enable-block` CODE -# region โ€” as opposed to the /* */-anchored disable-block above (auto-scoped -# by the comment's own close, untouched by this). This form works on real -# code, explicitly closed by `sql-lint:enable-block` (or open to EOF if never -# closed), so a whole chunk โ€” e.g. borrowed code deliberately left -# unreformatted โ€” can be silenced without wrapping it in a comment. Nested -# regions are tracked as a stack, so an inner region closes before an outer -# one. Stores results directly on each line hashref (region_all / a -# region_rules set) for is_suppressed() to check. -sub annotate_region_suppressions { - my ($lines) = @_; - my @stack; - for my $l (@$lines) { - if ($l->{text} =~ /sql-lint:disable-block\s+(\S+)/) { - push @stack, $1; - } - for my $r (@stack) { - if ($r eq 'all') { $l->{region_all} = 1 } - else { $l->{region_rules}{$r} = 1 } - } - if ($l->{text} =~ /sql-lint:enable-block\b/ && @stack) { - pop @stack; - } - } -} - -# True if the finding is suppressed by an inline pragma. -sub is_suppressed { - my ($lines, $lineno, $rule_id) = @_; - my $idx = $lineno - 1; - return 0 if $idx < 0 || $idx > $#$lines; - - my $l = $lines->[$idx]; - return 1 if $l->{region_all}; - return 1 if $l->{region_rules} && $l->{region_rules}{$rule_id}; - - my $text = $l->{text}; - # Same-line directive. Matches `sql-lint:disable`, `disable-line`, or - # `disable-block` (the optional `-line`/`-block` suffix), followed by either - # this rule's id or the literal `all`. \Q...\E escapes any regex-special - # chars in $rule_id; the trailing \b stops `all` (or a rule id) from also - # matching when it is only a prefix of a longer following word. - return 1 if $text =~ /sql-lint:disable(?:-line|-block)?\s+(?:\Q$rule_id\E|all)\b/; - # `/* EXCLUDED CODE` marks the start of a commented-out code block โ€” a - # friendly alias for `sql-lint:disable-block all`. Matching here covers the - # opener line itself (e.g. its comment-opening finding). - return 1 if $text =~ m{/\*\s*EXCLUDED CODE\b}; - - if ($idx > 0) { - my $prev = $lines->[$idx - 1]{text}; - return 1 if $prev =~ /sql-lint:disable-next-line\s+(?:\Q$rule_id\E|all)\b/; - } - - # Block-level: a directive on the /* that opened the enclosing block comment - # suppresses the rule for every line in the block. This lets a /* */ block be - # used to comment out code without the linter demanding a " * " prefix (or - # other comment styling) on each line. `EXCLUDED CODE` is the short alias. - my $open = $lines->[$idx]{block_open}; - if (defined $open) { - my $open_text = $lines->[$open - 1]{text}; - # Same rule-id/`all` matching as the same-line directive above (see that - # comment for the regex breakdown), but only the `disable-block` form - # counts on the opening line. - return 1 if $open_text =~ /sql-lint:disable-block\s+(?:\Q$rule_id\E|all)\b/; - return 1 if $open_text =~ m{/\*\s*EXCLUDED CODE\b}; - } - return 0; -} - -# Convenience: push a finding unless suppressed. -sub maybe_finding { - my ($out, $lines, $file, $lineno, $rule, $msg) = @_; - return if is_suppressed($lines, $lineno, $rule); - push @$out, { file => $file, line => $lineno, rule => $rule, message => $msg }; -} - -# ============================================================ -# Rules -# -# Each rule is a subroutine receiving ($file, $lines) and returning a -# list of finding hashrefs. Use maybe_finding() for suppression checks. -# Rules examining comment formatting check COMMENT-state lines; all -# others check CODE-state lines only. -# ============================================================ - -sub check_stacked_dashes { - my ($file, $lines) = @_; - my @out; - my ($start, $end) = (0, 0); - - # Up to 2 consecutive -- lines is allowed (a short two-line remark reads - # fine as --); only 3+ must become a /* */ block. - my $flush = sub { - return unless $start && ($end - $start) >= 2; - my $n = $end - $start + 1; - maybe_finding(\@out, $lines, $file, $start, 'comment-stacked-dashes', - "$n consecutive -- comment lines ($start-$end); use /* */ for multi-line comments"); - }; - - for my $l (@$lines) { - if ($l->{state} eq 'CODE' && $l->{text} =~ /^\s*--/) { # line-comment line - $start ||= $l->{lineno}; - $end = $l->{lineno}; - } else { - $flush->(); - ($start, $end) = (0, 0); - } - } - $flush->(); - return @out; -} - -sub check_comment_opening { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'CODE'; - next if $l->{text} =~ /^\s*--/; # skip line-comment lines - my $cc = strip_strings($l->{text}, $l->{in_string}); - # Strip /* ... */ before -- (same reason as code_content: -- inside - # a block comment would eat the closing */ if stripped first). - $cc =~ s|/\*.*?\*/||g; - $cc =~ s/\s*--.*$//; - next unless $cc =~ m|/\*|; # must contain a block comment opener - (my $after = $cc) =~ s|.*?/\*||; # text after the /* - next unless $after =~ /\S/; # something non-space follows /* - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-opening', - 'text after opening /*; put /* on its own line'); - } - return @out; -} - -sub check_single_line_block_comment { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'CODE'; - next if $l->{text} =~ /^\s*--/; # skip line-comment lines - my $cc = strip_strings($l->{text}, $l->{in_string}); - # NB: stripping -- before /* */ can eat a closing */ when -- appears - # inside a block comment (e.g. /* hint -- note */). That causes a - # false negative, but the line is already a violation regardless. - $cc =~ s/\s*--.*$//; - next unless $cc =~ m|/\*| && $cc =~ m|\*/|; # has both /* and */ - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-single-line', - 'single-line /* */ comment; use -- for single-line comments'); - } - return @out; -} - -sub check_comment_line_prefix { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'COMMENT'; # only inside /* */ blocks - next if $l->{text} =~ m|\*/|; # closing line โ€” other rule handles - (my $s = $l->{text}) =~ s/^\s*//; # strip leading whitespace - next if $s =~ /^\* / || $s eq '*'; # " * text" or bare " *" - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-line-prefix', - 'comment line must start with " * " (space-star-space)'); - } - return @out; -} - -sub check_comment_padding { - my ($file, $lines) = @_; - my @out; - for my $i (0 .. $#$lines) { - my $l = $lines->[$i]; - next unless $l->{state} eq 'COMMENT'; - next unless $l->{text} =~ /^\s*\*\s*$/; # bare * line - - # Leading: previous line opened the comment (CODE state) - if ($i > 0 && $lines->[$i-1]{state} eq 'CODE') { - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-padding', - 'unnecessary blank line at start of block comment'); - next; - } - - # Trailing: next line closes the comment - if ($i < $#$lines && $lines->[$i+1]{text} =~ /\*\//) { - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-padding', - 'unnecessary blank line at end of block comment'); - } - } - return @out; -} - -sub check_comment_closing { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'COMMENT'; # only inside /* */ blocks - next unless $l->{text} =~ m|\*/|; # must contain closing */ - next if $l->{text} =~ m|^\s*\*/\s*$|; # */ alone on its line is fine - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'comment-closing', - 'closing */ must be alone on its own line'); - } - return @out; -} - -sub check_trailing_comma { - my ($file, $lines) = @_; - my @out; - for my $l (@$lines) { - next unless $l->{state} eq 'CODE'; - next if $l->{text} =~ /^\s*--/; # skip line-comment lines - # Keep a sentinel for strings: a leading-comma item whose payload is a - # string (e.g. `, 'desc'`) must not be mistaken for a trailing comma, - # while a genuine trailing comma after a string (e.g. `'desc',`) must - # still be caught. code_content (which deletes strings) collapses both - # to a bare `,`; code_skeleton keeps the comma on the correct side. - my $cc = code_skeleton($l->{text}, $l->{in_string}); - $cc =~ s/\s+$//; # trim trailing whitespace - next unless $cc =~ /,$/; # ends with comma after stripping - # The comma must follow an item to be "trailing". A line that reduces to - # a bare leading comma is not a violation โ€” e.g. a leading comma alone on - # its line introducing a multi-line item (the comma, then the item's - # first line indented below it). Pure string items like `, 'x'` keep - # their item via the sentinel above, so they never reach this point. - (my $before = $cc) =~ s/,\s*$//; - next unless $before =~ /\S/; - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'trailing-comma', - 'trailing comma; use leading comma style'); - } - return @out; -} - -sub check_prefer_short_type { - my ($file, $lines) = @_; - my @out; - - # Multi-word type aliases โ€” checked via regex, before single-word - my @multi = ( - [ qr/character\s+varying/i, 'varchar' ], - [ qr/timestamp\s+without\s+time\s+zone/i, 'timestamp' ], - [ qr/timestamp\s+with\s+time\s+zone/i, 'timestamptz' ], - [ qr/time\s+without\s+time\s+zone/i, 'time' ], - [ qr/time\s+with\s+time\s+zone/i, 'timetz' ], - ); - - # Single-word type aliases โ€” checked via \bword\b match - my @single = ( - [ 'integer', 'int' ], - [ 'int4', 'int' ], - [ 'int2', 'smallint' ], - [ 'int8', 'bigint' ], - [ 'float4', 'real' ], - [ 'float8', 'double precision' ], - [ 'bool', 'boolean' ], - [ 'decimal', 'numeric' ], - ); - - for my $l (@$lines) { - next unless $l->{state} eq 'CODE'; - next if $l->{text} =~ /^\s*--/; # skip line-comment lines - my $cc = lc code_content($l->{text}, $l->{in_string}); - - for my $m (@multi) { - if ($cc =~ $m->[0]) { - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'prefer-short-type', - "use \"$m->[1]\" instead of \"$&\""); - } - } - for my $s (@single) { - my ($word, $sug) = @$s; - if ($cc =~ /\b\Q$word\E\b/) { - maybe_finding(\@out, $lines, $file, $l->{lineno}, 'prefer-short-type', - "use \"$sug\" instead of \"$word\""); - } - } - } - return @out; -} - -# ============================================================ -# Rule registry -# ============================================================ - -my @rules = ( - { id => 'comment-closing', check => \&check_comment_closing }, - { id => 'comment-line-prefix', check => \&check_comment_line_prefix }, - { id => 'comment-opening', check => \&check_comment_opening }, - { id => 'comment-padding', check => \&check_comment_padding }, - { id => 'comment-single-line', check => \&check_single_line_block_comment }, - { id => 'comment-stacked-dashes', check => \&check_stacked_dashes }, - { id => 'prefer-short-type', check => \&check_prefer_short_type }, - { id => 'trailing-comma', check => \&check_trailing_comma }, -); - -# A file whose first line marks it as generated (both "GENERATED" and "DO NOT -# EDIT", matched independently so either word order works, e.g. "GENERATED -# FILE! DO NOT EDIT!") is a build artifact (e.g. a version-specific install -# script concatenated from a hand-edited .sql/.sql.in source that is linted -# on its own), so skip linting it. -sub file_is_generated { - my ($file) = @_; - open my $fh, '<', $file or return 0; - my $first = <$fh>; - close $fh; - return defined($first) && $first =~ /DO NOT EDIT/i && $first =~ /GENERATED/i; -} - -# ============================================================ -# Main -# ============================================================ - -my ($help, $quiet, $scan_mode); -GetOptions( - 'h|help' => \$help, - 'q|quiet' => \$quiet, - 'scan' => \$scan_mode, -) or exit 2; - -if ($help) { - # Extract usage from this script's header comment (lines 3 through - # the first non-comment line, with the leading "# " prefix stripped). - open my $fh, '<', $0 or die; - while (<$fh>) { - last unless /^#/; - next if $. < 3; - s/^# ?//; - print; - } - exit 0; -} - -unless (@ARGV) { - warn "ERROR: no files specified\n"; - exit 2; -} - -# Expand directories to *.sql and *.sql.in files (cat_tools authors the -# hand-maintained SQL source as a .sql.in template expanded at build time), -# excluding dependency dirs. -my @files; -for my $arg (@ARGV) { - if (-d $arg) { - File::Find::find({ - wanted => sub { - return unless -f && /\.sql(?:\.in)?$/; - my $p = $File::Find::name; - return if $p =~ m{/(?:deps|docker/deps|pgxntool|\.claude/worktrees)/}; - push @files, $p; - }, - no_chdir => 1, - }, $arg); - } elsif (-f $arg) { - push @files, $arg; - } else { - warn "WARNING: $arg: no such file or directory\n"; - } -} -@files = sort @files; - -unless (@files) { - warn "ERROR: no SQL files found\n"; - exit 2; -} - -# --scan: dump annotated lines and exit (for debugging and scanner tests). -if ($scan_mode) { - for my $file (@files) { - my $lines = scan_file($file); - printf "%d|%s|%s|%s\n", $_->{lineno}, $_->{state}, - ($_->{in_string} ? 'S' : '.'), $_->{text} for @$lines; - } - exit 0; -} - -# Run all rules on all files. -my @findings; -for my $file (@files) { - next if file_is_generated($file); - my $lines = scan_file($file); - annotate_region_suppressions($lines); - for my $rule (@rules) { - push @findings, $rule->{check}->($file, $lines); - } -} - -# Sort by file, then line number. -@findings = sort { $a->{file} cmp $b->{file} || $a->{line} <=> $b->{line} } @findings; - -# Display. -printf "%s:%d: [%s] %s\n", $_->{file}, $_->{line}, $_->{rule}, $_->{message} - for @findings; - -# Summary and exit code. -my $count = scalar @findings; -if (!$quiet && $count > 0) { - printf STDERR "\n%d finding(s).\n", $count; -} -if ($count > 0) { - my $code = $count + 10; - $code = 255 if $code > 255; - exit $code; -} diff --git a/lint/sql/test/01-fixtures.t b/lint/sql/test/01-fixtures.t deleted file mode 100644 index 42218d8..0000000 --- a/lint/sql/test/01-fixtures.t +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env perl -# -# Data-driven fixture tests for sql-lint. -# -# Good fixtures (*.good.sql): must produce zero findings. -# Bad fixtures (*.bad.sql): must have a "-- expect-findings: N" header -# and produce exactly N findings. -# -# Adding a new fixture requires NO changes to this file. - -use strict; -use warnings; -use Test::More; -use File::Basename; -use File::Spec; - -my $test_dir = dirname(__FILE__); -my $lint = File::Spec->catfile($test_dir, '..', 'bin', 'sql-lint'); - -# -- Good fixtures: zero findings each ---------------------------------------- - -my @good = sort glob("$test_dir/fixtures/*.good.sql"); -ok(@good > 0, 'found good fixtures') or BAIL_OUT('no good fixtures'); - -subtest 'good fixtures produce zero findings' => sub { - plan tests => scalar @good; - for my $file (@good) { - my $name = basename($file); - my $output = `$lint -q "$file" 2>&1`; - my $rc = $? >> 8; - is($rc, 0, "$name: clean") or diag($output); - } -}; - -# -- Bad fixtures: expected finding counts from header ------------------------- - -my @bad = sort glob("$test_dir/fixtures/*.bad.sql"); -ok(@bad > 0, 'found bad fixtures') or BAIL_OUT('no bad fixtures'); - -subtest 'bad fixtures produce expected findings' => sub { - plan tests => scalar @bad * 2; - for my $file (@bad) { - my $name = basename($file); - my $expected = parse_expect($file); - - if (!defined $expected) { - fail("$name: missing '-- expect-findings: N' header"); - fail("$name: (skipped count check)"); - next; - } - - my $output = `$lint -q "$file" 2>&1`; - my $rc = $? >> 8; - isnt($rc, 0, "$name: non-zero exit"); - - my @lines = grep { /\S/ } split /\n/, $output; - is(scalar @lines, $expected, "$name: $expected finding(s)") - or diag($output); - } -}; - -done_testing(); - -sub parse_expect { - my ($file) = @_; - open my $fh, '<', $file or return undef; - while (<$fh>) { - return $1 if /^--\s*expect-findings:\s*(\d+)/; - last unless /^--/; - } - return undef; -} diff --git a/lint/sql/test/02-scanner.t b/lint/sql/test/02-scanner.t deleted file mode 100644 index fbd8a8d..0000000 --- a/lint/sql/test/02-scanner.t +++ /dev/null @@ -1,307 +0,0 @@ -#!/usr/bin/env perl -# -# Scanner behavior and regression tests for sql-lint. -# -# Use this file for: -# - Scanner-specific edge cases (state transitions, nesting) -# - Regression tests for specific bugs (1-5 line SQL snippets) -# - Behavior assertions on finding content (not just count) -# -# Use fixtures (01-fixtures.t) for whole-file patterns instead. - -use strict; -use warnings; -use Test::More; -use File::Basename; -use File::Spec; -use File::Temp qw(tempfile tempdir); - -my $test_dir = dirname(__FILE__); -my $lint = File::Spec->catfile($test_dir, '..', 'bin', 'sql-lint'); - -# -- Helpers ------------------------------------------------------------------- - -sub lint_string { - my ($sql) = @_; - my ($fh, $tmp) = tempfile(SUFFIX => '.sql', UNLINK => 1); - print $fh $sql; - close $fh; - my $output = `$lint -q "$tmp" 2>&1`; - my $rc = $? >> 8; - return ($rc, $output); -} - -# Write $sql to $filename inside a fresh temp directory, then lint the -# directory itself (not the file directly) โ€” for asserting which extensions -# the directory walk picks up. -sub lint_dir_with_file { - my ($filename, $sql) = @_; - my $dir = tempdir(CLEANUP => 1); - my $path = File::Spec->catfile($dir, $filename); - open my $fh, '>', $path or die "cannot write $path: $!"; - print $fh $sql; - close $fh; - my $output = `$lint -q "$dir" 2>&1`; - my $rc = $? >> 8; - return ($rc, $output); -} - -sub scan_string { - my ($sql) = @_; - my ($fh, $tmp) = tempfile(SUFFIX => '.sql', UNLINK => 1); - print $fh $sql; - close $fh; - return `$lint --scan "$tmp" 2>&1`; -} - -# -- Scanner tests ------------------------------------------------------------- - -subtest 'scanner: CODE lines outside comments' => sub { - my $out = scan_string("SELECT 1;\n"); - like($out, qr/^1\|CODE\|\.\|SELECT 1;$/, 'basic CODE line'); -}; - -subtest 'scanner: block comment labeled COMMENT' => sub { - my $out = scan_string("/*\n * comment\n */\nSELECT 1;\n"); - my @lines = split /\n/, $out; - like($lines[0], qr/\|CODE\|.\|/, 'opening /* is CODE'); - like($lines[1], qr/\|COMMENT\|.\|/, 'body is COMMENT'); - like($lines[2], qr/\|COMMENT\|.\|/, 'closing */ is COMMENT'); - like($lines[3], qr/\|CODE\|.\|/, 'after close is CODE'); -}; - -subtest 'scanner: nested block comments' => sub { - my $out = scan_string("/* outer\n /* inner */\nstill outer\n*/\nSELECT 1;\n"); - my @lines = split /\n/, $out; - like($lines[2], qr/\|COMMENT\|.\|/, 'inner */ only closes inner โ€” still in outer'); - like($lines[3], qr/\|COMMENT\|.\|/, 'outer closing */ is COMMENT'); - like($lines[4], qr/\|CODE\|.\|/, 'after outer close is CODE'); -}; - -subtest 'scanner: /* inside string is not a comment' => sub { - my $out = scan_string("SELECT '/* not a comment */';\nSELECT 2;\n"); - my @lines = split /\n/, $out; - like($lines[0], qr/\|CODE\|.\|/, 'line with /* in string is CODE'); - like($lines[1], qr/\|CODE\|.\|/, 'next line is still CODE'); -}; - -subtest 'scanner: -- prevents /* from opening block' => sub { - my $out = scan_string("SELECT 1; -- /* not a block comment\nSELECT 2;\n"); - my @lines = split /\n/, $out; - like($lines[1], qr/\|CODE\|.\|/, 'next line is CODE (/* was in line comment)'); -}; - -subtest 'scanner: /* in comment body increments depth' => sub { - # This is PostgreSQL-correct: /* inside /* */ is nested. - # The single */ only closes the inner one. - my $out = scan_string("/*\n * mentions /* in prose\n */\nSELECT 1;\n"); - my @lines = split /\n/, $out; - like($lines[3], qr/\|COMMENT\|.\|/, - 'SELECT after */ is still COMMENT โ€” nested /* needs its own */'); -}; - -# -- Regression tests ---------------------------------------------------------- - -subtest 'comma inside single-quoted string is not flagged' => sub { - my ($rc, $out) = lint_string("SELECT 'hello,';\n"); - is($rc, 0, 'exit 0'); -}; - -subtest 'pure -- comment line with comma not flagged as trailing-comma' => sub { - my ($rc, $out) = lint_string("-- SELECT a,\n-- SELECT b,\nSELECT 1;\n"); - unlike($out, qr/trailing-comma/, 'no trailing-comma finding'); -}; - -subtest '/* inside single-quoted string not flagged as comment-opening' => sub { - my ($rc, $out) = lint_string("SELECT '/* not a comment';\n"); - is($rc, 0, 'exit 0'); -}; - -subtest 'comma after /* on same line not flagged as trailing-comma' => sub { - my ($rc, $out) = lint_string("/* text,\n */\nSELECT 1;\n"); - unlike($out, qr/trailing-comma/, 'no trailing-comma finding'); -}; - -subtest '/* */ inside -- comment not flagged as comment-single-line' => sub { - my ($rc, $out) = lint_string("-- Use /* */ for multi-line comments.\nSELECT 1;\n"); - unlike($out, qr/comment-single-line/, 'no comment-single-line finding'); -}; - -subtest '/* */ inside string not flagged as comment-single-line' => sub { - my ($rc, $out) = lint_string("SELECT '/* not a comment */';\n"); - unlike($out, qr/comment-single-line/, 'no comment-single-line finding'); -}; - -subtest 'properly formatted block comment is clean' => sub { - my ($rc, $out) = lint_string("/*\n * First line.\n *\n * Third line.\n */\nSELECT 1;\n"); - unlike($out, qr/comment-line-prefix/, 'no comment-line-prefix finding'); -}; - -subtest 'indented block comment is clean' => sub { - my $sql = <<'SQL'; -CREATE FUNCTION f() RETURNS void LANGUAGE plpgsql AS $body$ -BEGIN - /* - * Indented block comment. - */ - RAISE NOTICE 'hi'; -END; -$body$; -SQL - my ($rc, $out) = lint_string($sql); - unlike($out, qr/comment-line-prefix/, 'no comment-line-prefix finding'); - unlike($out, qr/comment-closing/, 'no comment-closing finding'); -}; - -subtest 'closing */ alone on its line is clean' => sub { - my ($rc, $out) = lint_string("/*\n * Comment.\n */\nSELECT 1;\n"); - unlike($out, qr/comment-closing/, 'no comment-closing finding'); -}; - -subtest 'findings are sorted by line number' => sub { - my ($rc, $out) = lint_string("-- line one\n-- line two\n\n/* bad opening text\n */\n\nSELECT a,\n"); - my @nums = $out =~ /:(\d+):/g; - my @sorted = sort { $a <=> $b } @nums; - is_deeply(\@nums, \@sorted, 'findings in line order'); -}; - -subtest 'empty file produces no findings' => sub { - my ($rc, $out) = lint_string(''); - is($rc, 0, 'exit 0'); -}; - -subtest 'exit code encodes finding count' => sub { - my ($rc, $out) = lint_string("SELECT a,\n"); - is($rc, 11, 'exit 11 = 1 finding + 10'); -}; - -subtest '-- /* line not flagged as comment-opening' => sub { - my ($rc, $out) = lint_string("-- /* this is a line comment\nSELECT 1;\n"); - unlike($out, qr/comment-opening/, 'no comment-opening finding'); -}; - -subtest 'code after inline /* -- */ comment is not lost' => sub { - my ($rc, $out) = lint_string("CREATE TABLE t(a /* flag -- TODO */ integer);\n"); - like($out, qr/prefer-short-type/, 'integer after inline comment is still linted'); -}; - -subtest 'verbose type suggestions are correct' => sub { - my ($rc, $out) = lint_string("CREATE TABLE t(a integer, b bool, c int2);\n"); - like($out, qr/"int" instead of "integer"/, 'integer -> int'); - like($out, qr/"boolean" instead of "bool"/, 'bool -> boolean'); - like($out, qr/"smallint" instead of "int2"/, 'int2 -> smallint'); -}; - -subtest 'inline suppression prevents finding' => sub { - my ($rc, $out) = lint_string( - "SELECT a, -- sql-lint:disable trailing-comma\n"); - is($rc, 0, 'suppressed finding exits 0'); -}; - -subtest 'block suppression: disable-block and EXCLUDED CODE alias' => sub { - # Unprefixed code lines inside a /* */ block trip comment-line-prefix. - my $body = "raw line, not prefixed\nSELECT not_ready(\n foo\n , bar\n);\n"; - - my ($rc_bare) = lint_string("/*\n$body*/\nSELECT 1;\n"); - isnt($rc_bare, 0, 'plain /* */ block flags the unprefixed lines'); - - my ($rc_db) = lint_string("/* sql-lint:disable-block all\n$body*/\nSELECT 1;\n"); - is($rc_db, 0, 'sql-lint:disable-block all suppresses the whole block'); - - my ($rc_ex) = lint_string("/* EXCLUDED CODE\n$body*/\nSELECT 1;\n"); - is($rc_ex, 0, 'EXCLUDED CODE alias suppresses the whole block'); - - my ($rc_ex2) = lint_string("/* EXCLUDED CODE: disabled until #123\n$body*/\nSELECT 1;\n"); - is($rc_ex2, 0, 'EXCLUDED CODE with a trailing description still suppresses'); -}; - -subtest 'region suppression: disable-block/enable-block around real code' => sub { - # Unlike the /* */-anchored disable-block above, this pair works on real - # (uncommented) code and is closed explicitly rather than by a comment's - # own */ -- e.g. for a chunk of borrowed code left deliberately - # unreformatted. - my ($rc_all, $out_all) = lint_string( - "-- sql-lint:disable-block all\nSELECT a,\nSELECT b integer,\n-- sql-lint:enable-block\nSELECT c,\n"); - isnt($rc_all, 0, 'finding after enable-block still flagged'); - unlike($out_all, qr/:2:|:3:/, 'no findings inside the disable-block/enable-block region'); - like($out_all, qr/:5:/, 'finding on line 5 (after enable-block) still reported'); - - my (undef, $out_rule) = lint_string( - "-- sql-lint:disable-block trailing-comma\nSELECT a integer,\n-- sql-lint:enable-block\n"); - like($out_rule, qr/prefer-short-type/, 'disabling one rule does not suppress other rules in the region'); - unlike($out_rule, qr/trailing-comma/, 'disabling one rule does suppress that rule in the region'); - - my ($rc_unclosed) = lint_string("-- sql-lint:disable-block all\nSELECT a,\nSELECT b,\n"); - is($rc_unclosed, 0, 'unclosed disable-block suppresses through EOF'); -}; - -subtest 'multi-line single-quoted string keeps state across lines' => sub { - my $out = scan_string("SELECT 'start\n/* inside the string\nend';\nSELECT 2;\n"); - my @lines = split /\n/, $out; - like($lines[1], qr/\|CODE\|S\|/, 'continuation line inside string has in_string=S'); - like($lines[3], qr/\|CODE\|\.\|/, 'code after string close is CODE with no in_string'); -}; - -subtest 'multi-line string does not cause trailing-comma false positive' => sub { - my ($rc, $out) = lint_string("SELECT 'hello,\nworld';\n"); - unlike($out, qr/trailing-comma/, 'comma inside multi-line string not flagged'); -}; - -subtest 'multi-line string continuation line not checked for types' => sub { - my ($rc, $out) = lint_string("SELECT 'integer\nbool';\n"); - unlike($out, qr/prefer-short-type/, 'type name inside multi-line string not flagged'); -}; - -subtest 'trailing comma before -- with no leading space is flagged' => sub { - # code_content must strip the line comment even without whitespace before --, - # matching the scanner which treats -- as a comment start unconditionally. - my ($rc, $out) = lint_string("SELECT a,--comment\nSELECT 1;\n"); - like($out, qr/trailing-comma/, 'trailing comma detected when -- has no leading space'); -}; - -subtest '-- with no leading space hides a following /* (no false comment-opening)' => sub { - # `1--x /* y`: -- starts a line comment, so the /* is inside it and must not - # be treated as a block-comment opener. The per-rule -- stripping must match - # the scanner (no leading-whitespace requirement). - my ($rc, $out) = lint_string("SELECT 1--x /* y\nSELECT 2;\n"); - unlike($out, qr/comment-opening/, 'no false comment-opening when -- has no leading space'); -}; - -subtest '-- with no leading space hides a following /* */ (no false comment-single-line)' => sub { - my ($rc, $out) = lint_string("SELECT 1--x /* c */\nSELECT 2;\n"); - unlike($out, qr/comment-single-line/, 'no false comment-single-line when -- has no leading space'); -}; - -# -- Edge cases ---------------------------------------------------------------- - -subtest 'no arguments prints usage and exits 2' => sub { - my $out = `$lint 2>&1`; - my $rc = $? >> 8; - is($rc, 2, 'exit 2 for no args'); -}; - -subtest 'nonexistent file exits 2' => sub { - my $out = `$lint -q /nonexistent/file.sql 2>&1`; - my $rc = $? >> 8; - is($rc, 2, 'exit 2 for missing file'); -}; - -subtest 'directory scanning finds files' => sub { - my ($rc, $out) = lint_string("SELECT 1;\n"); - is($rc, 0, 'clean file via directory'); -}; - -subtest 'directory scanning finds .sql.in files' => sub { - # cat_tools authors the hand-maintained SQL source as a .sql.in template - # expanded at build time; the directory walk must not skip it. - my ($rc, $out) = lint_dir_with_file('example.sql.in', "SELECT a,\n"); - like($out, qr/trailing-comma/, '.sql.in file was scanned and linted'); -}; - -subtest '.sql.in generated-file marker still skips the file' => sub { - my ($rc, $out) = lint_dir_with_file('example.sql.in', - "-- GENERATED FILE! DO NOT EDIT! See source.sql.in\nSELECT a,\n"); - is($rc, 0, 'marker (GENERATED ... DO NOT EDIT, either order) skips the file'); -}; - -done_testing(); diff --git a/lint/sql/test/fixtures/comment-closing.bad.sql b/lint/sql/test/fixtures/comment-closing.bad.sql deleted file mode 100644 index 17fd7dd..0000000 --- a/lint/sql/test/fixtures/comment-closing.bad.sql +++ /dev/null @@ -1,7 +0,0 @@ --- expect-findings: 2 -/* - * Text before the closing marker. */ - -/* - * Another comment - * with text before closing */ diff --git a/lint/sql/test/fixtures/comment-closing.good.sql b/lint/sql/test/fixtures/comment-closing.good.sql deleted file mode 100644 index c2e5646..0000000 --- a/lint/sql/test/fixtures/comment-closing.good.sql +++ /dev/null @@ -1,7 +0,0 @@ -/* - * Closing marker is on its own line. - */ - -/* - * Another properly closed comment. - */ diff --git a/lint/sql/test/fixtures/comment-line-prefix.bad.sql b/lint/sql/test/fixtures/comment-line-prefix.bad.sql deleted file mode 100644 index 428fda6..0000000 --- a/lint/sql/test/fixtures/comment-line-prefix.bad.sql +++ /dev/null @@ -1,6 +0,0 @@ --- expect-findings: 2 -/* - Missing the star prefix on this line. - * This line is fine. -also missing the star prefix. - */ diff --git a/lint/sql/test/fixtures/comment-line-prefix.good.sql b/lint/sql/test/fixtures/comment-line-prefix.good.sql deleted file mode 100644 index 9b87003..0000000 --- a/lint/sql/test/fixtures/comment-line-prefix.good.sql +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Every content line starts with " * ". - * Including this one. - */ - -/* sql-lint:disable-block all -A disable-block directive on the opening line exempts the whole block, so code -commented out like this needs no " * " prefixes on each line. -SELECT not_ready_yet( - foo - , bar -); -*/ - -/* EXCLUDED CODE โ€” the friendly alias for disable-block all, for commented-out code -SELECT also_not_ready( - foo - , bar -); -*/ diff --git a/lint/sql/test/fixtures/comment-opening.bad.sql b/lint/sql/test/fixtures/comment-opening.bad.sql deleted file mode 100644 index 74259ac..0000000 --- a/lint/sql/test/fixtures/comment-opening.bad.sql +++ /dev/null @@ -1,7 +0,0 @@ --- expect-findings: 2 -/* This text should not be on the opening line - * of a multi-line comment. - */ - -/* Also bad: text after the opening - */ diff --git a/lint/sql/test/fixtures/comment-opening.good.sql b/lint/sql/test/fixtures/comment-opening.good.sql deleted file mode 100644 index b703aff..0000000 --- a/lint/sql/test/fixtures/comment-opening.good.sql +++ /dev/null @@ -1,9 +0,0 @@ -/* - * Opening marker is alone on its line โ€” correct. - */ - -/* - * Another properly opened block comment. - */ - --- /* this is a line comment, not a block comment opening diff --git a/lint/sql/test/fixtures/comment-padding.bad.sql b/lint/sql/test/fixtures/comment-padding.bad.sql deleted file mode 100644 index f62ff6b..0000000 --- a/lint/sql/test/fixtures/comment-padding.bad.sql +++ /dev/null @@ -1,10 +0,0 @@ --- expect-findings: 2 -/* - * - * Leading blank line above is unnecessary. - */ - -/* - * Trailing blank line below is unnecessary. - * - */ diff --git a/lint/sql/test/fixtures/comment-padding.good.sql b/lint/sql/test/fixtures/comment-padding.good.sql deleted file mode 100644 index 428528e..0000000 --- a/lint/sql/test/fixtures/comment-padding.good.sql +++ /dev/null @@ -1,9 +0,0 @@ -/* - * No padding lines here. - */ - -/* - * Interior blank lines are fine. - * - * Like the one above. - */ diff --git a/lint/sql/test/fixtures/comment-single-line.bad.sql b/lint/sql/test/fixtures/comment-single-line.bad.sql deleted file mode 100644 index 9e1e56a..0000000 --- a/lint/sql/test/fixtures/comment-single-line.bad.sql +++ /dev/null @@ -1,4 +0,0 @@ --- expect-findings: 2 -/* This is a single-line block comment. */ - -/* Also caught: inline with code */ SELECT 1; diff --git a/lint/sql/test/fixtures/comment-single-line.good.sql b/lint/sql/test/fixtures/comment-single-line.good.sql deleted file mode 100644 index e946f34..0000000 --- a/lint/sql/test/fixtures/comment-single-line.good.sql +++ /dev/null @@ -1,7 +0,0 @@ --- Single-line comments are fine. - -/* - * Multi-line block comments are fine. - */ - -/* suppressed single-line block comment */ -- sql-lint:disable comment-single-line diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql b/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql deleted file mode 100644 index 9b72b35..0000000 --- a/lint/sql/test/fixtures/comment-stacked-dashes.bad.sql +++ /dev/null @@ -1,8 +0,0 @@ --- expect-findings: 2 --- This is a multi-line comment --- written with stacked dash lines. --- It should use /* */ instead. - --- This is another --- stacked comment --- spanning three lines. diff --git a/lint/sql/test/fixtures/comment-stacked-dashes.good.sql b/lint/sql/test/fixtures/comment-stacked-dashes.good.sql deleted file mode 100644 index 6d14e02..0000000 --- a/lint/sql/test/fixtures/comment-stacked-dashes.good.sql +++ /dev/null @@ -1,21 +0,0 @@ --- A single-line comment is fine. - --- Another single-line comment, separated by a blank line. - --- Two consecutive -- lines are also fine; --- only a run of 3+ must become a /* */ block. - --- /* this is a line comment, not a block comment opening - ---/* no space variant - --- This is a stacked comment -- sql-lint:disable comment-stacked-dashes --- of three lines that would otherwise --- be flagged, but is suppressed. - -/* - * This block comment contains lines that look like stacked dashes: - * -- line one - * -- line two - * But they are inside a block comment and should NOT be flagged. - */ diff --git a/lint/sql/test/fixtures/generated-file-skipped.good.sql b/lint/sql/test/fixtures/generated-file-skipped.good.sql deleted file mode 100644 index 187f5b4..0000000 --- a/lint/sql/test/fixtures/generated-file-skipped.good.sql +++ /dev/null @@ -1,5 +0,0 @@ -/* DO NOT EDIT - AUTO-GENERATED FILE */ -/* This single-line block comment would trip comment-single-line, and the stacked --- dashes below would trip comment-stacked-dashes, but a file whose first line is --- the AUTO-GENERATED marker is skipped entirely, so this fixture yields zero --- findings. */ diff --git a/lint/sql/test/fixtures/prefer-short-type.bad.sql b/lint/sql/test/fixtures/prefer-short-type.bad.sql deleted file mode 100644 index 84db8a4..0000000 --- a/lint/sql/test/fixtures/prefer-short-type.bad.sql +++ /dev/null @@ -1,12 +0,0 @@ --- expect-findings: 9 -CREATE TABLE long_types( - long_types_id integer NOT NULL PRIMARY KEY - , flag bool NOT NULL - , name character varying(100) NOT NULL - , amount decimal NOT NULL - , score float8 - , small_val int2 - , big_val int8 - , approx float4 - , internal int4 -); diff --git a/lint/sql/test/fixtures/prefer-short-type.good.sql b/lint/sql/test/fixtures/prefer-short-type.good.sql deleted file mode 100644 index 1d83749..0000000 --- a/lint/sql/test/fixtures/prefer-short-type.good.sql +++ /dev/null @@ -1,19 +0,0 @@ -CREATE TABLE short_types( - short_types_id int NOT NULL PRIMARY KEY - , flag boolean NOT NULL - , name varchar(100) NOT NULL - , label text NOT NULL - , amount numeric NOT NULL - , score double precision - , small_val smallint - , big_val bigint - , approx real - , ts timestamp - , tstz timestamptz -); -/* - * Type names inside identifiers should not be flagged. - * "is_real_value", "integer_count", "boolean_flag" are identifiers, not types. - */ -CREATE FUNCTION is_real_value(integer_count int, boolean_flag boolean) RETURNS boolean -LANGUAGE sql AS $$SELECT true$$; diff --git a/lint/sql/test/fixtures/trailing-comma.bad.sql b/lint/sql/test/fixtures/trailing-comma.bad.sql deleted file mode 100644 index d20b727..0000000 --- a/lint/sql/test/fixtures/trailing-comma.bad.sql +++ /dev/null @@ -1,30 +0,0 @@ --- expect-findings: 7 -SELECT - column1, - column2, - column3 -FROM some_table -; - -CREATE TABLE bad_table( - id serial NOT NULL PRIMARY KEY, - name text NOT NULL, - status text -); - -INSERT INTO t(a, b, c) -VALUES ( - 1, - 2, - 3 -); - -/* - * A genuine trailing comma after a lone string literal must still be caught - * (the sentinel keeps the comma on the trailing side after string-stripping). - */ -INSERT INTO t(a) -VALUES ( - 'x', - 'y' -); diff --git a/lint/sql/test/fixtures/trailing-comma.good.sql b/lint/sql/test/fixtures/trailing-comma.good.sql deleted file mode 100644 index 405a6d6..0000000 --- a/lint/sql/test/fixtures/trailing-comma.good.sql +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Lines that are pure -- comments should not trigger trailing-comma, - * even if the comment text contains a comma. - */ - --- This is a comment with a trailing comma, - --- And another one with a comma, - -SELECT column1 -- inline comment - , column2 -- another inline comment - , column3 -FROM some_table -; - -SELECT - column1, -- sql-lint:disable trailing-comma - column2 -; - -/* - * Leading-comma items whose entire payload is a string literal are correct - * leading-comma style and must NOT be flagged. (Regression: stripping the - * string used to leave a bare leading comma that looked like a trailing one.) - */ -SELECT is( - 1 - , 'a description' - , 'another, with an embedded comma' -); - -CALL pg_temp.routine( - 'FUNCTION' - , 'clean_routine_args' - , 'args text' -); - --- A leading comma may sit alone on its line when it introduces a multi-line item. -SELECT row_eq( - 'SELECT ' || call - , row(out) - , - 'SELECT ' || call - || ' should return ' - || coalesce(out::text, 'NULL') -); From 1be1d2bccbdc0f9a839ddbd00e121bd6866ea03c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 17:17:52 -0500 Subject: [PATCH 04/11] Explain why linting is disabled on the borrowed pg_all_foreign_keys view Bumps .vendor/linter to Postgres-Extensions/linter@b8632c2, which also fixes a bug the missing comment surfaced: annotate_region_suppressions() captured the rule id with (\S+), so a same-line reason after a colon (no space before it, e.g. `all: some reason`) got swallowed into the captured token -- "all:" never equals "all", so the region silently stopped suppressing anything. Fixed upstream to anchor the capture to the rule id itself. make lint, make install, and make verify-results all still green. --- .vendor/linter | 2 +- sql/cat_tools--0.3.0.sql.in | 2 +- sql/cat_tools.sql.in | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.vendor/linter b/.vendor/linter index e8575e5..b8632c2 160000 --- a/.vendor/linter +++ b/.vendor/linter @@ -1 +1 @@ -Subproject commit e8575e5c04b4ca28304d36d661f73650527876d5 +Subproject commit b8632c2a3d93de664a45f4622235871e8f19cf78 diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index 30b8020..f52462e 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -1282,7 +1282,7 @@ $$ @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ --- sql-lint:disable-block all +-- sql-lint:disable-block all: copied verbatim from upstream, do not reformat CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS SELECT n1.nspname AS fk_schema_name, diff --git a/sql/cat_tools.sql.in b/sql/cat_tools.sql.in index 30b8020..f52462e 100644 --- a/sql/cat_tools.sql.in +++ b/sql/cat_tools.sql.in @@ -1282,7 +1282,7 @@ $$ @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ --- sql-lint:disable-block all +-- sql-lint:disable-block all: copied verbatim from upstream, do not reformat CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS SELECT n1.nspname AS fk_schema_name, From 1f956fbe9ce15e52bd90627d813bc054dfd2e146 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 17:24:04 -0500 Subject: [PATCH 05/11] sql.mk: tag the version-file copy step directly, not the shared build rule Simpler than the previous approach: instead of the shared sql/%.sql build rule branching on $* (whether the stem contains --), the copy step that makes sql/cat_tools--.sql.in from sql/cat_tools.sql.in now tags its own @generated@ divider markers as 'VERSIONED FILE! @generated@' (anchored to the whole line, so the copy doesn't touch the handful of inline coincidental @generated@ occurrences on real code lines either). The shared build rule just resolves whichever form -- tagged or bare -- it finds, with no knowledge of which file it's building. One consequence of tagging in the copy step rather than the build rule: the echo-inserted top/bottom boundary markers (added fresh at build time, not part of the .sql.in content) stay plain even for a version-file build -- only the ~53 internal divider markers that came from the copied content carry the tag. Still plenty visible at a glance, just not on literally every marker in the file. Regenerated sql/cat_tools--0.3.0.sql.in to match. make lint, make install, and make verify-results all still green. --- sql.mk | 43 +++++++-------- sql/cat_tools--0.3.0.sql.in | 106 ++++++++++++++++++------------------ 2 files changed, 73 insertions(+), 76 deletions(-) diff --git a/sql.mk b/sql.mk index 3154c2c..3c1ad44 100644 --- a/sql.mk +++ b/sql.mk @@ -207,40 +207,37 @@ endef # are matched anchored to the WHOLE line -- not a plain substring -- so an # @generated@ that coincidentally appears as a trailing annotation on a real # code line (there are a few in cat_tools.sql.in) is left untouched instead -# of being clobbered into generic marker text. $* (the %-match) tells apart a -# version-specific file (stem contains --, e.g. cat_tools--0.2.2) from the -# master (stem is just cat_tools): version files get an extra "VERSIONED -# FILE!" tag up front so it's obvious at a glance that the file is a frozen -# snapshot that must never be hand-edited. +# of being clobbered into generic marker text. A marker line may or may not +# already carry the "VERSIONED FILE! " prefix (see the copy rule below, which +# adds it when making a version-specific .sql.in) -- either way resolves to +# the same trailing "GENERATED FILE! DO NOT EDIT!" text, just with that extra +# tag kept up front for version files, so it's obvious at a glance that the +# file is a frozen snapshot that must never be hand-edited. sql/%.sql: sql/%.sql.in pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | awk -v src='$<' -v stem='$*' '\ - /^@generated@$$/ { \ - if (stem ~ /--/) print "-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See " src; \ - else print "-- GENERATED FILE! DO NOT EDIT! See " src; \ - next \ - } \ - { print }' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed \ + -e 's#^VERSIONED FILE! @generated@$$#-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See $<#' \ + -e 's#^@generated@$$#-- GENERATED FILE! DO NOT EDIT! See $<#' \ + > $@.tmp $(_apply_version_seds) mv $@.tmp $@ -# Make the current version's .sql.in by copying the base source; the pattern -# rule above then turns it into the final .sql with SED substitutions applied. +# Make the current version's .sql.in by copying the base source, tagging its +# @generated@ markers as version-specific along the way (see the pattern rule +# above for how that tag is resolved at .sql build time). Anchored to the +# WHOLE line for the same reason as above: a coincidental inline @generated@ +# on a real code line must not gain the tag either. # (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control - cp $< $@ + sed -e 's/^@generated@$$/VERSIONED FILE! @generated@/' $< > $@ # Override control.mk's rule (which builds EXTENSION_VERSION_FILES straight from # cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with # version-conditional substitutions applied. GNU Make's "overriding recipe" # warning for this target is expected. -# -# $* is not meaningful for this static (non-pattern) rule, so unlike the -# pattern rule above this always tags with "VERSIONED FILE!" -- everything -# built here is, by definition (see EXTENSION_VERSION_FILES above), a -# version-specific file. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | awk -v src='$<' '\ - /^@generated@$$/ { print "-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See " src; next } \ - { print }' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed \ + -e 's#^VERSIONED FILE! @generated@$$#-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See $<#' \ + -e 's#^@generated@$$#-- GENERATED FILE! DO NOT EDIT! See $<#' \ + > $@.tmp $(_apply_version_seds) mv $@.tmp $@ diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index f52462e..2572467 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -1,4 +1,4 @@ -@generated@ +VERSIONED FILE! @generated@ DO $$ BEGIN @@ -21,7 +21,7 @@ GRANT USAGE ON SCHEMA cat_tools TO cat_tools__usage; ALTER DEFAULT PRIVILEGES IN SCHEMA cat_tools GRANT USAGE ON TYPES TO cat_tools__usage; CREATE SCHEMA _cat_tools; -@generated@ +VERSIONED FILE! @generated@ CREATE FUNCTION __cat_tools.exec( sql text @@ -50,7 +50,7 @@ SELECT array_to_string(array( ) $body$; -@generated@ +VERSIONED FILE! @generated@ /* * Starting in PG12 oid columns in catalog tables are no longer hidden, so we @@ -117,7 +117,7 @@ COMMENT ON FUNCTION %s( $template$ ; -@generated@ +VERSIONED FILE! @generated@ BEGIN PERFORM __cat_tools.exec( format( @@ -157,7 +157,7 @@ BEGIN END $body$; -@generated@ +VERSIONED FILE! @generated@ /* * These _cat_tools helper functions are created via __cat_tools.create_function @@ -264,7 +264,7 @@ $body$ GRANT USAGE ON SCHEMA _cat_tools TO cat_tools__usage; -@generated@ +VERSIONED FILE! @generated@ -- Data type definitions CREATE TYPE cat_tools.constraint_type AS ENUM( @@ -520,7 +520,7 @@ $body$ , 'Mapping from cat_tools.routine_proparallel to cat_tools.routine_parallel_safety' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__arg_types' @@ -535,7 +535,7 @@ $body$ , 'Returns all argument types for a function as an array of regtype' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__arg_names' @@ -577,7 +577,7 @@ $body$ , 'Returns all argument names for a function as an array of text. Empty strings are converted to NULL.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__arg_types_text' @@ -590,7 +590,7 @@ $body$ , 'Returns all argument types for a function as a comma-separated text string' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__arg_names_text' @@ -603,7 +603,7 @@ $body$ , 'Returns all argument names for a function as a comma-separated text string' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_types' @@ -628,7 +628,7 @@ $body$ defining a function.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_names' @@ -653,7 +653,7 @@ $body$ behavior). Unnamed arguments appear as NULL in the result array.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_types_text' @@ -669,7 +669,7 @@ $body$ ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_names_text' @@ -685,7 +685,7 @@ $body$ ); -@generated@ +VERSIONED FILE! @generated@ -- Deprecated wrapper functions for backwards compatibility SELECT __cat_tools.create_function( @@ -705,7 +705,7 @@ $body$ includes IN, INOUT, and VARIADIC arguments.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.function__arg_types_text' @@ -724,7 +724,7 @@ $body$ includes IN, INOUT, and VARIADIC arguments.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.regprocedure' @@ -746,10 +746,10 @@ $body$ ); -@generated@ +VERSIONED FILE! @generated@ -@generated@ +VERSIONED FILE! @generated@ CREATE TYPE cat_tools.object_type AS ENUM( -- pg_class @@ -815,7 +815,7 @@ CREATE TYPE cat_tools.object_type AS ENUM( ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.objects__shared' @@ -858,7 +858,7 @@ $body$ , 'Returns true if object_type is a shared object.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.objects__address_unsupported' @@ -884,7 +884,7 @@ $body$ , 'cat_tools__usage' , 'Returns set of object types not supported by pg_get_object_address().' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__is_address_unsupported' , 'object_type cat_tools.object_type' @@ -906,7 +906,7 @@ $body$ , 'Returns true if object type is not supported by pg_get_object_address().' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__catalog' @@ -964,7 +964,7 @@ $body$ , 'cat_tools__usage' , 'Returns catalog table that is used to store objects' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__catalog' , 'object_type text' @@ -974,7 +974,7 @@ SELECT __cat_tools.create_function( , 'Returns catalog table that is used to store objects' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__address_classid' @@ -999,7 +999,7 @@ SELECT __cat_tools.create_function( , 'Returns the classid used by the pg_*_object*() functions for an object_type' ); -@generated@ +VERSIONED FILE! @generated@ CREATE TABLE _cat_tools.catalog_metadata( object_catalog pg_catalog.regclass @@ -1028,7 +1028,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__reg_type' @@ -1057,7 +1057,7 @@ SELECT __cat_tools.create_function( , 'Returns the object identifier type (ie: regclass) associated with a system catalog (ie: pg_class).' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.object__reg_type_catalog' @@ -1094,9 +1094,9 @@ $body$ , 'Returns the system catalog that stores a particular object identifier type.' ); -@generated@ +VERSIONED FILE! @generated@ -@generated@ +VERSIONED FILE! @generated@ CREATE OR REPLACE VIEW _cat_tools.pg_depend_identity_v AS -- SED: REQUIRES 9.3! SELECT o.type AS object_type -- SED: REQUIRES 9.3! @@ -1124,7 +1124,7 @@ CREATE OR REPLACE VIEW _cat_tools.pg_depend_identity_v AS -- SED: REQUIRES 9.3! WHERE classid = 0 -- SED: REQUIRES 9.3! ; -- SED: REQUIRES 9.3! -@generated@ +VERSIONED FILE! @generated@ CREATE OR REPLACE VIEW cat_tools.pg_class_v AS SELECT * @@ -1139,7 +1139,7 @@ CREATE OR REPLACE VIEW cat_tools.pg_class_v AS ; GRANT SELECT ON cat_tools.pg_class_v TO cat_tools__usage; -@generated@ +VERSIONED FILE! @generated@ /* * On PG11+, pg_attribute gained attmissingval (pseudo-type anyarray, not usable in views). @@ -1203,7 +1203,7 @@ $fmt$ )); REVOKE ALL ON _cat_tools.column FROM public; -@generated@ +VERSIONED FILE! @generated@ /* * Starting in PG12, oid became a visible column in system catalogs. @@ -1258,7 +1258,7 @@ SELECT __cat_tools.create_function( $$ ); -@generated@ +VERSIONED FILE! @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ SELECT __cat_tools.create_function( @@ -1279,7 +1279,7 @@ SELECT __cat_tools.create_function( $$ ); -@generated@ +VERSIONED FILE! @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ -- sql-lint:disable-block all: copied verbatim from upstream, do not reformat @@ -1351,7 +1351,7 @@ AS -- sql-lint:enable-block GRANT SELECT ON cat_tools.pg_all_foreign_keys TO cat_tools__usage; -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.pg_attribute__get' @@ -1380,7 +1380,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.pg_extension__get' @@ -1431,7 +1431,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ -- Text versions SELECT __cat_tools.create_function( @@ -1464,7 +1464,7 @@ $body$ ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.get_serial_sequence' @@ -1495,7 +1495,7 @@ $body$ , 'Return sequence that is associated with a column. Unlike the pg_get_serial_sequence, throw an exception if there is no sequence associated with the column.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.sequence__last' @@ -1520,7 +1520,7 @@ $$ , 'Return the last value assigned to a column with an associated sequence.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.sequence__next' @@ -1545,7 +1545,7 @@ $$ , 'Return the next value to assign to a column with an associated sequence. THIS ADVANCES THE SEQUENCE.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.setval' @@ -1585,7 +1585,7 @@ $$ , 'Changes the value for a sequence associated with a column. next_value is the next value the sequence will assign. See also sequence__last_value.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.enum_range' @@ -1602,7 +1602,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.enum_range_srf' @@ -1624,7 +1624,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.relation__is_temp' @@ -1667,7 +1667,7 @@ $body$ , 'Returns an array of quoted column names for a relation in ordinal position order.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.name__check' @@ -1683,7 +1683,7 @@ $body$ , 'cat_tools__usage' ); -@generated@ +VERSIONED FILE! @generated@ /* * Trigger functions @@ -1713,7 +1713,7 @@ $body$ , 'Return the OID for a trigger. Returns NULL if trigger does not exist.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.trigger__get_oid' @@ -1742,7 +1742,7 @@ $body$ , 'Return the OID for a trigger. Throws an undefined_object error if the trigger does not exist.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.trigger__parse' @@ -1870,7 +1870,7 @@ $body$ , 'Provide details about a trigger.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.trigger__parse' @@ -1904,7 +1904,7 @@ $body$ , 'Provide details about a trigger.' ); -@generated@ +VERSIONED FILE! @generated@ SELECT __cat_tools.create_function( 'cat_tools.trigger__args_as_text' @@ -1923,7 +1923,7 @@ $body$ , 'Convert function_arguments as returned by trigger__parse() to text (for backwards compatibility).' ); -@generated@ +VERSIONED FILE! @generated@ INSERT INTO _cat_tools.catalog_metadata(object_catalog, reg_type, namespace_field) SELECT object__catalog @@ -1958,7 +1958,7 @@ UPDATE _cat_tools.catalog_metadata -- Cluster to get rid of dead rows CLUSTER _cat_tools.catalog_metadata USING catalog_metadata__pk_object_catalog; -@generated@ +VERSIONED FILE! @generated@ /* * Drop "temporary" objects From 4c9f1d510c7d5ecf5063ce0891646b128bc0971c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 24 Jul 2026 18:07:07 -0500 Subject: [PATCH 06/11] sql.mk: put VERSIONED FILE! after @generated@ instead of before @generated@ itself is what the shared build rule's sed substitutes into the '-- GENERATED FILE! DO NOT EDIT!' marker -- prefixing 'VERSIONED FILE! ' before it meant the shared rule had to special-case matching that whole prefixed string separately from a bare @generated@ line, duplicating the substitution. Appending ' VERSIONED FILE!' after @generated@ instead means the shared rule's substitution -- now anchored to just the START of the line, not the whole line -- resolves the @generated@ prefix exactly the same way whether or not a version tag follows, and whatever trailing text is present just rides along into the final marker text. So the two build recipes (the sql/%.sql pattern rule and the EXTENSION_VERSION_FILES override) both drop back to the single substitution they had before any of the VERSIONED FILE! work -- just with the anchor now caring about line start rather than nothing at all, which is what actually avoids clobbering the handful of inline @generated@ occurrences that are trailing annotations on real code lines. Regenerated sql/cat_tools--0.3.0.sql.in to match. make lint, make install, and make verify-results all still green. --- sql.mk | 43 +++++++-------- sql/cat_tools--0.3.0.sql.in | 106 ++++++++++++++++++------------------ 2 files changed, 72 insertions(+), 77 deletions(-) diff --git a/sql.mk b/sql.mk index 3c1ad44..41498e9 100644 --- a/sql.mk +++ b/sql.mk @@ -202,42 +202,37 @@ endef # The recipe builds $@.tmp then moves it into place atomically. # TODO: refactor the version handling into a function. # -# The boundary/divider @generated@ markers (added by the echo's above, plus -# any standalone ones already in the source used as visual section dividers) -# are matched anchored to the WHOLE line -- not a plain substring -- so an -# @generated@ that coincidentally appears as a trailing annotation on a real -# code line (there are a few in cat_tools.sql.in) is left untouched instead -# of being clobbered into generic marker text. A marker line may or may not -# already carry the "VERSIONED FILE! " prefix (see the copy rule below, which -# adds it when making a version-specific .sql.in) -- either way resolves to -# the same trailing "GENERATED FILE! DO NOT EDIT!" text, just with that extra -# tag kept up front for version files, so it's obvious at a glance that the -# file is a frozen snapshot that must never be hand-edited. +# @generated@ is what actually becomes the "-- GENERATED FILE! DO NOT EDIT!" +# marker below -- it's matched anchored to the START of the line only (not +# the whole line), so any trailing text after it survives untouched. That's +# what lets the copy rule below tag a version file's own @generated@ markers +# with a trailing "VERSIONED FILE!" (see there): this substitution doesn't +# need to know or care whether that tag is present, it just resolves the +# @generated@ prefix either way. The start anchor also means an @generated@ +# that coincidentally appears as a trailing annotation on a real code line +# (there are a few in cat_tools.sql.in) is left untouched instead of being +# clobbered into generic marker text -- it's never at the start of its line. sql/%.sql: sql/%.sql.in pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed \ - -e 's#^VERSIONED FILE! @generated@$$#-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See $<#' \ - -e 's#^@generated@$$#-- GENERATED FILE! DO NOT EDIT! See $<#' \ - > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#^@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ # Make the current version's .sql.in by copying the base source, tagging its -# @generated@ markers as version-specific along the way (see the pattern rule -# above for how that tag is resolved at .sql build time). Anchored to the -# WHOLE line for the same reason as above: a coincidental inline @generated@ -# on a real code line must not gain the tag either. +# @generated@ divider markers as version-specific along the way: the pattern +# rule above resolves the leading @generated@ regardless, so appending +# " VERSIONED FILE!" survives into the final marker text, making it obvious +# at a glance that the built .sql is a frozen snapshot that must never be +# hand-edited. Anchored to the START of the line so a coincidental inline +# @generated@ on a real code line (not at the start) is left untouched. # (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control - sed -e 's/^@generated@$$/VERSIONED FILE! @generated@/' $< > $@ + sed -e 's/^@generated@$$/@generated@ VERSIONED FILE!/' $< > $@ # Override control.mk's rule (which builds EXTENSION_VERSION_FILES straight from # cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with # version-conditional substitutions applied. GNU Make's "overriding recipe" # warning for this target is expected. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed \ - -e 's#^VERSIONED FILE! @generated@$$#-- VERSIONED FILE! GENERATED FILE! DO NOT EDIT! See $<#' \ - -e 's#^@generated@$$#-- GENERATED FILE! DO NOT EDIT! See $<#' \ - > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#^@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index 2572467..f7a89d6 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -1,4 +1,4 @@ -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! DO $$ BEGIN @@ -21,7 +21,7 @@ GRANT USAGE ON SCHEMA cat_tools TO cat_tools__usage; ALTER DEFAULT PRIVILEGES IN SCHEMA cat_tools GRANT USAGE ON TYPES TO cat_tools__usage; CREATE SCHEMA _cat_tools; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! CREATE FUNCTION __cat_tools.exec( sql text @@ -50,7 +50,7 @@ SELECT array_to_string(array( ) $body$; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * Starting in PG12 oid columns in catalog tables are no longer hidden, so we @@ -117,7 +117,7 @@ COMMENT ON FUNCTION %s( $template$ ; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! BEGIN PERFORM __cat_tools.exec( format( @@ -157,7 +157,7 @@ BEGIN END $body$; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * These _cat_tools helper functions are created via __cat_tools.create_function @@ -264,7 +264,7 @@ $body$ GRANT USAGE ON SCHEMA _cat_tools TO cat_tools__usage; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -- Data type definitions CREATE TYPE cat_tools.constraint_type AS ENUM( @@ -520,7 +520,7 @@ $body$ , 'Mapping from cat_tools.routine_proparallel to cat_tools.routine_parallel_safety' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__arg_types' @@ -535,7 +535,7 @@ $body$ , 'Returns all argument types for a function as an array of regtype' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__arg_names' @@ -577,7 +577,7 @@ $body$ , 'Returns all argument names for a function as an array of text. Empty strings are converted to NULL.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__arg_types_text' @@ -590,7 +590,7 @@ $body$ , 'Returns all argument types for a function as a comma-separated text string' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__arg_names_text' @@ -603,7 +603,7 @@ $body$ , 'Returns all argument names for a function as a comma-separated text string' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_types' @@ -628,7 +628,7 @@ $body$ defining a function.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_names' @@ -653,7 +653,7 @@ $body$ behavior). Unnamed arguments appear as NULL in the result array.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_types_text' @@ -669,7 +669,7 @@ $body$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.routine__parse_arg_names_text' @@ -685,7 +685,7 @@ $body$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -- Deprecated wrapper functions for backwards compatibility SELECT __cat_tools.create_function( @@ -705,7 +705,7 @@ $body$ includes IN, INOUT, and VARIADIC arguments.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.function__arg_types_text' @@ -724,7 +724,7 @@ $body$ includes IN, INOUT, and VARIADIC arguments.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.regprocedure' @@ -746,10 +746,10 @@ $body$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! CREATE TYPE cat_tools.object_type AS ENUM( -- pg_class @@ -815,7 +815,7 @@ CREATE TYPE cat_tools.object_type AS ENUM( ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.objects__shared' @@ -858,7 +858,7 @@ $body$ , 'Returns true if object_type is a shared object.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.objects__address_unsupported' @@ -884,7 +884,7 @@ $body$ , 'cat_tools__usage' , 'Returns set of object types not supported by pg_get_object_address().' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__is_address_unsupported' , 'object_type cat_tools.object_type' @@ -906,7 +906,7 @@ $body$ , 'Returns true if object type is not supported by pg_get_object_address().' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__catalog' @@ -964,7 +964,7 @@ $body$ , 'cat_tools__usage' , 'Returns catalog table that is used to store objects' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__catalog' , 'object_type text' @@ -974,7 +974,7 @@ SELECT __cat_tools.create_function( , 'Returns catalog table that is used to store objects' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__address_classid' @@ -999,7 +999,7 @@ SELECT __cat_tools.create_function( , 'Returns the classid used by the pg_*_object*() functions for an object_type' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! CREATE TABLE _cat_tools.catalog_metadata( object_catalog pg_catalog.regclass @@ -1028,7 +1028,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__reg_type' @@ -1057,7 +1057,7 @@ SELECT __cat_tools.create_function( , 'Returns the object identifier type (ie: regclass) associated with a system catalog (ie: pg_class).' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.object__reg_type_catalog' @@ -1094,9 +1094,9 @@ $body$ , 'Returns the system catalog that stores a particular object identifier type.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! CREATE OR REPLACE VIEW _cat_tools.pg_depend_identity_v AS -- SED: REQUIRES 9.3! SELECT o.type AS object_type -- SED: REQUIRES 9.3! @@ -1124,7 +1124,7 @@ CREATE OR REPLACE VIEW _cat_tools.pg_depend_identity_v AS -- SED: REQUIRES 9.3! WHERE classid = 0 -- SED: REQUIRES 9.3! ; -- SED: REQUIRES 9.3! -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! CREATE OR REPLACE VIEW cat_tools.pg_class_v AS SELECT * @@ -1139,7 +1139,7 @@ CREATE OR REPLACE VIEW cat_tools.pg_class_v AS ; GRANT SELECT ON cat_tools.pg_class_v TO cat_tools__usage; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * On PG11+, pg_attribute gained attmissingval (pseudo-type anyarray, not usable in views). @@ -1203,7 +1203,7 @@ $fmt$ )); REVOKE ALL ON _cat_tools.column FROM public; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * Starting in PG12, oid became a visible column in system catalogs. @@ -1258,7 +1258,7 @@ SELECT __cat_tools.create_function( $$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ SELECT __cat_tools.create_function( @@ -1279,7 +1279,7 @@ SELECT __cat_tools.create_function( $$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ -- sql-lint:disable-block all: copied verbatim from upstream, do not reformat @@ -1351,7 +1351,7 @@ AS -- sql-lint:enable-block GRANT SELECT ON cat_tools.pg_all_foreign_keys TO cat_tools__usage; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.pg_attribute__get' @@ -1380,7 +1380,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.pg_extension__get' @@ -1431,7 +1431,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! -- Text versions SELECT __cat_tools.create_function( @@ -1464,7 +1464,7 @@ $body$ ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.get_serial_sequence' @@ -1495,7 +1495,7 @@ $body$ , 'Return sequence that is associated with a column. Unlike the pg_get_serial_sequence, throw an exception if there is no sequence associated with the column.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.sequence__last' @@ -1520,7 +1520,7 @@ $$ , 'Return the last value assigned to a column with an associated sequence.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.sequence__next' @@ -1545,7 +1545,7 @@ $$ , 'Return the next value to assign to a column with an associated sequence. THIS ADVANCES THE SEQUENCE.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.setval' @@ -1585,7 +1585,7 @@ $$ , 'Changes the value for a sequence associated with a column. next_value is the next value the sequence will assign. See also sequence__last_value.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.enum_range' @@ -1602,7 +1602,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.enum_range_srf' @@ -1624,7 +1624,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.relation__is_temp' @@ -1667,7 +1667,7 @@ $body$ , 'Returns an array of quoted column names for a relation in ordinal position order.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.name__check' @@ -1683,7 +1683,7 @@ $body$ , 'cat_tools__usage' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * Trigger functions @@ -1713,7 +1713,7 @@ $body$ , 'Return the OID for a trigger. Returns NULL if trigger does not exist.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.trigger__get_oid' @@ -1742,7 +1742,7 @@ $body$ , 'Return the OID for a trigger. Throws an undefined_object error if the trigger does not exist.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.trigger__parse' @@ -1870,7 +1870,7 @@ $body$ , 'Provide details about a trigger.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.trigger__parse' @@ -1904,7 +1904,7 @@ $body$ , 'Provide details about a trigger.' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! SELECT __cat_tools.create_function( 'cat_tools.trigger__args_as_text' @@ -1923,7 +1923,7 @@ $body$ , 'Convert function_arguments as returned by trigger__parse() to text (for backwards compatibility).' ); -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! INSERT INTO _cat_tools.catalog_metadata(object_catalog, reg_type, namespace_field) SELECT object__catalog @@ -1958,7 +1958,7 @@ UPDATE _cat_tools.catalog_metadata -- Cluster to get rid of dead rows CLUSTER _cat_tools.catalog_metadata USING catalog_metadata__pk_object_catalog; -VERSIONED FILE! @generated@ +@generated@ VERSIONED FILE! /* * Drop "temporary" objects From de1690550fb23bb9b2ff0c62b578ea542ae130a0 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 26 Jul 2026 12:45:32 -0500 Subject: [PATCH 07/11] sql-lint:disable-block: fix wrong provenance name in comment The pg_all_foreign_keys view is borrowed from newsysviews (per the attribution comment right above), not "upstream" -- match it. Co-Authored-By: Claude Sonnet 5 --- sql/cat_tools--0.3.0.sql.in | 2 +- sql/cat_tools.sql.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index f7a89d6..c817f89 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -1282,7 +1282,7 @@ $$ @generated@ VERSIONED FILE! -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ --- sql-lint:disable-block all: copied verbatim from upstream, do not reformat +-- sql-lint:disable-block all: copied verbatim from newsysviews, do not reformat CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS SELECT n1.nspname AS fk_schema_name, diff --git a/sql/cat_tools.sql.in b/sql/cat_tools.sql.in index f52462e..339ff00 100644 --- a/sql/cat_tools.sql.in +++ b/sql/cat_tools.sql.in @@ -1282,7 +1282,7 @@ $$ @generated@ -- Borrowed from newsysviews: http://pgfoundry.org/projects/newsysviews/ --- sql-lint:disable-block all: copied verbatim from upstream, do not reformat +-- sql-lint:disable-block all: copied verbatim from newsysviews, do not reformat CREATE OR REPLACE VIEW cat_tools.pg_all_foreign_keys AS SELECT n1.nspname AS fk_schema_name, From 53ee55a69267d0376728f5b39657a9a0108326cd Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 26 Jul 2026 12:52:39 -0500 Subject: [PATCH 08/11] sql.mk: explain why the EXTENSION_VERSION_FILES rule must be overridden Name the actual conflicting rule (root control.mk, generated by pgxntool from cat_tools.control) and why GNU Make would prefer it over the sql/%.sql pattern rule if we didn't override it here. Co-Authored-By: Claude Sonnet 5 --- sql.mk | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sql.mk b/sql.mk index 41498e9..840a8bb 100644 --- a/sql.mk +++ b/sql.mk @@ -228,10 +228,16 @@ sql/%.sql: sql/%.sql.in pgxntool/safesed $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control sed -e 's/^@generated@$$/@generated@ VERSIONED FILE!/' $< > $@ -# Override control.mk's rule (which builds EXTENSION_VERSION_FILES straight from -# cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with -# version-conditional substitutions applied. GNU Make's "overriding recipe" -# warning for this target is expected. +# This target already has a recipe: root control.mk (auto-generated by +# pgxntool from cat_tools.control, see pgxntool/base.mk) defines it as +# `cat sql/cat_tools.sql` with a generic one-line marker, no .sql.in layer +# and no _apply_version_seds. Since that's an explicit (non-pattern) rule +# for this exact target, GNU Make would use it over the sql/%.sql pattern +# rule above regardless of include order -- an explicit rule always beats +# a pattern rule for the same target. Overriding it here is the only way to +# route this target through the .sql.in / version-sed pipeline the rest of +# this file builds. GNU Make's "overriding recipe" warning for this target +# is expected. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed (echo @generated@ && cat $< && echo @generated@) | sed -e 's#^@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) From d6194a70eadc74f39dabeb7ccc075166b1d62011 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 26 Jul 2026 13:28:52 -0500 Subject: [PATCH 09/11] sql.mk: avoid "--" right after quoting the literal SQL comment marker That line already quotes "-- GENERATED FILE! DO NOT EDIT!"; using this file's usual "--" prose separator right after it read as if still talking about the SQL comment marker. Co-Authored-By: Claude Sonnet 5 --- sql.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql.mk b/sql.mk index 840a8bb..a3bfe0e 100644 --- a/sql.mk +++ b/sql.mk @@ -203,7 +203,7 @@ endef # TODO: refactor the version handling into a function. # # @generated@ is what actually becomes the "-- GENERATED FILE! DO NOT EDIT!" -# marker below -- it's matched anchored to the START of the line only (not +# marker below; it's matched anchored to the START of the line only (not # the whole line), so any trailing text after it survives untouched. That's # what lets the copy rule below tag a version file's own @generated@ markers # with a trailing "VERSIONED FILE!" (see there): this substitution doesn't From 461f3b7c4159acf50c7a3eb3b02ccf35e9951962 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 26 Jul 2026 13:30:26 -0500 Subject: [PATCH 10/11] sql.mk: drop the @generated@ anchor, accept the cosmetic fallout Anchoring only avoided appending marker/tag text onto the handful of @generated@ occurrences that coincidentally appear as trailing comments on real code lines in the borrowed pg_all_foreign_keys view. That's harmless (they're already inside a -- comment) and not worth the extra rule complexity -- plain substring match everywhere is simpler. Regenerated sql/cat_tools--0.3.0.sql.in to match. Co-Authored-By: Claude Sonnet 5 --- sql.mk | 26 ++++++++++++-------------- sql/cat_tools--0.3.0.sql.in | 10 +++++----- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/sql.mk b/sql.mk index a3bfe0e..99db1b6 100644 --- a/sql.mk +++ b/sql.mk @@ -203,17 +203,16 @@ endef # TODO: refactor the version handling into a function. # # @generated@ is what actually becomes the "-- GENERATED FILE! DO NOT EDIT!" -# marker below; it's matched anchored to the START of the line only (not -# the whole line), so any trailing text after it survives untouched. That's -# what lets the copy rule below tag a version file's own @generated@ markers -# with a trailing "VERSIONED FILE!" (see there): this substitution doesn't -# need to know or care whether that tag is present, it just resolves the -# @generated@ prefix either way. The start anchor also means an @generated@ -# that coincidentally appears as a trailing annotation on a real code line -# (there are a few in cat_tools.sql.in) is left untouched instead of being -# clobbered into generic marker text -- it's never at the start of its line. +# marker below. Not anchored -- a plain substring match -- so it also fires +# on the handful of @generated@ occurrences that coincidentally appear as +# trailing annotations on real code lines in cat_tools.sql.in, turning them +# into harmless (if odd-looking) extra comment text; that's fine, they're +# already inside a -- comment. That's what lets the copy rule below tag a +# version file's own @generated@ markers with a trailing "VERSIONED FILE!" +# (see there): this substitution doesn't need to know or care whether that +# tag is present, it just resolves the @generated@ prefix either way. sql/%.sql: sql/%.sql.in pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#^@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ @@ -222,11 +221,10 @@ sql/%.sql: sql/%.sql.in pgxntool/safesed # rule above resolves the leading @generated@ regardless, so appending # " VERSIONED FILE!" survives into the final marker text, making it obvious # at a glance that the built .sql is a frozen snapshot that must never be -# hand-edited. Anchored to the START of the line so a coincidental inline -# @generated@ on a real code line (not at the start) is left untouched. +# hand-edited. # (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control - sed -e 's/^@generated@$$/@generated@ VERSIONED FILE!/' $< > $@ + sed -e 's/@generated@/@generated@ VERSIONED FILE!/' $< > $@ # This target already has a recipe: root control.mk (auto-generated by # pgxntool from cat_tools.control, see pgxntool/base.mk) defines it as @@ -239,6 +237,6 @@ $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control # this file builds. GNU Make's "overriding recipe" warning for this target # is expected. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#^@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ diff --git a/sql/cat_tools--0.3.0.sql.in b/sql/cat_tools--0.3.0.sql.in index c817f89..1603d89 100644 --- a/sql/cat_tools--0.3.0.sql.in +++ b/sql/cat_tools--0.3.0.sql.in @@ -1301,7 +1301,7 @@ AS WHEN 'u' THEN 'NONE' else null END AS match_type, - CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ + CASE k1.confdeltype WHEN 'a' THEN 'NO ACTION' -- @generated@ VERSIONED FILE! WHEN 'c' THEN 'CASCADE' WHEN 'd' THEN 'SET DEFAULT' WHEN 'n' THEN 'SET NULL' @@ -1315,7 +1315,7 @@ AS WHEN 'r' THEN 'RESTRICT' ELSE NULL END AS on_update, - k1.condeferrable AS is_deferrable, -- @generated@ + k1.condeferrable AS is_deferrable, -- @generated@ VERSIONED FILE! k1.condeferred AS is_deferred FROM pg_catalog.pg_constraint k1 JOIN pg_catalog.pg_namespace n1 ON (n1.oid = k1.connamespace) @@ -1323,7 +1323,7 @@ AS JOIN pg_catalog.pg_class c2 ON (c2.oid = k1.confrelid) JOIN pg_catalog.pg_namespace n2 ON (n2.oid = c2.relnamespace) JOIN pg_catalog.pg_depend d ON ( - d.classid = 'pg_constraint'::pg_catalog.regclass -- @generated@ + d.classid = 'pg_constraint'::pg_catalog.regclass -- @generated@ VERSIONED FILE! AND d.objid = k1.oid AND d.objsubid = 0 AND d.deptype = 'n' @@ -1332,14 +1332,14 @@ AS ) JOIN pg_catalog.pg_class ci ON (ci.oid = d.refobjid AND ci.relkind = 'i') LEFT JOIN pg_depend d2 ON ( - d2.classid = 'pg_class'::pg_catalog.regclass -- @generated@ + d2.classid = 'pg_class'::pg_catalog.regclass -- @generated@ VERSIONED FILE! AND d2.objid = ci.oid AND d2.objsubid = 0 AND d2.deptype = 'i' AND d2.refclassid = 'pg_constraint'::pg_catalog.regclass AND d2.refobjsubid = 0 ) - LEFT JOIN pg_catalog.pg_constraint k2 ON ( -- @generated@ + LEFT JOIN pg_catalog.pg_constraint k2 ON ( -- @generated@ VERSIONED FILE! k2.oid = d2.refobjid AND k2.contype IN ('p', 'u') ) From 7d80c0a3d6ab83e8d199ad70311d526b5adabdfe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 26 Jul 2026 13:34:37 -0500 Subject: [PATCH 11/11] sql.mk: add an overview comment for the two .sql.in build chains One comment upfront laying out how sql/cat_tools.sql.in feeds both sql/cat_tools.sql and (via the committed per-version .sql.in snapshot) sql/cat_tools--X.Y.Z.sql, so the per-rule comments no longer each need to re-explain the whole relationship. Co-Authored-By: Claude Sonnet 5 --- sql.mk | 64 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/sql.mk b/sql.mk index 99db1b6..9ac4483 100644 --- a/sql.mk +++ b/sql.mk @@ -199,43 +199,51 @@ define _apply_version_seds {print}' $@.tmp > $@.tmp2 && mv $@.tmp2 $@.tmp endef -# The recipe builds $@.tmp then moves it into place atomically. -# TODO: refactor the version handling into a function. +# ---------------------------------------------------------------------------- +# Two .sql.in sources feed the same .sql.in -> .sql transform below (wrap with +# @generated@ markers, then _apply_version_seds), producing two different +# targets: +# +# sql/cat_tools.sql.in (hand-maintained master, committed) +# --pattern rule--> sql/cat_tools.sql +# +# sql/cat_tools.sql.in +# --copy rule, tags @generated@ as "VERSIONED FILE!"--> +# sql/cat_tools--X.Y.Z.sql.in (committed per-version snapshot; frozen +# once released, see CLAUDE.md's SQL file conventions) +# --override rule, same transform as the pattern rule--> +# sql/cat_tools--X.Y.Z.sql = $(EXTENSION_VERSION_FILES) +# (what CREATE EXTENSION actually installs) +# +# The bottom (override) rule can't just be left to the sql/%.sql pattern rule +# matching it: control.mk (auto-generated by pgxntool from cat_tools.control, +# see pgxntool/base.mk) already defines its OWN recipe for +# $(EXTENSION_VERSION_FILES) -- straight from cat_tools.sql, no .sql.in layer, +# no version seds -- and GNU Make always prefers an explicit rule over a +# pattern rule for the same target. Overriding it here is the only way to +# route that target through the same .sql.in / version-sed pipeline as +# everything else; its "overriding recipe" warning is expected. Both final +# steps build $@.tmp then move it into place atomically. # -# @generated@ is what actually becomes the "-- GENERATED FILE! DO NOT EDIT!" -# marker below. Not anchored -- a plain substring match -- so it also fires -# on the handful of @generated@ occurrences that coincidentally appear as -# trailing annotations on real code lines in cat_tools.sql.in, turning them -# into harmless (if odd-looking) extra comment text; that's fine, they're -# already inside a -- comment. That's what lets the copy rule below tag a -# version file's own @generated@ markers with a trailing "VERSIONED FILE!" -# (see there): this substitution doesn't need to know or care whether that -# tag is present, it just resolves the @generated@ prefix either way. +# TODO: refactor the version handling into a function. +# ---------------------------------------------------------------------------- + +# @generated@ becomes the "-- GENERATED FILE! DO NOT EDIT!" marker below via a +# plain, unanchored substring match -- it also fires on a handful of +# coincidental @generated@ occurrences inside real-code comments in +# cat_tools.sql.in, which is harmless (already inside a -- comment). sql/%.sql: sql/%.sql.in pgxntool/safesed (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds) mv $@.tmp $@ -# Make the current version's .sql.in by copying the base source, tagging its -# @generated@ divider markers as version-specific along the way: the pattern -# rule above resolves the leading @generated@ regardless, so appending -# " VERSIONED FILE!" survives into the final marker text, making it obvious -# at a glance that the built .sql is a frozen snapshot that must never be -# hand-edited. -# (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) +# Appends " VERSIONED FILE!" after every @generated@ occurrence; the pattern +# rule above resolves the leading @generated@ either way, so the tag rides +# through into the final marker text untouched. $(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control sed -e 's/@generated@/@generated@ VERSIONED FILE!/' $< > $@ -# This target already has a recipe: root control.mk (auto-generated by -# pgxntool from cat_tools.control, see pgxntool/base.mk) defines it as -# `cat sql/cat_tools.sql` with a generic one-line marker, no .sql.in layer -# and no _apply_version_seds. Since that's an explicit (non-pattern) rule -# for this exact target, GNU Make would use it over the sql/%.sql pattern -# rule above regardless of include order -- an explicit rule always beats -# a pattern rule for the same target. Overriding it here is the only way to -# route this target through the .sql.in / version-sed pipeline the rest of -# this file builds. GNU Make's "overriding recipe" warning for this target -# is expected. +# See the overview above for why this duplicates the pattern rule's recipe. $(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp $(_apply_version_seds)