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
1 change: 1 addition & 0 deletions crates/pgls_configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl PartialConfiguration {
}),
plpgsql_check: Some(PartialPlPgSqlCheckConfiguration {
enabled: Some(true),
..Default::default()
}),
db: Some(PartialDatabaseConfiguration {
connection_string: None,
Expand Down
65 changes: 63 additions & 2 deletions crates/pgls_configuration/src/plpgsql_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bpaf::Bpaf;
use pgls_configuration_macros::{Merge, Partial};
use serde::{Deserialize, Serialize};

/// The configuration for type checking.
/// The configuration for plpgsql_check.
#[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)]
#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))]
#[partial(cfg_attr(feature = "schema", derive(schemars::JsonSchema)))]
Expand All @@ -11,10 +11,71 @@ pub struct PlPgSqlCheckConfiguration {
/// if `false`, it disables the feature and pglpgsql_check won't be executed. `true` by default
#[partial(bpaf(hide))]
pub enabled: bool,

/// Stop processing at the first error. `true` by default.
#[partial(bpaf(hide))]
pub fatal_errors: bool,

/// Show warnings about attribute count mismatches, variable overlaps, unused variables, and
/// unwanted casting. `true` by default.
#[partial(bpaf(hide))]
pub other_warnings: bool,

/// Show warnings regarding missing RETURN statements, shadowed variables, dead code, and
/// unused parameters. `true` by default.
#[partial(bpaf(hide))]
pub extra_warnings: bool,

/// Flag performance issues like declared types with modifiers and implicit casts that may
/// prevent index usage. `false` by default.
#[partial(bpaf(hide))]
pub performance_warnings: bool,

/// Identify potential SQL injection vulnerabilities in dynamic statements. `false` by default.
#[partial(bpaf(hide))]
pub security_warnings: bool,

/// Detect deprecated patterns like explicit cursor name assignments in refcursor variables.
/// `false` by default.
#[partial(bpaf(hide))]
pub compatibility_warnings: bool,

/// Disable all warnings, overriding individual warning parameters. `false` by default.
#[partial(bpaf(hide))]
pub without_warnings: bool,

/// Enable all warnings, overriding individual warning parameters. `false` by default.
#[partial(bpaf(hide))]
pub all_warnings: bool,

/// Activate in-comment options embedded in function source code. `true` by default.
#[partial(bpaf(hide))]
pub use_incomment_options: bool,

/// Raise warnings when in-comment options are utilized. `false` by default.
#[partial(bpaf(hide))]
pub incomment_options_usage_warning: bool,

/// Permit variables holding constant values to be used like constants. `true` by default.
#[partial(bpaf(hide))]
pub constant_tracing: bool,
}

impl Default for PlPgSqlCheckConfiguration {
fn default() -> Self {
Self { enabled: true }
Self {
enabled: true,
fatal_errors: true,
other_warnings: true,
extra_warnings: true,
performance_warnings: false,
security_warnings: false,
compatibility_warnings: false,
without_warnings: false,
all_warnings: false,
use_incomment_options: true,
incomment_options_usage_warning: false,
constant_tracing: true,
}
}
}
73 changes: 71 additions & 2 deletions crates/pgls_plpgsql_check/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ pub struct PlPgSqlCheckParams<'a> {
pub sql: &'a str,
pub ast: &'a pgls_query::NodeEnum,
pub schema_cache: &'a pgls_schema_cache::SchemaCache,
pub fatal_errors: bool,
pub other_warnings: bool,
pub extra_warnings: bool,
pub performance_warnings: bool,
pub security_warnings: bool,
pub compatibility_warnings: bool,
pub without_warnings: bool,
pub all_warnings: bool,
pub use_incomment_options: bool,
pub incomment_options_usage_warning: bool,
pub constant_tracing: bool,
}

#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -129,6 +140,52 @@ fn build_function_identifier(
}
}

/// Build extra named parameters for plpgsql_check_function().
/// Only includes parameters that differ from plpgsql_check defaults.
fn build_extra_params(options: &PlPgSqlCheckParams<'_>) -> String {
let mut params = Vec::new();

if !options.fatal_errors {
params.push("fatal_errors := false".to_string());
}
if !options.other_warnings {
params.push("other_warnings := false".to_string());
}
if !options.extra_warnings {
params.push("extra_warnings := false".to_string());
}
if options.performance_warnings {
params.push("performance_warnings := true".to_string());
}
if options.security_warnings {
params.push("security_warnings := true".to_string());
}
if options.compatibility_warnings {
params.push("compatibility_warnings := true".to_string());
}
if options.without_warnings {
params.push("without_warnings := true".to_string());
}
if options.all_warnings {
params.push("all_warnings := true".to_string());
}
if !options.use_incomment_options {
params.push("use_incomment_options := false".to_string());
}
if options.incomment_options_usage_warning {
params.push("incomment_options_usage_warning := true".to_string());
}
if !options.constant_tracing {
params.push("constant_tracing := false".to_string());
}

if params.is_empty() {
String::new()
} else {
format!(", {}", params.join(", "))
}
}

pub async fn check_plpgsql(
params: PlPgSqlCheckParams<'_>,
) -> Result<Vec<PlPgSqlCheckDiagnostic>, sqlx::Error> {
Expand Down Expand Up @@ -175,6 +232,7 @@ pub async fn check_plpgsql(
sqlx::query(&sql_with_replace).execute(&mut *tx).await?;

// run plpgsql_check and collect results with their relations
let extra = build_extra_params(&params);
let results_with_relations: Vec<(String, Option<String>)> = if is_trigger {
let mut results = Vec::new();

Expand All @@ -185,7 +243,7 @@ pub async fn check_plpgsql(
let relation = format!("{}.{}", trigger.table_schema, trigger.table_name);

let result: Option<String> = sqlx::query_scalar(&format!(
"select plpgsql_check_function('{fn_identifier}', '{relation}', format := 'json')"
"select plpgsql_check_function('{fn_identifier}', '{relation}', format := 'json'{extra})"
))
.fetch_optional(&mut *tx)
.await?
Expand All @@ -200,7 +258,7 @@ pub async fn check_plpgsql(
results
} else {
let result: Option<String> = sqlx::query_scalar(&format!(
"select plpgsql_check_function('{fn_identifier}', format := 'json')"
"select plpgsql_check_function('{fn_identifier}', format := 'json'{extra})"
))
.fetch_optional(&mut *tx)
.await?
Expand Down Expand Up @@ -253,6 +311,17 @@ mod tests {
sql: create_fn_sql,
ast: &ast,
schema_cache: &schema_cache,
fatal_errors: true,
other_warnings: true,
extra_warnings: true,
performance_warnings: false,
security_warnings: false,
compatibility_warnings: false,
without_warnings: false,
all_warnings: false,
use_incomment_options: true,
incomment_options_usage_warning: false,
constant_tracing: true,
})
.await?;

Expand Down
40 changes: 37 additions & 3 deletions crates/pgls_workspace/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,17 @@ fn to_typecheck_settings(conf: TypecheckConfiguration) -> TypecheckSettings {
fn to_plpgsql_check_settings(conf: PlPgSqlCheckConfiguration) -> PlPgSqlCheckSettings {
PlPgSqlCheckSettings {
enabled: conf.enabled,
fatal_errors: conf.fatal_errors,
other_warnings: conf.other_warnings,
extra_warnings: conf.extra_warnings,
performance_warnings: conf.performance_warnings,
security_warnings: conf.security_warnings,
compatibility_warnings: conf.compatibility_warnings,
without_warnings: conf.without_warnings,
all_warnings: conf.all_warnings,
use_incomment_options: conf.use_incomment_options,
incomment_options_usage_warning: conf.incomment_options_usage_warning,
constant_tracing: conf.constant_tracing,
}
}

Expand Down Expand Up @@ -613,16 +624,39 @@ impl Default for PglinterSettings {
}
}

/// Type checking settings for the entire workspace
/// plpgsql_check settings for the entire workspace
#[derive(Debug)]
pub struct PlPgSqlCheckSettings {
/// Enabled by default
pub enabled: bool,
pub fatal_errors: bool,
pub other_warnings: bool,
pub extra_warnings: bool,
pub performance_warnings: bool,
pub security_warnings: bool,
pub compatibility_warnings: bool,
pub without_warnings: bool,
pub all_warnings: bool,
pub use_incomment_options: bool,
pub incomment_options_usage_warning: bool,
pub constant_tracing: bool,
}

impl Default for PlPgSqlCheckSettings {
fn default() -> Self {
Self { enabled: true }
Self {
enabled: true,
fatal_errors: true,
other_warnings: true,
extra_warnings: true,
performance_warnings: false,
security_warnings: false,
compatibility_warnings: false,
without_warnings: false,
all_warnings: false,
use_incomment_options: true,
incomment_options_usage_warning: false,
constant_tracing: true,
}
}
}

Expand Down
24 changes: 24 additions & 0 deletions crates/pgls_workspace/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,19 @@ impl Workspace for WorkspaceServer {
{
let typecheck_enabled = settings.typecheck.enabled;
let plpgsql_check_enabled = settings.plpgsql_check.enabled;
let plpgsql_check_fatal_errors = settings.plpgsql_check.fatal_errors;
let plpgsql_check_other_warnings = settings.plpgsql_check.other_warnings;
let plpgsql_check_extra_warnings = settings.plpgsql_check.extra_warnings;
let plpgsql_check_performance_warnings = settings.plpgsql_check.performance_warnings;
let plpgsql_check_security_warnings = settings.plpgsql_check.security_warnings;
let plpgsql_check_compatibility_warnings =
settings.plpgsql_check.compatibility_warnings;
let plpgsql_check_without_warnings = settings.plpgsql_check.without_warnings;
let plpgsql_check_all_warnings = settings.plpgsql_check.all_warnings;
let plpgsql_check_use_incomment_options = settings.plpgsql_check.use_incomment_options;
let plpgsql_check_incomment_options_usage_warning =
settings.plpgsql_check.incomment_options_usage_warning;
let plpgsql_check_constant_tracing = settings.plpgsql_check.constant_tracing;
if (typecheck_enabled || plpgsql_check_enabled)
&& let Some(pool) = self.get_current_connection()
{
Expand Down Expand Up @@ -632,6 +645,17 @@ impl Workspace for WorkspaceServer {
sql: id.content(),
ast: &ast,
schema_cache: schema_cache.as_ref(),
fatal_errors: plpgsql_check_fatal_errors,
other_warnings: plpgsql_check_other_warnings,
extra_warnings: plpgsql_check_extra_warnings,
performance_warnings: plpgsql_check_performance_warnings,
security_warnings: plpgsql_check_security_warnings,
compatibility_warnings: plpgsql_check_compatibility_warnings,
without_warnings: plpgsql_check_without_warnings,
all_warnings: plpgsql_check_all_warnings,
use_incomment_options: plpgsql_check_use_incomment_options,
incomment_options_usage_warning: plpgsql_check_incomment_options_usage_warning,
constant_tracing: plpgsql_check_constant_tracing,
},
)
.await
Expand Down
1 change: 1 addition & 0 deletions crates/pgls_workspace/src/workspace/server.tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ async fn test_disable_plpgsql_check(test_db: PgPool) {
configuration: PartialConfiguration {
plpgsql_check: Some(PartialPlPgSqlCheckConfiguration {
enabled: Some(false),
..Default::default()
}),
..Default::default()
},
Expand Down
Loading
Loading