From f9344094efca8a6d1cfa5d829788812c113bd593 Mon Sep 17 00:00:00 2001 From: Filipe Roque Date: Fri, 27 Mar 2026 00:57:05 +0000 Subject: [PATCH] Adds rule require-enum-value-ordering --- .../postgres/PostgresSqlQualityProfile.java | 2 + .../postgres/PostgresSqlRulesDefinition.java | 10 +++- .../RequireEnumValueOrderingCheck.java | 27 +++++++++++ .../postgres/require-enum-value-ordering.md | 48 +++++++++++++++++++ .../sonar/postgres/PostgresSqlSensorTest.java | 27 ++++++++++- 5 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/premiumminds/sonar/postgres/visitors/RequireEnumValueOrderingCheck.java create mode 100644 src/main/resources/com/premiumminds/sonar/postgres/require-enum-value-ordering.md diff --git a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java index 71f8c0a..5e715b3 100644 --- a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java +++ b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java @@ -28,6 +28,7 @@ import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_PREFER_TEXT_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_RENAMING_COLUMN; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_RENAMING_TABLE; +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_REQUIRE_ENUM_VALUE_ORDERING; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_SETTING_NOT_NULLABLE_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_VACUUM_FULL; @@ -64,6 +65,7 @@ public void define(Context context) { activateRule(profile, RULE_DISALLOWED_DO); activateRule(profile, RULE_ONLY_SCHEMA_MIGRATIONS); activateRule(profile, RULE_ONLY_LOWER_CASE_NAMES); + activateRule(profile, RULE_REQUIRE_ENUM_VALUE_ORDERING); profile.done(); } diff --git a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java index 8287e0d..3b5095d 100644 --- a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java +++ b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java @@ -24,6 +24,7 @@ import com.premiumminds.sonar.postgres.visitors.PreferTextFieldVisitorCheck; import com.premiumminds.sonar.postgres.visitors.RenameColumnVisitorCheck; import com.premiumminds.sonar.postgres.visitors.RenameTableVisitorCheck; +import com.premiumminds.sonar.postgres.visitors.RequireEnumValueOrderingCheck; import com.premiumminds.sonar.postgres.visitors.RobustStatementsVisitorCheck; import com.premiumminds.sonar.postgres.visitors.SettingNotNullVisitorCheck; import com.premiumminds.sonar.postgres.visitors.VacuumFullVisitorCheck; @@ -64,6 +65,7 @@ public class PostgresSqlRulesDefinition implements RulesDefinition { public static final RuleKey RULE_DISALLOWED_DO = RuleKey.of(REPOSITORY, "disallowed-do"); public static final RuleKey RULE_ONLY_SCHEMA_MIGRATIONS = RuleKey.of(REPOSITORY, "only-schema-migrations"); public static final RuleKey RULE_ONLY_LOWER_CASE_NAMES = RuleKey.of(REPOSITORY, "only-lower-case-names"); + public static final RuleKey RULE_REQUIRE_ENUM_VALUE_ORDERING = RuleKey.of(REPOSITORY, "require-enum-value-ordering"); @Override public void define(Context context) { @@ -206,6 +208,11 @@ public void define(Context context) { .setType(RuleType.BUG) .setMarkdownDescription(getClass().getResource("only-lower-case-names.md")); + repository.createRule(RULE_REQUIRE_ENUM_VALUE_ORDERING.rule()) + .setName("require-enum-value-ordering rule") + .setType(RuleType.BUG) + .setMarkdownDescription(getClass().getResource("require-enum-value-ordering.md")); + repository.done(); } @@ -234,6 +241,7 @@ public static List allChecks(){ new PreferIdentityVisitorCheck(), new DisallowedDoVisitorCheck(), new OneMigrationPerFileVisitorCheck(), - new OnlySchemaMigrationsVisitorCheck()); + new OnlySchemaMigrationsVisitorCheck(), + new RequireEnumValueOrderingCheck()); } } diff --git a/src/main/java/com/premiumminds/sonar/postgres/visitors/RequireEnumValueOrderingCheck.java b/src/main/java/com/premiumminds/sonar/postgres/visitors/RequireEnumValueOrderingCheck.java new file mode 100644 index 0000000..609fb8e --- /dev/null +++ b/src/main/java/com/premiumminds/sonar/postgres/visitors/RequireEnumValueOrderingCheck.java @@ -0,0 +1,27 @@ +package com.premiumminds.sonar.postgres.visitors; + +import com.premiumminds.sonar.postgres.protobuf.AlterEnumStmt; +import org.sonar.api.rule.RuleKey; +import org.sonar.check.Rule; + +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_REQUIRE_ENUM_VALUE_ORDERING; + +@Rule(key = "require-enum-value-ordering") +public class RequireEnumValueOrderingCheck extends AbstractVisitorCheck { + + @Override + public void visit(AlterEnumStmt alterEnumStmt) { + + System.out.println(alterEnumStmt); + if (alterEnumStmt.getNewValNeighbor() == null || alterEnumStmt.getNewValNeighbor().isBlank()){ + newIssue("ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering."); + } + + super.visit(alterEnumStmt); + } + + @Override + protected RuleKey getRule() { + return RULE_REQUIRE_ENUM_VALUE_ORDERING; + } +} diff --git a/src/main/resources/com/premiumminds/sonar/postgres/require-enum-value-ordering.md b/src/main/resources/com/premiumminds/sonar/postgres/require-enum-value-ordering.md new file mode 100644 index 0000000..0d8cba7 --- /dev/null +++ b/src/main/resources/com/premiumminds/sonar/postgres/require-enum-value-ordering.md @@ -0,0 +1,48 @@ +== problem + +Using ``ALTER TYPE ... ADD VALUE`` without specifying ``BEFORE`` or ``AFTER`` appends the new value to the end of the enum type. + +This can lead to unexpected ordering, especially when enum values have a meaningful order (e.g., workflow status, severity, priority). + +``sql +-- existing type +CREATE TYPE status AS ENUM ( + 'draft', + 'submitted', + 'approved', + 'rejected' +); +-- existing query to find all not yet approved +select * from papers where status < 'approved'; + +-- bad +ALTER TYPE status ADD VALUE 'pending_review'; + +-- now our query isn't including any of the 'pending_review' items! +`` + +== solution + +Explicitly specify where the new value should be inserted using ``BEFORE`` or ``AFTER``. + +``sql +-- existing type +CREATE TYPE status AS ENUM ( + 'draft', + 'submitted', + 'approved', + 'rejected' +); + +-- existing query to find all not yet approved +select * from papers where status < 'approved'; + +-- good +ALTER TYPE status ADD VALUE 'pending_review' BEFORE 'approved'; +-- or +ALTER TYPE status ADD VALUE 'pending_review' AFTER 'submitted'; +`` + +== links + + * https://www.postgresql.org/docs/current/sql-altertype.html diff --git a/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java b/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java index 29a4d43..5df4146 100644 --- a/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java +++ b/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java @@ -45,6 +45,7 @@ import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_PREFER_TEXT_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_RENAMING_COLUMN; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_RENAMING_TABLE; +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_REQUIRE_ENUM_VALUE_ORDERING; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_SETTING_NOT_NULLABLE_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_VACUUM_FULL; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -113,7 +114,8 @@ void testAllRules() { RULE_ONE_MIGRATION_PER_FILE, RULE_DISALLOWED_DO, RULE_ONLY_SCHEMA_MIGRATIONS, - RULE_ONLY_LOWER_CASE_NAMES + RULE_ONLY_LOWER_CASE_NAMES, + RULE_REQUIRE_ENUM_VALUE_ORDERING ); sensor.execute(contextTester); @@ -960,6 +962,29 @@ void banTruncateCascade() { assertEquals(1, fileMap.size()); } + @Test + void requireEnumValueOrdering() { + createFile(contextTester, "file1.sql", "ALTER TYPE my_enum ADD VALUE 'new_value';"); + createFile(contextTester, "file2.sql", "ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value';"); + createFile(contextTester, "file3-ok.sql", "ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';"); + createFile(contextTester, "file4-ok.sql", "ALTER TYPE my_enum ADD VALUE 'new_value' AFTER 'existing_value';"); + + final RuleKey rule = RULE_REQUIRE_ENUM_VALUE_ORDERING; + PostgresSqlSensor sensor = getPostgresSqlSensor(rule); + sensor.execute(contextTester); + + final Map> issueMap = groupByRuleAndFile(contextTester.allIssues()); + + final Map fileMap = issueMap.get(rule); + + assertEquals("ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering.", + fileMap.get(":file1.sql").primaryLocation().message()); + assertEquals("ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering.", + fileMap.get(":file2.sql").primaryLocation().message()); + + assertEquals(2, fileMap.size()); + } + private PostgresSqlSensor getPostgresSqlSensor(RuleKey... ruleKey) { ActiveRulesBuilder activeRulesBuilder = new ActiveRulesBuilder();