From 071647a203df7d12fadfc96bda95027a56e7ee54 Mon Sep 17 00:00:00 2001 From: subotac <73706465+subotac@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:21:24 +0200 Subject: [PATCH] Generic: Include closing parentheses in function spans --- .../fuzz_seeds/generic_2050_function_span.sql | 1 + src/ast/mod.rs | 3 ++ src/ast/spans.rs | 4 +- src/ast/visitor.rs | 1 + src/parser/mod.rs | 10 ++-- src/test_utils.rs | 1 + tests/sqlparser_bigquery.rs | 2 + tests/sqlparser_clickhouse.rs | 6 +++ tests/sqlparser_common.rs | 47 +++++++++++++++++++ tests/sqlparser_duckdb.rs | 1 + tests/sqlparser_hive.rs | 1 + tests/sqlparser_mssql.rs | 2 + tests/sqlparser_postgres.rs | 6 +++ tests/sqlparser_redshift.rs | 1 + tests/sqlparser_snowflake.rs | 8 ++++ tests/sqlparser_sqlite.rs | 1 + 16 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 fuzz/fuzz_seeds/generic_2050_function_span.sql diff --git a/fuzz/fuzz_seeds/generic_2050_function_span.sql b/fuzz/fuzz_seeds/generic_2050_function_span.sql new file mode 100644 index 0000000000..fe00f984ce --- /dev/null +++ b/fuzz/fuzz_seeds/generic_2050_function_span.sql @@ -0,0 +1 @@ +SELECT foo(); diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 87618bbc5f..d71011a01b 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -8418,6 +8418,9 @@ impl fmt::Display for FunctionArguments { #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] pub struct FunctionArgumentList { + /// The closing parenthesis token. + #[cfg_attr(feature = "serde", serde(default = "AttachedToken::empty"))] + pub closing_token: AttachedToken, /// `[ ALL | DISTINCT ]` pub duplicate_treatment: Option, /// The function arguments. diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 74e0844d56..008878bb85 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -1864,6 +1864,7 @@ impl Spanned for FunctionArguments { impl Spanned for FunctionArgumentList { fn span(&self) -> Span { let FunctionArgumentList { + closing_token, duplicate_treatment: _, // enum args, clauses, @@ -1873,7 +1874,8 @@ impl Spanned for FunctionArgumentList { // # todo: duplicate-treatment span args.iter() .map(|i| i.span()) - .chain(clauses.iter().map(|i| i.span())), + .chain(clauses.iter().map(|i| i.span())) + .chain(iter::once(closing_token.0.span)), ) } } diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index c78ff2bfb9..56fb458b67 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -687,6 +687,7 @@ where /// name: ObjectName::from(vec![Ident::new("f")]), /// uses_odbc_syntax: false, /// args: FunctionArguments::List(FunctionArgumentList { +/// closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), /// duplicate_treatment: None, /// args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(old_expr))], /// clauses: vec![], diff --git a/src/parser/mod.rs b/src/parser/mod.rs index f5a6ff8c71..8b83abf9d0 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2583,8 +2583,9 @@ impl<'a> Parser<'a> { arg: FunctionArgExpr::Expr(self.parse_expr()?), operator: FunctionArgOperator::Space, }; - self.expect_token(&Token::RParen)?; + let closing_token = self.expect_token(&Token::RParen)?.into(); Ok(FunctionArgumentList { + closing_token, duplicate_treatment: None, args: vec![arg], clauses: vec![], @@ -19792,8 +19793,10 @@ impl<'a> Parser<'a> { )); } - if self.consume_token(&Token::RParen) { + if self.peek_token_ref().token == Token::RParen { + let closing_token = self.next_token().into(); return Ok(FunctionArgumentList { + closing_token, duplicate_treatment: None, args: vec![], clauses, @@ -19866,8 +19869,9 @@ impl<'a> Parser<'a> { )); } - self.expect_token(&Token::RParen)?; + let closing_token = self.expect_token(&Token::RParen)?.into(); Ok(FunctionArgumentList { + closing_token, duplicate_treatment, args, clauses, diff --git a/src/test_utils.rs b/src/test_utils.rs index 4c9bbb2bc2..963f385f12 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -442,6 +442,7 @@ pub fn call(function: &str, args: impl IntoIterator) -> Expr { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: crate::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: args .into_iter() diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index b110423370..27f402a2bd 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -2281,6 +2281,8 @@ fn parse_map_access_expr() { )]), parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( number("2").with_empty_span(), diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 2c59d66cad..9ba1aa1475 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -196,6 +196,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -851,6 +852,8 @@ fn parse_create_table_with_primary_key() { assert_eq!( actual.args, FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Identifier( Ident::new(arg) )),)], @@ -908,6 +911,7 @@ fn parse_create_table_with_variant_default_expressions() { name: ObjectName::from(vec![Ident::new("now")]), uses_odbc_syntax: false, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![], duplicate_treatment: None, clauses: vec![], @@ -929,6 +933,7 @@ fn parse_create_table_with_variant_default_expressions() { name: ObjectName::from(vec![Ident::new("now")]), uses_odbc_syntax: false, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![], duplicate_treatment: None, clauses: vec![], @@ -958,6 +963,7 @@ fn parse_create_table_with_variant_default_expressions() { name: ObjectName::from(vec![Ident::new("toString")]), uses_odbc_syntax: false, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Identifier(Ident::new("c")) ))], diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 396e898bca..61d64d73da 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1438,6 +1438,7 @@ fn parse_select_count_wildcard() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Wildcard)], clauses: vec![], @@ -1461,6 +1462,7 @@ fn parse_select_count_distinct() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: Some(DuplicateTreatment::Distinct), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::UnaryOp { op: UnaryOperator::Plus, @@ -3569,6 +3571,7 @@ fn parse_select_having() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Wildcard)], clauses: vec![], @@ -3600,6 +3603,7 @@ fn parse_select_qualify() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -4021,6 +4025,7 @@ fn parse_listagg() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: Some(DuplicateTreatment::Distinct), args: vec![ FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Identifier(Ident::new( @@ -6378,6 +6383,7 @@ fn parse_named_argument_function() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![ FunctionArg::Named { @@ -6418,6 +6424,7 @@ fn parse_named_argument_function_with_eq_operator() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![ FunctionArg::Named { @@ -6493,6 +6500,7 @@ fn parse_window_functions() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -6635,6 +6643,8 @@ fn test_parse_named_window() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident { @@ -6670,6 +6680,8 @@ fn test_parse_named_window() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident { @@ -11640,6 +11652,7 @@ fn parse_time_functions() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -13407,6 +13420,7 @@ fn parse_call() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( (Value::SingleQuotedString("a".to_string())).with_empty_span() @@ -13926,6 +13940,8 @@ fn parse_map_access_expr() { )]), parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( (number("2")).with_empty_span(), @@ -14266,6 +14282,7 @@ fn test_selective_aggregation() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("name")) @@ -14285,6 +14302,8 @@ fn test_selective_aggregation() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("name")) @@ -17078,6 +17097,7 @@ fn parse_composite_access_expr() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("a")) @@ -17102,6 +17122,7 @@ fn parse_composite_access_expr() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("a")) @@ -17128,6 +17149,7 @@ fn parse_composite_access_expr() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("a")), @@ -22099,3 +22121,28 @@ fn parse_qualified_literal_prefix_requires_adjacent_period() { dialects.tokenizes_to(sql, vec![token]); } } + +#[test] +fn function_spans_include_closing_parentheses() { + for call in ["foo()", "foo(1)", "foo(bar())", "foo( /* comment */ )"] { + let sql = format!("SELECT {call}"); + let statements = all_dialects() + .run_parser_method(&sql, |parser| parser.parse_statements()) + .unwrap(); + let Statement::Query(query) = &statements[0] else { + panic!("expected a query"); + }; + let SetExpr::Select(select) = query.body.as_ref() else { + panic!("expected a select"); + }; + let UnnamedExpr(Expr::Function(function)) = &select.projection[0] else { + panic!("expected a function"); + }; + assert_eq!(function.span().start, Location::new(1, 8), "{call}"); + assert_eq!( + function.span().end, + Location::new(1, sql.len() as u64 + 1), + "{call}" + ); + } +} diff --git a/tests/sqlparser_duckdb.rs b/tests/sqlparser_duckdb.rs index 216d514713..184f067f58 100644 --- a/tests/sqlparser_duckdb.rs +++ b/tests/sqlparser_duckdb.rs @@ -638,6 +638,7 @@ fn test_duckdb_named_argument_function_with_assignment_operator() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![ FunctionArg::Named { diff --git a/tests/sqlparser_hive.rs b/tests/sqlparser_hive.rs index ab4d4f141a..b16ca2357c 100644 --- a/tests/sqlparser_hive.rs +++ b/tests/sqlparser_hive.rs @@ -505,6 +505,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 62f3a53efd..afcedd4916 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -898,6 +898,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -1893,6 +1894,7 @@ fn parse_create_table_with_valid_options() { parameters: FunctionArguments::None, args: FunctionArguments::List( FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![ FunctionArg::Unnamed( diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index da83d4f71a..5670262fa2 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -3401,6 +3401,8 @@ fn parse_create_indices_with_operator_classes() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![ FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Identifier( @@ -4360,6 +4362,7 @@ fn test_composite_value() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Array( Array { @@ -5144,6 +5147,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -6296,6 +6300,8 @@ fn parse_create_table_with_partition_by() { assert_eq!("RANGE", f.name.to_string()); assert_eq!( FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, clauses: vec![], args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( diff --git a/tests/sqlparser_redshift.rs b/tests/sqlparser_redshift.rs index 5bf1a89328..400593effb 100644 --- a/tests/sqlparser_redshift.rs +++ b/tests/sqlparser_redshift.rs @@ -141,6 +141,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index 0c37194467..8a2337fa03 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -541,6 +541,8 @@ fn test_snowflake_create_table_cluster_by() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("c")) ))], @@ -1446,6 +1448,7 @@ fn parse_delimited_identifiers() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![], clauses: vec![], @@ -1664,6 +1667,8 @@ fn test_alter_table_clustering() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: + sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("c3")) ))], @@ -4723,6 +4728,7 @@ fn test_snowflake_identifier_function() { assert_eq!( *args, FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( Value::SingleQuotedString("email".to_string()).into() )))], @@ -4744,6 +4750,7 @@ fn test_snowflake_identifier_function() { assert_eq!( *args, FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( Value::SingleQuotedString("\"Email\"".to_string()).into() )))], @@ -4768,6 +4775,7 @@ fn test_snowflake_identifier_function() { assert_eq!( *args, FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( Value::SingleQuotedString("alias1".to_string()).into() )))], diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index 10059c89e0..6767834526 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -443,6 +443,7 @@ fn parse_window_function_with_filter() { uses_odbc_syntax: false, parameters: FunctionArguments::None, args: FunctionArguments::List(FunctionArgumentList { + closing_token: sqlparser::ast::helpers::attached_token::AttachedToken::empty(), duplicate_treatment: None, args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr( Expr::Identifier(Ident::new("x"))