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..c6bfa50551 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_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/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..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/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..0b7fe1435e --- /dev/null +++ b/crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs @@ -0,0 +1,117 @@ +use crate::checkers::ast::Checker; +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; +use ruff_python_semantic::analyze::typing; +use ruff_text_size::Ranged; + +/// ## What it does +/// 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 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() +/// # Passing positional arguments will raise a runtime error in Airflow 3.0 +/// collector.create_asset("s3://bucket/key") +/// ``` +/// +/// Use instead: +/// ```python +/// from airflow.lineage.hook import HookLineageCollector +/// +/// collector = HookLineageCollector() +/// # 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_name: String, + change_type: FunctionSignatureChangeType, +} + +impl Violation for Airflow3FunctionSignatureChange { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + let Airflow3FunctionSignatureChange { + function_name, + change_type, + } = self; + match change_type { + FunctionSignatureChangeType::KeywordOnly { .. } => { + format!("`{function_name}` only accepts keyword arguments in Airflow 3.0") + } + } + } + + fn fix_title(&self) -> Option { + let Airflow3FunctionSignatureChange { change_type, .. } = self; + match change_type { + FunctionSignatureChangeType::KeywordOnly { message } => Some(message.to_string()), + } + } +} + +/// AIR303 +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_keyword_only_method(checker, call_expr, arguments); + } +} + +fn check_keyword_only_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 change_type = match qualname.segments() { + ["airflow", "lineage", "hook", "HookLineageCollector"] => match attr.as_str() { + "create_asset" => { + if arguments.find_positional(0).is_some() { + FunctionSignatureChangeType::KeywordOnly { + message: "Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`)", + } + } else { + // No positional args, no violation + return; + } + } + _ => return, + }, + _ => return, + }; + + checker.report_diagnostic( + Airflow3FunctionSignatureChange { + function_name: attr.to_string(), + change_type, + }, + attr.range(), + ); +} 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/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..42a6fce886 --- /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: 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 + | + 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: Pass positional arguments as keyword arguments (e.g., `create_asset(uri=...)`) 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",