diff --git a/crates/pgls_configuration/src/format.rs b/crates/pgls_configuration/src/format.rs index 7855571cd..12750c781 100644 --- a/crates/pgls_configuration/src/format.rs +++ b/crates/pgls_configuration/src/format.rs @@ -70,6 +70,37 @@ impl From for pgls_pretty_print::renderer::KeywordCase { } } +/// How an explicit cast is spelled. +#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, Merge, PartialEq, Serialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "lowercase")] +pub enum CastStyle { + #[default] + Cast, + Operator, +} + +impl FromStr for CastStyle { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s { + "cast" => Ok(Self::Cast), + "operator" => Ok(Self::Operator), + _ => Err("Value not supported for CastStyle. Use 'cast' or 'operator'."), + } + } +} + +impl From for pgls_pretty_print::CastStyle { + fn from(style: CastStyle) -> Self { + match style { + CastStyle::Cast => Self::Cast, + CastStyle::Operator => Self::Operator, + } + } +} + /// The configuration for SQL formatting. #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)] #[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] @@ -97,6 +128,10 @@ pub struct FormatConfiguration { /// Data type casing (text, varchar, int): "upper" or "lower". Default: "lower". #[partial(bpaf(long("type-case")))] pub type_case: KeywordCase, + /// How an explicit cast is spelled: "cast" for `CAST(x AS t)`, "operator" for `x::t`. + /// Default: "cast". + #[partial(bpaf(long("cast-style")))] + pub cast_style: CastStyle, /// If `true`, skip formatting of SQL function bodies (keep them verbatim). Default: `false`. #[partial(bpaf(long("skip-fn-bodies")))] pub skip_fn_bodies: bool, @@ -118,6 +153,7 @@ impl Default for FormatConfiguration { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + cast_style: CastStyle::default(), skip_fn_bodies: false, ignore: Default::default(), include: Default::default(), diff --git a/crates/pgls_pretty_print/src/emitter.rs b/crates/pgls_pretty_print/src/emitter.rs index 368c79b23..19c0a6db6 100644 --- a/crates/pgls_pretty_print/src/emitter.rs +++ b/crates/pgls_pretty_print/src/emitter.rs @@ -1,3 +1,4 @@ +use crate::FormatConfig; pub use crate::codegen::group_kind::GroupKind; pub use crate::codegen::token_kind::TokenKind; @@ -22,14 +23,28 @@ pub enum LayoutEvent { IndentEnd, } -#[derive(Debug, Default)] +/// Collects layout events for the renderer. +/// +/// The emitter holds the configuration because some options decide which tokens exist at all, +/// such as where a comma sits in a list, and not merely how a token is rendered. +#[derive(Debug)] pub struct EventEmitter { pub events: Vec, + config: FormatConfig, } impl EventEmitter { - pub fn new() -> Self { - Self::default() + pub fn new(config: FormatConfig) -> Self { + Self { + events: Vec::new(), + config, + } + } + + // Later option PRs inspect this while deciding which layout events to emit. + #[allow(dead_code)] + pub fn config(&self) -> &FormatConfig { + &self.config } pub fn token(&mut self, token: TokenKind) { diff --git a/crates/pgls_pretty_print/src/lib.rs b/crates/pgls_pretty_print/src/lib.rs index 008e4faa8..999a059ef 100644 --- a/crates/pgls_pretty_print/src/lib.rs +++ b/crates/pgls_pretty_print/src/lib.rs @@ -31,6 +31,16 @@ pub enum FormatError { BetaUnsupported { message: String }, } +/// How an explicit cast is spelled. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub enum CastStyle { + /// `CAST(expr AS type)`. + #[default] + Cast, + /// `expr::type`. + Operator, +} + /// Configuration for the SQL formatter. #[derive(Debug, Clone)] pub struct FormatConfig { @@ -46,6 +56,8 @@ pub struct FormatConfig { pub constant_case: KeywordCase, /// Casing for data types (text, varchar, int). Default: Lower. pub type_case: KeywordCase, + /// How an explicit cast is spelled. Default: Cast. + pub cast_style: CastStyle, } impl Default for FormatConfig { @@ -57,6 +69,7 @@ impl Default for FormatConfig { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + cast_style: CastStyle::default(), } } } @@ -104,7 +117,7 @@ pub fn format_statement( config: &FormatConfig, ) -> Result { // Emit layout events from AST - let mut emitter = emitter::EventEmitter::new(); + let mut emitter = emitter::EventEmitter::new(config.clone()); nodes::emit_node_enum(ast, &mut emitter); // Render to string diff --git a/crates/pgls_pretty_print/src/nodes/type_cast.rs b/crates/pgls_pretty_print/src/nodes/type_cast.rs index 225adf9dc..b94465e13 100644 --- a/crates/pgls_pretty_print/src/nodes/type_cast.rs +++ b/crates/pgls_pretty_print/src/nodes/type_cast.rs @@ -1,17 +1,27 @@ use crate::{ - TokenKind, + CastStyle, TokenKind, emitter::{EventEmitter, GroupKind, LineType}, }; -use pgls_query::protobuf::TypeCast; +use pgls_query::{ + Node, NodeEnum, + protobuf::{AExprKind, TypeCast}, +}; pub(super) fn emit_type_cast(e: &mut EventEmitter, n: &TypeCast) { e.group_start(GroupKind::TypeCast); - // CAST(expr AS type) syntax + match e.config().cast_style { + CastStyle::Operator => emit_operator_cast(e, n), + CastStyle::Cast => emit_cast_call(e, n), + } + + e.group_end(); +} + +fn emit_cast_call(e: &mut EventEmitter, n: &TypeCast) { e.token(TokenKind::CAST_KW); e.token(TokenKind::L_PAREN); - // Emit the expression if let Some(ref arg) = n.arg { super::emit_node(arg, e); } @@ -20,12 +30,56 @@ pub(super) fn emit_type_cast(e: &mut EventEmitter, n: &TypeCast) { e.token(TokenKind::AS_KW); e.space(); - // Emit the type if let Some(ref type_name) = n.type_name { super::emit_type_name(e, type_name); } e.token(TokenKind::R_PAREN); +} - e.group_end(); +fn emit_operator_cast(e: &mut EventEmitter, n: &TypeCast) { + if let Some(ref arg) = n.arg { + // `::` binds tighter than every infix operator, so anything that is not a self contained + // primary expression has to be parenthesised: `a + b::int` would cast b alone. + if needs_parentheses(arg) { + e.token(TokenKind::L_PAREN); + super::emit_node(arg, e); + e.token(TokenKind::R_PAREN); + } else { + super::emit_node(arg, e); + } + } + + e.token(TokenKind::IDENT("::".to_string())); + + if let Some(ref type_name) = n.type_name { + super::emit_type_name(e, type_name); + } +} + +/// Whitelist of self contained expressions that can carry a `::` without parentheses. +/// +/// Anything absent from this list is parenthesised, which is always valid SQL. Listing the unsafe +/// kinds instead would fail open on any node nobody thought about. +fn needs_parentheses(node: &Node) -> bool { + match node.node.as_ref() { + Some(NodeEnum::AExpr(a_expr)) => a_expr.kind != AExprKind::AexprNullif as i32, + Some( + NodeEnum::AConst(_) + | NodeEnum::ColumnRef(_) + | NodeEnum::ParamRef(_) + | NodeEnum::FuncCall(_) + | NodeEnum::NullIfExpr(_) + | NodeEnum::TypeCast(_) + | NodeEnum::SubLink(_) + | NodeEnum::CaseExpr(_) + | NodeEnum::CoalesceExpr(_) + | NodeEnum::MinMaxExpr(_) + | NodeEnum::ArrayExpr(_) + | NodeEnum::RowExpr(_) + | NodeEnum::AIndirection(_) + | NodeEnum::AArrayExpr(_), + ) => false, + _ => true, + } } diff --git a/crates/pgls_pretty_print/src/renderer.rs b/crates/pgls_pretty_print/src/renderer.rs index 36dcbbff9..83e708cb6 100644 --- a/crates/pgls_pretty_print/src/renderer.rs +++ b/crates/pgls_pretty_print/src/renderer.rs @@ -325,7 +325,7 @@ mod tests { #[test] fn test_keyword_case_upper() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); @@ -342,7 +342,7 @@ mod tests { #[test] fn test_keyword_case_lower() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); @@ -359,7 +359,7 @@ mod tests { #[test] fn test_constant_case_upper() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::NULL); @@ -382,7 +382,7 @@ mod tests { #[test] fn test_constant_case_lower() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::NULL); @@ -402,7 +402,7 @@ mod tests { #[test] fn test_mixed_case_settings() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); diff --git a/crates/pgls_pretty_print/tests/data/single/cast_operator_style.sql b/crates/pgls_pretty_print/tests/data/single/cast_operator_style.sql new file mode 100644 index 000000000..13dbd976e --- /dev/null +++ b/crates/pgls_pretty_print/tests/data/single/cast_operator_style.sql @@ -0,0 +1,12 @@ +-- pgls-format: castStyle=operator +SELECT + CAST(t.id AS bigint), + CAST(t.name AS text), + CAST(t.id AS public.object_id), + CAST(nullif(t.a, '') AS date), + CAST(t.a + t.b AS int), + CAST(t.a || t.b AS text), + CAST(CAST(t.a AS text) AS bigint), + CAST((SELECT max(u.id) FROM s.u) AS bigint), + CAST(CASE WHEN t.a THEN 1 ELSE 2 END AS text) +FROM s.t; diff --git a/crates/pgls_pretty_print/tests/data/single/format_config_header.sql b/crates/pgls_pretty_print/tests/data/single/format_config_header.sql new file mode 100644 index 000000000..80810d099 --- /dev/null +++ b/crates/pgls_pretty_print/tests/data/single/format_config_header.sql @@ -0,0 +1,6 @@ +-- pgls-format: keywordCase=upper, indentStyle=tabs, indentSize=4, lineWidth=80 +SELECT + t.a, + t.b +FROM s.t +WHERE t.a > 1; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_100.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_100.snap new file mode 100644 index 000000000..ed3cc76d1 --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_100.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/cast_operator_style.sql +--- +select + t.id::bigint, + t.name::text, + t.id::public.object_id, + nullif(t.a, '')::date, + (t.a + t.b)::int, + (t.a || t.b)::text, + t.a::text::bigint, + (select MAX(u.id) from s.u)::bigint, + case when t.a then 1 else 2 end::text +from + s.t; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_80.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_80.snap new file mode 100644 index 000000000..ed3cc76d1 --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__cast_operator_style_80.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/cast_operator_style.sql +--- +select + t.id::bigint, + t.name::text, + t.id::public.object_id, + nullif(t.a, '')::date, + (t.a + t.b)::int, + (t.a || t.b)::text, + t.a::text::bigint, + (select MAX(u.id) from s.u)::bigint, + case when t.a then 1 else 2 end::text +from + s.t; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap new file mode 100644 index 000000000..6c5d4d49e --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap @@ -0,0 +1,5 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/format_config_header.sql +--- +SELECT t.a, t.b FROM s.t WHERE t.a > 1; diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index 0eb68ed4d..7a3dcd1d5 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -3,10 +3,11 @@ use dir_test::{Fixture, dir_test}; use insta::{assert_snapshot, with_settings}; use pgls_pretty_print::{ + CastStyle, FormatConfig, emitter::EventEmitter, nodes::emit_node_enum, normalize::normalize_ast, - renderer::{IndentStyle, RenderConfig, Renderer}, + renderer::{IndentStyle, KeywordCase, RenderConfig, Renderer}, }; /// Line widths to test - each test file is run at both widths @@ -37,12 +38,64 @@ enum StringState { Dollar(Vec), } +/// A fixture may open with `-- pgls-format: key=value, key=value` to declare the configuration it +/// must be rendered with. Keeping it in the fixture rather than in the harness is what lets an +/// option that is off by default own its own test data. +/// +/// It returns a `FormatConfig`, not a `RenderConfig`: some options decide which tokens exist and +/// are therefore read by the emitter, so the fixture has to reach both sides of the pipeline. +fn parse_fixture(content: &str) -> (FormatConfig, Option, String) { + const HEADER: &str = "-- pgls-format:"; + + let mut config = FormatConfig::default(); + let mut explicit_width = None; + + let Some(rest) = content.strip_prefix(HEADER) else { + return (config, None, content.to_string()); + }; + + let (header, sql) = match rest.split_once('\n') { + Some((header, sql)) => (header, sql), + None => (rest, ""), + }; + + for entry in header.split(',') { + let Some((key, value)) = entry.split_once('=') else { + panic!("malformed pgls-format entry: {entry}"); + }; + + match (key.trim(), value.trim()) { + ("lineWidth", value) => { + let width = value.parse().expect("lineWidth must be a number"); + config.line_width = width; + explicit_width = Some(width); + } + ("indentSize", value) => { + config.indent_size = value.parse().expect("indentSize must be a number"); + } + ("indentStyle", "tabs") => config.indent_style = IndentStyle::Tabs, + ("indentStyle", "spaces") => config.indent_style = IndentStyle::Spaces, + ("keywordCase", "upper") => config.keyword_case = KeywordCase::Upper, + ("keywordCase", "lower") => config.keyword_case = KeywordCase::Lower, + ("constantCase", "upper") => config.constant_case = KeywordCase::Upper, + ("constantCase", "lower") => config.constant_case = KeywordCase::Lower, + ("typeCase", "upper") => config.type_case = KeywordCase::Upper, + ("typeCase", "lower") => config.type_case = KeywordCase::Lower, + ("castStyle", "operator") => config.cast_style = CastStyle::Operator, + ("castStyle", "cast") => config.cast_style = CastStyle::Cast, + (key, value) => panic!("unknown pgls-format entry: {key}={value}"), + } + } + + (config, explicit_width, sql.to_string()) +} + #[dir_test( dir: "$CARGO_MANIFEST_DIR/tests/data/single/", glob: "*.sql", )] fn test_single(fixture: Fixture<&str>) { - let content = fixture.content(); + let (fixture_config, explicit_width, content) = parse_fixture(fixture.content()); println!("Original content:\n{content}"); @@ -53,24 +106,28 @@ fn test_single(fixture: Fixture<&str>) { .and_then(|x| x.strip_suffix(".sql")) .unwrap(); - // Run test at each configured line width - for &max_line_length in &LINE_WIDTHS { + let widths: Vec = match explicit_width { + Some(width) => vec![width], + None => LINE_WIDTHS.to_vec(), + }; + + for max_line_length in widths { let test_name = format!("{base_test_name}_{max_line_length}"); - let parsed = pgls_query::parse(content).expect("Failed to parse SQL"); + let parsed = pgls_query::parse(&content).expect("Failed to parse SQL"); let mut ast = parsed.into_root().expect("No root node found"); println!("Parsed AST: {ast:#?}"); - let mut emitter = EventEmitter::new(); + // The emitter gets the fixture config, not the default one: an option that decides which + // tokens exist is read here, before the renderer ever sees the events. + let mut emitter = EventEmitter::new(fixture_config.clone()); emit_node_enum(&ast, &mut emitter); let mut output = String::new(); let config = RenderConfig { max_line_length, - indent_size: 2, - indent_style: IndentStyle::Spaces, - ..Default::default() + ..RenderConfig::from(fixture_config.clone()) }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); @@ -119,19 +176,23 @@ fn test_multi(fixture: Fixture<&str>) { } } - let content = fixture.content(); + let (fixture_config, explicit_width, content) = parse_fixture(fixture.content()); let input_file = absolute_fixture_path; let base_test_name = absolute_fixture_path .file_name() .and_then(|x| x.strip_suffix(".sql")) .unwrap(); - // Run test at each configured line width - for &max_line_length in &LINE_WIDTHS { + let widths: Vec = match explicit_width { + Some(width) => vec![width], + None => LINE_WIDTHS.to_vec(), + }; + + for max_line_length in widths { let test_name = format!("{base_test_name}_{max_line_length}"); // Split the content into statements - let split_result = pgls_statement_splitter::split(content); + let split_result = pgls_statement_splitter::split(&content); let mut formatted_statements = Vec::new(); for range in &split_result.ranges { @@ -147,15 +208,15 @@ fn test_multi(fixture: Fixture<&str>) { println!("Parsed AST: {ast:#?}"); - let mut emitter = EventEmitter::new(); + // The emitter gets the fixture config, not the default one: an option that decides + // which tokens exist is read here, before the renderer ever sees the events. + let mut emitter = EventEmitter::new(fixture_config.clone()); emit_node_enum(&ast, &mut emitter); let mut output = String::new(); let config = RenderConfig { max_line_length, - indent_size: 2, - indent_style: IndentStyle::Spaces, - ..Default::default() + ..RenderConfig::from(fixture_config.clone()) }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); diff --git a/crates/pgls_workspace/src/settings.rs b/crates/pgls_workspace/src/settings.rs index 70d864dc0..3ed0ac15c 100644 --- a/crates/pgls_workspace/src/settings.rs +++ b/crates/pgls_workspace/src/settings.rs @@ -18,7 +18,7 @@ use pgls_configuration::{ database::PartialDatabaseConfiguration, diagnostics::InvalidIgnorePattern, files::FilesConfiguration, - format::{FormatConfiguration, IndentStyle, KeywordCase}, + format::{CastStyle, FormatConfiguration, IndentStyle, KeywordCase}, migrations::{MigrationsConfiguration, PartialMigrationsConfiguration}, pglinter::PglinterConfiguration, plpgsql_check::PlPgSqlCheckConfiguration, @@ -376,6 +376,7 @@ fn to_formatter_settings( keyword_case: conf.keyword_case, constant_case: conf.constant_case, type_case: conf.type_case, + cast_style: conf.cast_style, skip_fn_bodies: conf.skip_fn_bodies, ignored_files: to_matcher(working_directory.clone(), Some(&conf.ignore))?, included_files: to_matcher(working_directory.clone(), Some(&conf.include))?, @@ -578,6 +579,9 @@ pub struct FormatterSettings { /// Data type casing (text, varchar, int): upper or lower. Default: lower. pub type_case: KeywordCase, + /// How an explicit cast is spelled: cast or operator. Default: cast. + pub cast_style: CastStyle, + /// If true, skip formatting of SQL function bodies (keep them verbatim). Default: false. pub skip_fn_bodies: bool, @@ -598,6 +602,7 @@ impl Default for FormatterSettings { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + cast_style: CastStyle::default(), skip_fn_bodies: false, ignored_files: Matcher::empty(), included_files: Matcher::empty(), diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index ec20cf15c..777ad05aa 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -937,6 +937,7 @@ impl Workspace for WorkspaceServer { keyword_case: settings.formatter.keyword_case.into(), constant_case: settings.formatter.constant_case.into(), type_case: settings.formatter.type_case.into(), + cast_style: settings.formatter.cast_style.into(), }; let mut diagnostics = Vec::new(); diff --git a/docs/features/formatting.md b/docs/features/formatting.md index e7700a0d6..1bacce856 100644 --- a/docs/features/formatting.md +++ b/docs/features/formatting.md @@ -37,6 +37,7 @@ Configure formatting behavior in your `postgres-language-server.jsonc`: | `keywordCase` | `"lower"` | Casing for SQL keywords: `"upper"` or `"lower"` | | `constantCase` | `"lower"` | Casing for constants (NULL, TRUE, FALSE): `"upper"` or `"lower"` | | `typeCase` | `"lower"` | Casing for data types (text, int, varchar): `"upper"` or `"lower"` | +| `castStyle` | `"cast"` | How an explicit cast is spelled: `"cast"` for `CAST(x AS t)`, `"operator"` for `x::t` | ### Example Output diff --git a/docs/schema.json b/docs/schema.json index d21ba9ee1..d71f6c14c 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -288,6 +288,14 @@ }, "additionalProperties": false }, + "CastStyle": { + "description": "How an explicit cast is spelled.", + "type": "string", + "enum": [ + "cast", + "operator" + ] + }, "Cluster": { "description": "A list of rules that belong to this group", "type": "object", @@ -454,6 +462,17 @@ "description": "The configuration for SQL formatting.", "type": "object", "properties": { + "castStyle": { + "description": "How an explicit cast is spelled: \"cast\" for `CAST(x AS t)`, \"operator\" for `x::t`. Default: \"cast\".", + "anyOf": [ + { + "$ref": "#/definitions/CastStyle" + }, + { + "type": "null" + } + ] + }, "constantCase": { "description": "Constant casing (NULL, TRUE, FALSE): \"upper\" or \"lower\". Default: \"lower\".", "anyOf": [