From 5608b859bfe67b95253d587439383951f3212bcb Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Wed, 17 Dec 2025 21:40:17 -0500 Subject: [PATCH 1/3] add new code AIR303 and add rule for checking function signature changes that only accept keyword arguments --- .../test/fixtures/airflow/AIR303_args.py | 10 ++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../ruff_linter/src/rules/airflow/helpers.rs | 5 + crates/ruff_linter/src/rules/airflow/mod.rs | 1 + .../rules/function_signature_change_in_3.rs | 150 ++++++++++++++++++ .../src/rules/airflow/rules/mod.rs | 2 + .../src/rules/airflow/rules/removal_in_3.rs | 7 + .../airflow/rules/suggested_to_update_3_0.rs | 7 + ...airflow__tests__AIR303_AIR303_args.py.snap | 26 +++ ruff.schema.json | 1 + 11 files changed, 213 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/airflow/AIR303_args.py create mode 100644 crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs create mode 100644 crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/airflow/AIR303_args.py b/crates/ruff_linter/resources/test/fixtures/airflow/AIR303_args.py new file mode 100644 index 0000000000..299c670e8a --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/airflow/AIR303_args.py @@ -0,0 +1,10 @@ +from __future__ import annotations + +from airflow.lineage.hook import HookLineageCollector + +# airflow.lineage.hook +hlc = HookLineageCollector() +hlc.create_asset("there") +hlc.create_asset("should", "be", "no", "posarg") +hlc.create_asset(name="but", uri="kwargs are ok") +hlc.create_asset() \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 0957aee346..4b53e710c4 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1278,6 +1278,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { if checker.is_rule_enabled(Rule::Airflow3SuggestedUpdate) { airflow::rules::airflow_3_0_suggested_update_expr(checker, expr); } + if checker.is_rule_enabled(Rule::Airflow3FunctionSignatureChange) { + airflow::rules::airflow_3_keyword_args_only_function(checker, expr); + } if checker.is_rule_enabled(Rule::UnnecessaryCastToInt) { ruff::rules::unnecessary_cast_to_int(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 74cb38bddb..89dd21d357 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1123,6 +1123,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Airflow, "002") => rules::airflow::rules::AirflowDagNoScheduleArgument, (Airflow, "301") => rules::airflow::rules::Airflow3Removal, (Airflow, "302") => rules::airflow::rules::Airflow3MovedToProvider, + (Airflow, "303") => rules::airflow::rules::Airflow3FunctionSignatureChange, (Airflow, "311") => rules::airflow::rules::Airflow3SuggestedUpdate, (Airflow, "312") => rules::airflow::rules::Airflow3SuggestedToMoveToProvider, diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index 4b2c25f93f..078a0acce7 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -21,6 +21,11 @@ pub(crate) enum Replacement { Message(&'static str), // The attribute name of a class has been changed. AttrName(&'static str), + // The attribute name of a class has been changed with extra Message. + AttrNameWithMessage { + attr_name: &'static str, + message: &'static str, + }, // Symbols updated in Airflow 3 with replacement // e.g., `airflow.datasets.Dataset` to `airflow.sdk.Asset` Rename { diff --git a/crates/ruff_linter/src/rules/airflow/mod.rs b/crates/ruff_linter/src/rules/airflow/mod.rs index 0824b327dd..a00d84dbad 100644 --- a/crates/ruff_linter/src/rules/airflow/mod.rs +++ b/crates/ruff_linter/src/rules/airflow/mod.rs @@ -47,6 +47,7 @@ mod tests { #[test_case(Rule::Airflow3MovedToProvider, Path::new("AIR302_zendesk.py"))] #[test_case(Rule::Airflow3MovedToProvider, Path::new("AIR302_standard.py"))] #[test_case(Rule::Airflow3MovedToProvider, Path::new("AIR302_try.py"))] + #[test_case(Rule::Airflow3FunctionSignatureChange, Path::new("AIR303_args.py"))] #[test_case(Rule::Airflow3SuggestedUpdate, Path::new("AIR311_args.py"))] #[test_case(Rule::Airflow3SuggestedUpdate, Path::new("AIR311_names.py"))] #[test_case(Rule::Airflow3SuggestedUpdate, Path::new("AIR311_try.py"))] diff --git a/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs new file mode 100644 index 0000000000..a0031e3c2c --- /dev/null +++ b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs @@ -0,0 +1,150 @@ +use crate::checkers::ast::Checker; +use crate::rules::airflow::helpers::Replacement; +use crate::{Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{Arguments, Expr, ExprAttribute, ExprCall}; +use ruff_python_semantic::Modules; +use ruff_python_semantic::analyze::typing; +use ruff_text_size::Ranged; + +#[derive(ViolationMetadata)] +#[violation_metadata(stable_since = "0.13.0")] +pub(crate) struct Airflow3FunctionSignatureChange { + deprecated: String, + replacement: Replacement, +} + +/// ## What it does +/// Checks for Airflow function calls that use positional arguments when the +/// function signature changed to keyword-only arguments in Airflow 3.0. +/// +/// ## Why is this bad? +/// Airflow 3.0 changed certain function signatures to only accept keyword +/// arguments. Using positional arguments will cause runtime errors. +/// +/// ## Example +/// ```python +/// from airflow.lineage.hook import HookLineageCollector +/// +/// collector = HookLineageCollector() +/// # Using positional arguments (will fail in Airflow 3.0) +/// collector.create_asset("s3://bucket/key") +/// ``` +/// +/// Use instead: +/// ```python +/// from airflow.lineage.hook import HookLineageCollector +/// +/// collector = HookLineageCollector() +/// # Using keyword arguments +/// collector.create_asset(uri="s3://bucket/key") +/// ``` +impl Violation for Airflow3FunctionSignatureChange { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let Airflow3FunctionSignatureChange { + deprecated, + replacement, + } = self; + match replacement { + Replacement::None + | Replacement::AttrName(_) + | Replacement::Message(_) + | Replacement::AttrNameWithMessage { + attr_name: _, + message: _, + } + | Replacement::Rename { module: _, name: _ } + | Replacement::SourceModuleMoved { module: _, name: _ } => { + format!("`{deprecated}` only accepts keyword arguments in Airflow 3.0") + } + } + } + + fn fix_title(&self) -> Option { + let Airflow3FunctionSignatureChange { replacement, .. } = self; + match replacement { + Replacement::None => None, + Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), + Replacement::AttrNameWithMessage { attr_name, message } => { + Some(format!("Use `{attr_name}` instead; {message}")) + } + Replacement::Message(message) => Some((*message).to_string()), + Replacement::Rename { module, name } => { + Some(format!("Use `{name}` from `{module}` instead.")) + } + Replacement::SourceModuleMoved { module, name } => { + Some(format!("Use `{name}` from `{module}` instead.")) + } + } + } +} + +/// AIR303 +pub(crate) fn airflow_3_keyword_args_only_function(checker: &Checker, expr: &Expr) { + if !checker.semantic().seen_module(Modules::AIRFLOW) { + return; + } + + if let Expr::Call(call_expr @ ExprCall { arguments, .. }) = expr { + check_method(checker, call_expr, arguments); + } +} + +fn check_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) { + let Expr::Attribute(ExprAttribute { attr, value, .. }) = &*call_expr.func else { + return; + }; + + let Some(qualname) = typing::resolve_assignment(value, checker.semantic()) else { + return; + }; + + let replacement = match qualname.segments() { + ["airflow", "lineage", "hook", "HookLineageCollector"] => match attr.as_str() { + "create_dataset" => { + if arguments.find_positional(0).is_some() { + Replacement::AttrNameWithMessage { + attr_name: "create_asset", + message: "Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error", + } + } else { + Replacement::AttrName("create_asset") + } + } + "create_asset" => { + if arguments.find_positional(0).is_some() { + Replacement::Message( + "Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error", + ) + } else { + return; + } + } + _ => return, + }, + _ => return, + }; + // Create the `Fix` first to avoid cloning `Replacement`. + let fix = if let Replacement::AttrName(name) = replacement { + Some(Fix::safe_edit(Edit::range_replacement( + name.to_string(), + attr.range(), + ))) + } else { + None + }; + + let mut diagnostic = checker.report_diagnostic( + Airflow3FunctionSignatureChange { + deprecated: attr.to_string(), + replacement, + }, + attr.range(), + ); + if let Some(fix) = fix { + diagnostic.set_fix(fix); + } +} diff --git a/crates/ruff_linter/src/rules/airflow/rules/mod.rs b/crates/ruff_linter/src/rules/airflow/rules/mod.rs index 9b71032b5f..e68ca3aad5 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/mod.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/mod.rs @@ -1,4 +1,5 @@ pub(crate) use dag_schedule_argument::*; +pub(crate) use function_signature_change_in_3::*; pub(crate) use moved_to_provider_in_3::*; pub(crate) use removal_in_3::*; pub(crate) use suggested_to_move_to_provider_in_3::*; @@ -6,6 +7,7 @@ pub(crate) use suggested_to_update_3_0::*; pub(crate) use task_variable_name::*; mod dag_schedule_argument; +mod function_signature_change_in_3; mod moved_to_provider_in_3; mod removal_in_3; mod suggested_to_move_to_provider_in_3; diff --git a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs index 562f37230d..b25980d4dd 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs @@ -60,6 +60,10 @@ impl Violation for Airflow3Removal { Replacement::None | Replacement::AttrName(_) | Replacement::Message(_) + | Replacement::AttrNameWithMessage { + attr_name: _, + message: _, + } | Replacement::Rename { module: _, name: _ } | Replacement::SourceModuleMoved { module: _, name: _ } => { format!("`{deprecated}` is removed in Airflow 3.0") @@ -72,6 +76,9 @@ impl Violation for Airflow3Removal { match replacement { Replacement::None => None, Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), + Replacement::AttrNameWithMessage { attr_name, message } => { + Some(format!("Use `{attr_name}` instead; {message}")) + } Replacement::Message(message) => Some((*message).to_string()), Replacement::Rename { module, name } => { Some(format!("Use `{name}` from `{module}` instead.")) diff --git a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs index 327576cf9d..8f072859bf 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs @@ -56,6 +56,10 @@ impl Violation for Airflow3SuggestedUpdate { Replacement::None | Replacement::AttrName(_) | Replacement::Message(_) + | Replacement::AttrNameWithMessage { + attr_name: _, + message: _, + } | Replacement::Rename { module: _, name: _ } | Replacement::SourceModuleMoved { module: _, name: _ } => { format!( @@ -71,6 +75,9 @@ impl Violation for Airflow3SuggestedUpdate { match replacement { Replacement::None => None, Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), + Replacement::AttrNameWithMessage { attr_name, message } => { + Some(format!("Use `{attr_name}` instead; {message}")) + } Replacement::Message(message) => Some((*message).to_string()), Replacement::Rename { module, name } => { Some(format!("Use `{name}` from `{module}` instead.")) diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap new file mode 100644 index 0000000000..c07a4edccd --- /dev/null +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff_linter/src/rules/airflow/mod.rs +--- +AIR303 `create_asset` only accepts keyword arguments in Airflow 3.0 + --> AIR303_args.py:7:5 + | +5 | # airflow.lineage.hook +6 | hlc = HookLineageCollector() +7 | hlc.create_asset("there") + | ^^^^^^^^^^^^ +8 | hlc.create_asset("should", "be", "no", "posarg") +9 | hlc.create_asset(name="but", uri="kwargs are ok") + | +help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error + +AIR303 `create_asset` only accepts keyword arguments in Airflow 3.0 + --> AIR303_args.py:8:5 + | + 6 | hlc = HookLineageCollector() + 7 | hlc.create_asset("there") + 8 | hlc.create_asset("should", "be", "no", "posarg") + | ^^^^^^^^^^^^ + 9 | hlc.create_asset(name="but", uri="kwargs are ok") +10 | hlc.create_asset() + | +help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error diff --git a/ruff.schema.json b/ruff.schema.json index 5af8837779..fd7a40b8f6 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2951,6 +2951,7 @@ "AIR30", "AIR301", "AIR302", + "AIR303", "AIR31", "AIR311", "AIR312", From 396e1115c61333b05422a3ddcf50ddcb4f5093e4 Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Fri, 19 Dec 2025 00:53:48 -0500 Subject: [PATCH 2/3] updates according to feedback --- .../src/checkers/ast/analyze/expression.rs | 2 +- .../ruff_linter/src/rules/airflow/helpers.rs | 5 -- .../rules/function_signature_change_in_3.rs | 65 ++++++++----------- .../src/rules/airflow/rules/removal_in_3.rs | 7 -- .../airflow/rules/suggested_to_update_3_0.rs | 7 -- ...airflow__tests__AIR303_AIR303_args.py.snap | 4 +- 6 files changed, 31 insertions(+), 59 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 4b53e710c4..c6bfa50551 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1279,7 +1279,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { airflow::rules::airflow_3_0_suggested_update_expr(checker, expr); } if checker.is_rule_enabled(Rule::Airflow3FunctionSignatureChange) { - airflow::rules::airflow_3_keyword_args_only_function(checker, expr); + airflow::rules::airflow_3_function_signature_change_expr(checker, expr); } if checker.is_rule_enabled(Rule::UnnecessaryCastToInt) { ruff::rules::unnecessary_cast_to_int(checker, call); diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index 078a0acce7..4b2c25f93f 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -21,11 +21,6 @@ pub(crate) enum Replacement { Message(&'static str), // The attribute name of a class has been changed. AttrName(&'static str), - // The attribute name of a class has been changed with extra Message. - AttrNameWithMessage { - attr_name: &'static str, - message: &'static str, - }, // Symbols updated in Airflow 3 with replacement // e.g., `airflow.datasets.Dataset` to `airflow.sdk.Asset` Rename { diff --git a/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs index a0031e3c2c..924e71fa7c 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs @@ -7,27 +7,22 @@ use ruff_python_semantic::Modules; use ruff_python_semantic::analyze::typing; use ruff_text_size::Ranged; -#[derive(ViolationMetadata)] -#[violation_metadata(stable_since = "0.13.0")] -pub(crate) struct Airflow3FunctionSignatureChange { - deprecated: String, - replacement: Replacement, -} - /// ## What it does -/// Checks for Airflow function calls that use positional arguments when the -/// function signature changed to keyword-only arguments in Airflow 3.0. +/// Checks for Airflow function calls that will raise a runtime error in Airflow 3.0 +/// due to function signature changes, such as functions that changed to accept only +/// keyword arguments, parameter reordering, or parameter type changes. /// /// ## Why is this bad? -/// Airflow 3.0 changed certain function signatures to only accept keyword -/// arguments. Using positional arguments will cause runtime errors. +/// Airflow 3.0 might introduce changes to function signatures. Code that +/// worked in Airflow 2.x may raise a runtime error if not updated in Airflow +/// 3.0. /// /// ## Example /// ```python /// from airflow.lineage.hook import HookLineageCollector /// /// collector = HookLineageCollector() -/// # Using positional arguments (will fail in Airflow 3.0) +/// # Passing positional arguments will raise a runtime error in Airflow 3.0 /// collector.create_asset("s3://bucket/key") /// ``` /// @@ -36,29 +31,32 @@ pub(crate) struct Airflow3FunctionSignatureChange { /// from airflow.lineage.hook import HookLineageCollector /// /// collector = HookLineageCollector() -/// # Using keyword arguments +/// # Passing arguments as keyword arguments instead of positional arguments /// collector.create_asset(uri="s3://bucket/key") /// ``` +#[derive(ViolationMetadata)] +#[violation_metadata(stable_since = "0.14.11")] +pub(crate) struct Airflow3FunctionSignatureChange { + function_def: String, + replacement: Replacement, +} + impl Violation for Airflow3FunctionSignatureChange { const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; #[derive_message_formats] fn message(&self) -> String { let Airflow3FunctionSignatureChange { - deprecated, + function_def, replacement, } = self; match replacement { Replacement::None | Replacement::AttrName(_) | Replacement::Message(_) - | Replacement::AttrNameWithMessage { - attr_name: _, - message: _, - } | Replacement::Rename { module: _, name: _ } | Replacement::SourceModuleMoved { module: _, name: _ } => { - format!("`{deprecated}` only accepts keyword arguments in Airflow 3.0") + format!("`{function_def}` only accepts keyword arguments in Airflow 3.0") } } } @@ -68,9 +66,6 @@ impl Violation for Airflow3FunctionSignatureChange { match replacement { Replacement::None => None, Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), - Replacement::AttrNameWithMessage { attr_name, message } => { - Some(format!("Use `{attr_name}` instead; {message}")) - } Replacement::Message(message) => Some((*message).to_string()), Replacement::Rename { module, name } => { Some(format!("Use `{name}` from `{module}` instead.")) @@ -83,17 +78,22 @@ impl Violation for Airflow3FunctionSignatureChange { } /// AIR303 -pub(crate) fn airflow_3_keyword_args_only_function(checker: &Checker, expr: &Expr) { +pub(crate) fn airflow_3_function_signature_change_expr(checker: &Checker, expr: &Expr) { if !checker.semantic().seen_module(Modules::AIRFLOW) { return; } + airflow_3_keyword_args_only_function(checker, expr); +} + +/// Check for functions that changed to only accept keyword arguments +fn airflow_3_keyword_args_only_function(checker: &Checker, expr: &Expr) { if let Expr::Call(call_expr @ ExprCall { arguments, .. }) = expr { - check_method(checker, call_expr, arguments); + check_keyword_only_method(checker, call_expr, arguments); } } -fn check_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) { +fn check_keyword_only_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) { let Expr::Attribute(ExprAttribute { attr, value, .. }) = &*call_expr.func else { return; }; @@ -104,22 +104,13 @@ fn check_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) let replacement = match qualname.segments() { ["airflow", "lineage", "hook", "HookLineageCollector"] => match attr.as_str() { - "create_dataset" => { - if arguments.find_positional(0).is_some() { - Replacement::AttrNameWithMessage { - attr_name: "create_asset", - message: "Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error", - } - } else { - Replacement::AttrName("create_asset") - } - } "create_asset" => { if arguments.find_positional(0).is_some() { Replacement::Message( - "Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error", + "Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`)", ) } else { + // No positional args, no violation return; } } @@ -139,7 +130,7 @@ fn check_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) let mut diagnostic = checker.report_diagnostic( Airflow3FunctionSignatureChange { - deprecated: attr.to_string(), + function_def: attr.to_string(), replacement, }, attr.range(), diff --git a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs index b25980d4dd..562f37230d 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs @@ -60,10 +60,6 @@ impl Violation for Airflow3Removal { Replacement::None | Replacement::AttrName(_) | Replacement::Message(_) - | Replacement::AttrNameWithMessage { - attr_name: _, - message: _, - } | Replacement::Rename { module: _, name: _ } | Replacement::SourceModuleMoved { module: _, name: _ } => { format!("`{deprecated}` is removed in Airflow 3.0") @@ -76,9 +72,6 @@ impl Violation for Airflow3Removal { match replacement { Replacement::None => None, Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), - Replacement::AttrNameWithMessage { attr_name, message } => { - Some(format!("Use `{attr_name}` instead; {message}")) - } Replacement::Message(message) => Some((*message).to_string()), Replacement::Rename { module, name } => { Some(format!("Use `{name}` from `{module}` instead.")) diff --git a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs index 8f072859bf..327576cf9d 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs @@ -56,10 +56,6 @@ impl Violation for Airflow3SuggestedUpdate { Replacement::None | Replacement::AttrName(_) | Replacement::Message(_) - | Replacement::AttrNameWithMessage { - attr_name: _, - message: _, - } | Replacement::Rename { module: _, name: _ } | Replacement::SourceModuleMoved { module: _, name: _ } => { format!( @@ -75,9 +71,6 @@ impl Violation for Airflow3SuggestedUpdate { match replacement { Replacement::None => None, Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), - Replacement::AttrNameWithMessage { attr_name, message } => { - Some(format!("Use `{attr_name}` instead; {message}")) - } Replacement::Message(message) => Some((*message).to_string()), Replacement::Rename { module, name } => { Some(format!("Use `{name}` from `{module}` instead.")) diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap index c07a4edccd..42a6fce886 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303_args.py.snap @@ -11,7 +11,7 @@ AIR303 `create_asset` only accepts keyword arguments in Airflow 3.0 8 | hlc.create_asset("should", "be", "no", "posarg") 9 | hlc.create_asset(name="but", uri="kwargs are ok") | -help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error +help: Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`) AIR303 `create_asset` only accepts keyword arguments in Airflow 3.0 --> AIR303_args.py:8:5 @@ -23,4 +23,4 @@ AIR303 `create_asset` only accepts keyword arguments in Airflow 3.0 9 | hlc.create_asset(name="but", uri="kwargs are ok") 10 | hlc.create_asset() | -help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error +help: Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`) From e20c94b8af8594c8ffd00b017b72c9ffb97daadd Mon Sep 17 00:00:00 2001 From: Kevin Yang Date: Fri, 19 Dec 2025 15:48:32 -0500 Subject: [PATCH 3/3] add new enum and remove fix/suggest --- .../ruff_linter/src/rules/airflow/helpers.rs | 6 ++ .../rules/function_signature_change_in_3.rs | 64 ++++++------------- 2 files changed, 26 insertions(+), 44 deletions(-) diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index 4b2c25f93f..75844363cf 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -35,6 +35,12 @@ pub(crate) enum Replacement { }, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum FunctionSignatureChangeType { + /// Function signature changed to only accept keyword arguments. + KeywordOnly { message: &'static str }, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) enum ProviderReplacement { Rename { diff --git a/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs index 924e71fa7c..0b7fe1435e 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs @@ -1,6 +1,6 @@ use crate::checkers::ast::Checker; -use crate::rules::airflow::helpers::Replacement; -use crate::{Edit, Fix, FixAvailability, Violation}; +use crate::rules::airflow::helpers::FunctionSignatureChangeType; +use crate::{FixAvailability, Violation}; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{Arguments, Expr, ExprAttribute, ExprCall}; use ruff_python_semantic::Modules; @@ -37,42 +37,30 @@ use ruff_text_size::Ranged; #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "0.14.11")] pub(crate) struct Airflow3FunctionSignatureChange { - function_def: String, - replacement: Replacement, + function_name: String, + change_type: FunctionSignatureChangeType, } impl Violation for Airflow3FunctionSignatureChange { - const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; #[derive_message_formats] fn message(&self) -> String { let Airflow3FunctionSignatureChange { - function_def, - replacement, + function_name, + change_type, } = self; - match replacement { - Replacement::None - | Replacement::AttrName(_) - | Replacement::Message(_) - | Replacement::Rename { module: _, name: _ } - | Replacement::SourceModuleMoved { module: _, name: _ } => { - format!("`{function_def}` only accepts keyword arguments in Airflow 3.0") + match change_type { + FunctionSignatureChangeType::KeywordOnly { .. } => { + format!("`{function_name}` only accepts keyword arguments in Airflow 3.0") } } } fn fix_title(&self) -> Option { - let Airflow3FunctionSignatureChange { replacement, .. } = self; - match replacement { - Replacement::None => None, - Replacement::AttrName(name) => Some(format!("Use `{name}` instead")), - Replacement::Message(message) => Some((*message).to_string()), - Replacement::Rename { module, name } => { - Some(format!("Use `{name}` from `{module}` instead.")) - } - Replacement::SourceModuleMoved { module, name } => { - Some(format!("Use `{name}` from `{module}` instead.")) - } + let Airflow3FunctionSignatureChange { change_type, .. } = self; + match change_type { + FunctionSignatureChangeType::KeywordOnly { message } => Some(message.to_string()), } } } @@ -102,13 +90,13 @@ fn check_keyword_only_method(checker: &Checker, call_expr: &ExprCall, arguments: return; }; - let replacement = match qualname.segments() { + let change_type = match qualname.segments() { ["airflow", "lineage", "hook", "HookLineageCollector"] => match attr.as_str() { "create_asset" => { if arguments.find_positional(0).is_some() { - Replacement::Message( - "Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`)", - ) + FunctionSignatureChangeType::KeywordOnly { + message: "Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`)", + } } else { // No positional args, no violation return; @@ -118,24 +106,12 @@ fn check_keyword_only_method(checker: &Checker, call_expr: &ExprCall, arguments: }, _ => return, }; - // Create the `Fix` first to avoid cloning `Replacement`. - let fix = if let Replacement::AttrName(name) = replacement { - Some(Fix::safe_edit(Edit::range_replacement( - name.to_string(), - attr.range(), - ))) - } else { - None - }; - let mut diagnostic = checker.report_diagnostic( + checker.report_diagnostic( Airflow3FunctionSignatureChange { - function_def: attr.to_string(), - replacement, + function_name: attr.to_string(), + change_type, }, attr.range(), ); - if let Some(fix) = fix { - diagnostic.set_fix(fix); - } }