Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -234,6 +241,7 @@ public static List<VisitorCheck> allChecks(){
new PreferIdentityVisitorCheck(),
new DisallowedDoVisitorCheck(),
new OneMigrationPerFileVisitorCheck(),
new OnlySchemaMigrationsVisitorCheck());
new OnlySchemaMigrationsVisitorCheck(),
new RequireEnumValueOrderingCheck());
}
}
Original file line number Diff line number Diff line change
@@ -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);

Check warning on line 15 in src/main/java/com/premiumminds/sonar/postgres/visitors/RequireEnumValueOrderingCheck.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this use of System.out by a logger.

See more on https://sonarcloud.io/project/issues?id=premium-minds_sonar-postgres-plugin&issues=AZ0szKodbExGPQYBjnfi&open=AZ0szKodbExGPQYBjnfi&pullRequest=51
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;
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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<RuleKey, Map<String, Issue>> issueMap = groupByRuleAndFile(contextTester.allIssues());

final Map<String, Issue> 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();

Expand Down
Loading