Skip to content

Commit f415087

Browse files
authored
refractor(splinter): allow non-supabase rules to run always (#597)
1 parent 43d262b commit f415087

21 files changed

+1736
-87
lines changed

.sqlx/query-02927e584e85871ba6f84c58e8b5e4454b2c36eaf034657d5d2d95633fb85bdb.json

Lines changed: 74 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.sqlx/query-425fc6118e76cea42cf256b3b0f11046dc8b77d84c94314a0ed0716e5803df69.json

Lines changed: 74 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,12 @@ cargo insta review
144144

145145
## Development Notes
146146

147+
### Code Quality Guidelines
148+
**IMPORTANT**: Always run `cargo clippy --all-targets --all-features` and fix all warnings after making code changes. Clippy warnings must be resolved before committing code to maintain code quality standards.
149+
150+
### Git Commit and PR Guidelines
151+
**IMPORTANT**: NEVER add "Claude" or any AI assistant name to commit messages or pull request descriptions. Commits and PRs should appear as authored by the human developer only.
152+
147153
### Code Generation
148154
Many parser structures are generated from PostgreSQL's protobuf definitions using procedural macros in `pgls_query_macros`. Run `just gen-lint` after modifying analyzer rules or configurations.
149155

crates/pgls_diagnostics/src/serde.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ impl From<super::Location<'_>> for Location {
164164
#[serde(rename_all = "camelCase")]
165165
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
166166
#[cfg_attr(test, derive(Eq, PartialEq))]
167-
168167
struct Advices {
169168
advices: Vec<Advice>,
170169
}

crates/pgls_diagnostics_categories/src/categories.rs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,28 +48,28 @@ define_categories! {
4848
"lint/safety/transactionNesting": "https://pg-language-server.com/latest/rules/transaction-nesting",
4949
// end lint rules
5050
// splinter rules start
51-
"splinter/performance/authRlsInitplan": "https://supabase.com/docs/guides/database/database-linter?lint=0003_auth_rls_initplan",
52-
"splinter/performance/duplicateIndex": "https://supabase.com/docs/guides/database/database-linter?lint=0009_duplicate_index",
53-
"splinter/performance/multiplePermissivePolicies": "https://supabase.com/docs/guides/database/database-linter?lint=0006_multiple_permissive_policies",
54-
"splinter/performance/noPrimaryKey": "https://supabase.com/docs/guides/database/database-linter?lint=0004_no_primary_key",
55-
"splinter/performance/tableBloat": "https://supabase.com/docs/guides/database/database-linter?lint=0020_table_bloat",
56-
"splinter/performance/unindexedForeignKeys": "https://supabase.com/docs/guides/database/database-linter?lint=0001_unindexed_foreign_keys",
57-
"splinter/performance/unusedIndex": "https://supabase.com/docs/guides/database/database-linter?lint=0005_unused_index",
58-
"splinter/security/authUsersExposed": "https://supabase.com/docs/guides/database/database-linter?lint=0002_auth_users_exposed",
59-
"splinter/security/extensionInPublic": "https://supabase.com/docs/guides/database/database-linter?lint=0014_extension_in_public",
60-
"splinter/security/extensionVersionsOutdated": "https://supabase.com/docs/guides/database/database-linter?lint=0022_extension_versions_outdated",
61-
"splinter/security/fkeyToAuthUnique": "https://supabase.com/docs/guides/database/database-linter?lint=0021_fkey_to_auth_unique",
62-
"splinter/security/foreignTableInApi": "https://supabase.com/docs/guides/database/database-linter?lint=0017_foreign_table_in_api",
63-
"splinter/security/functionSearchPathMutable": "https://supabase.com/docs/guides/database/database-linter?lint=0011_function_search_path_mutable",
64-
"splinter/security/insecureQueueExposedInApi": "https://supabase.com/docs/guides/database/database-linter?lint=0019_insecure_queue_exposed_in_api",
65-
"splinter/security/materializedViewInApi": "https://supabase.com/docs/guides/database/database-linter?lint=0016_materialized_view_in_api",
66-
"splinter/security/policyExistsRlsDisabled": "https://supabase.com/docs/guides/database/database-linter?lint=0007_policy_exists_rls_disabled",
67-
"splinter/security/rlsDisabledInPublic": "https://supabase.com/docs/guides/database/database-linter?lint=0013_rls_disabled_in_public",
68-
"splinter/security/rlsEnabledNoPolicy": "https://supabase.com/docs/guides/database/database-linter?lint=0008_rls_enabled_no_policy",
69-
"splinter/security/rlsReferencesUserMetadata": "https://supabase.com/docs/guides/database/database-linter?lint=0015_rls_references_user_metadata",
70-
"splinter/security/securityDefinerView": "https://supabase.com/docs/guides/database/database-linter?lint=0010_security_definer_view",
71-
"splinter/security/unsupportedRegTypes": "https://supabase.com/docs/guides/database/database-linter?lint=unsupported_reg_types",
72-
"splinter/unknown/unknown": "https://pg-language-server.com/latest",
51+
"splinter/performance/authRlsInitplan": "https://supabase.com/docs/guides/database/database-advisors?lint=0003_auth_rls_initplan",
52+
"splinter/performance/duplicateIndex": "https://supabase.com/docs/guides/database/database-advisors?lint=0009_duplicate_index",
53+
"splinter/performance/multiplePermissivePolicies": "https://supabase.com/docs/guides/database/database-advisors?lint=0006_multiple_permissive_policies",
54+
"splinter/performance/noPrimaryKey": "https://supabase.com/docs/guides/database/database-advisors?lint=0004_no_primary_key",
55+
"splinter/performance/tableBloat": "https://supabase.com/docs/guides/database/database-advisors",
56+
"splinter/performance/unindexedForeignKeys": "https://supabase.com/docs/guides/database/database-advisors?lint=0001_unindexed_foreign_keys",
57+
"splinter/performance/unusedIndex": "https://supabase.com/docs/guides/database/database-advisors?lint=0005_unused_index",
58+
"splinter/security/authUsersExposed": "https://supabase.com/docs/guides/database/database-advisors?lint=0002_auth_users_exposed",
59+
"splinter/security/extensionInPublic": "https://supabase.com/docs/guides/database/database-advisors?lint=0014_extension_in_public",
60+
"splinter/security/extensionVersionsOutdated": "https://supabase.com/docs/guides/database/database-advisors?lint=0022_extension_versions_outdated",
61+
"splinter/security/fkeyToAuthUnique": "https://supabase.com/docs/guides/database/database-advisors",
62+
"splinter/security/foreignTableInApi": "https://supabase.com/docs/guides/database/database-advisors?lint=0017_foreign_table_in_api",
63+
"splinter/security/functionSearchPathMutable": "https://supabase.com/docs/guides/database/database-advisors?lint=0011_function_search_path_mutable",
64+
"splinter/security/insecureQueueExposedInApi": "https://supabase.com/docs/guides/database/database-advisors?lint=0019_insecure_queue_exposed_in_api",
65+
"splinter/security/materializedViewInApi": "https://supabase.com/docs/guides/database/database-advisors?lint=0016_materialized_view_in_api",
66+
"splinter/security/policyExistsRlsDisabled": "https://supabase.com/docs/guides/database/database-advisors?lint=0007_policy_exists_rls_disabled",
67+
"splinter/security/rlsDisabledInPublic": "https://supabase.com/docs/guides/database/database-advisors?lint=0013_rls_disabled_in_public",
68+
"splinter/security/rlsEnabledNoPolicy": "https://supabase.com/docs/guides/database/database-advisors?lint=0008_rls_enabled_no_policy",
69+
"splinter/security/rlsReferencesUserMetadata": "https://supabase.com/docs/guides/database/database-advisors?lint=0015_rls_references_user_metadata",
70+
"splinter/security/securityDefinerView": "https://supabase.com/docs/guides/database/database-advisors?lint=0010_security_definer_view",
71+
"splinter/security/unsupportedRegTypes": "https://supabase.com/docs/guides/database/database-advisors?lint=unsupported_reg_types",
72+
"splinter/unknown/unknown": "https://supabase.com/docs/guides/database/database-advisors",
7373
// splinter rules end
7474
;
7575
// General categories

crates/pgls_splinter/.sqlx/query-02927e584e85871ba6f84c58e8b5e4454b2c36eaf034657d5d2d95633fb85bdb.json

Lines changed: 74 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/pgls_splinter/.sqlx/query-425fc6118e76cea42cf256b3b0f11046dc8b77d84c94314a0ed0716e5803df69.json

Lines changed: 74 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/pgls_splinter/build.rs

Lines changed: 121 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,40 @@ use std::path::Path;
55
// Update this commit SHA to pull in a new version of splinter.sql
66
const SPLINTER_COMMIT_SHA: &str = "27ea2ece65464213e466cd969cc61b6940d16219";
77

8+
// Rules that work on any PostgreSQL database
9+
const GENERIC_RULES: &[&str] = &[
10+
"unindexed_foreign_keys",
11+
"no_primary_key",
12+
"unused_index",
13+
"multiple_permissive_policies",
14+
"policy_exists_rls_disabled",
15+
"rls_enabled_no_policy",
16+
"duplicate_index",
17+
"extension_in_public",
18+
"table_bloat",
19+
"extension_versions_outdated",
20+
"function_search_path_mutable",
21+
"unsupported_reg_types",
22+
];
23+
24+
// Rules that require Supabase-specific infrastructure (auth schema, anon/authenticated roles, pgrst.db_schemas)
25+
const SUPABASE_ONLY_RULES: &[&str] = &[
26+
"auth_users_exposed",
27+
"auth_rls_initplan",
28+
"rls_disabled_in_public",
29+
"security_definer_view",
30+
"rls_references_user_metadata",
31+
"materialized_view_in_api",
32+
"foreign_table_in_api",
33+
"insecure_queue_exposed_in_api",
34+
"fkey_to_auth_unique",
35+
];
36+
837
fn main() {
938
let out_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
1039
let vendor_dir = Path::new(&out_dir).join("vendor");
11-
let sql_file = vendor_dir.join("splinter.sql");
40+
let generic_sql_file = vendor_dir.join("splinter_generic.sql");
41+
let supabase_sql_file = vendor_dir.join("splinter_supabase.sql");
1242
let sha_file = vendor_dir.join("COMMIT_SHA.txt");
1343

1444
// Create vendor directory if it doesn't exist
@@ -17,22 +47,23 @@ fn main() {
1747
}
1848

1949
// Check if we need to download
20-
let needs_download = if !sql_file.exists() || !sha_file.exists() {
21-
true
22-
} else {
23-
// Check if stored SHA matches current constant
24-
let stored_sha = fs::read_to_string(&sha_file)
25-
.expect("Failed to read COMMIT_SHA.txt")
26-
.trim()
27-
.to_string();
28-
stored_sha != SPLINTER_COMMIT_SHA
29-
};
50+
let needs_download =
51+
if !generic_sql_file.exists() || !supabase_sql_file.exists() || !sha_file.exists() {
52+
true
53+
} else {
54+
// Check if stored SHA matches current constant
55+
let stored_sha = fs::read_to_string(&sha_file)
56+
.expect("Failed to read COMMIT_SHA.txt")
57+
.trim()
58+
.to_string();
59+
stored_sha != SPLINTER_COMMIT_SHA
60+
};
3061

3162
if needs_download {
3263
println!(
3364
"cargo:warning=Downloading splinter.sql from GitHub (commit: {SPLINTER_COMMIT_SHA})"
3465
);
35-
download_and_process_sql(&sql_file);
66+
download_and_process_sql(&generic_sql_file, &supabase_sql_file);
3667
fs::write(&sha_file, SPLINTER_COMMIT_SHA).expect("Failed to write COMMIT_SHA.txt");
3768
}
3869

@@ -41,7 +72,7 @@ fn main() {
4172
println!("cargo:rerun-if-changed=vendor/COMMIT_SHA.txt");
4273
}
4374

44-
fn download_and_process_sql(dest_path: &Path) {
75+
fn download_and_process_sql(generic_dest: &Path, supabase_dest: &Path) {
4576
let url = format!(
4677
"https://raw.githubusercontent.com/supabase/splinter/{SPLINTER_COMMIT_SHA}/splinter.sql"
4778
);
@@ -61,10 +92,16 @@ fn download_and_process_sql(dest_path: &Path) {
6192
// Add "!" suffix to column aliases for sqlx non-null checking
6293
processed_content = add_not_null_markers(&processed_content);
6394

64-
// Write to destination
65-
fs::write(dest_path, processed_content).expect("Failed to write splinter.sql");
95+
// Split into generic and Supabase-specific queries (validates categorization)
96+
let (generic_queries, supabase_queries) = split_queries(&processed_content);
6697

67-
println!("cargo:warning=Successfully downloaded and processed splinter.sql");
98+
// Write to destination files
99+
fs::write(generic_dest, generic_queries).expect("Failed to write splinter_generic.sql");
100+
fs::write(supabase_dest, supabase_queries).expect("Failed to write splinter_supabase.sql");
101+
102+
println!(
103+
"cargo:warning=Successfully downloaded and processed splinter.sql into generic and Supabase-specific files"
104+
);
68105
}
69106

70107
fn remove_set_search_path(content: &str) -> String {
@@ -107,3 +144,71 @@ fn add_not_null_markers(content: &str) -> String {
107144

108145
result
109146
}
147+
148+
/// Extract rule name from a query fragment
149+
fn extract_rule_name_from_query(query: &str) -> String {
150+
// Look for pattern 'rule_name' as "name!"
151+
for line in query.lines() {
152+
if line.contains(" as \"name!\"") {
153+
if let Some(start) = line.rfind('\'') {
154+
if let Some(prev_quote) = line[..start].rfind('\'') {
155+
return line[prev_quote + 1..start].to_string();
156+
}
157+
}
158+
}
159+
}
160+
"unknown".to_string()
161+
}
162+
163+
fn split_queries(content: &str) -> (String, String) {
164+
// Split the union all queries based on rule names
165+
let queries: Vec<&str> = content.split("union all").collect();
166+
167+
let mut generic_queries = Vec::new();
168+
let mut supabase_queries = Vec::new();
169+
170+
for query in queries {
171+
// Extract the rule name from the query (it's the first 'name' field)
172+
let is_supabase = SUPABASE_ONLY_RULES
173+
.iter()
174+
.any(|rule| query.contains(&format!("'{rule}' as \"name!\"")));
175+
176+
let is_generic = GENERIC_RULES
177+
.iter()
178+
.any(|rule| query.contains(&format!("'{rule}' as \"name!\"")));
179+
180+
if is_supabase {
181+
supabase_queries.push(query);
182+
} else if is_generic {
183+
generic_queries.push(query);
184+
} else {
185+
// Extract rule name for better error message
186+
let rule_name = extract_rule_name_from_query(query);
187+
panic!(
188+
"Found unknown Splinter rule that is not categorized: {rule_name:?}\n\
189+
Please add this rule to either GENERIC_RULES or SUPABASE_ONLY_RULES in build.rs.\n\
190+
\n\
191+
Guidelines:\n\
192+
- GENERIC_RULES: Rules that work on any PostgreSQL database\n\
193+
- SUPABASE_ONLY_RULES: Rules that require Supabase infrastructure (auth schema, roles, pgrst.db_schemas)\n\
194+
\n\
195+
This prevents new Supabase-specific rules from breaking linting on non-Supabase databases."
196+
);
197+
}
198+
}
199+
200+
// Join queries with "union all" and wrap in parentheses
201+
let generic_sql = if generic_queries.is_empty() {
202+
String::new()
203+
} else {
204+
generic_queries.join("union all\n")
205+
};
206+
207+
let supabase_sql = if supabase_queries.is_empty() {
208+
String::new()
209+
} else {
210+
supabase_queries.join("union all\n")
211+
};
212+
213+
(generic_sql, supabase_sql)
214+
}

crates/pgls_splinter/src/convert.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ impl From<SplinterQueryResult> for SplinterDiagnostic {
2727
schema,
2828
object_name,
2929
object_type,
30-
remediation_url: result.remediation,
30+
remediation: result.remediation,
3131
additional_metadata,
3232
},
3333
}
@@ -47,6 +47,7 @@ fn parse_severity(level: &str) -> Severity {
4747
/// Convert rule name and group to a Category
4848
/// Note: Rule names use snake_case, but categories use camelCase
4949
fn rule_name_to_category(name: &str, group: &str) -> &'static Category {
50+
// we cannot use convert_case here because category! macro requires a string literal
5051
match (group, name) {
5152
("performance", "unindexed_foreign_keys") => {
5253
category!("splinter/performance/unindexedForeignKeys")

crates/pgls_splinter/src/diagnostics.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct SplinterAdvices {
4141
pub object_type: Option<String>,
4242

4343
/// URL to documentation/remediation guide
44-
pub remediation_url: String,
44+
pub remediation: String,
4545

4646
/// Additional rule-specific metadata (e.g., fkey_name, column, indexes)
4747
/// This contains fields that don't fit into the common structure
@@ -70,10 +70,10 @@ impl Advices for SplinterAdvices {
7070
}
7171
}
7272

73-
// Show remediation URL
73+
// Show remediation
7474
visitor.record_log(
7575
LogCategory::Info,
76-
&format!("Documentation: {}", &self.remediation_url),
76+
&format!("Remediation: {}", &self.remediation),
7777
)?;
7878

7979
Ok(())

0 commit comments

Comments
 (0)