[`airflow`] Third positional parameter not named `ti_key` should be flagged for `BaseOperatorLink.get_link` (`AIR303`) by sjyangkevin · Pull Request #22828 · astral-sh/ruff

sjyangkevin · 2026-01-24T03:28:09Z

Summary

Context:
apache/airflow#41641
apache/airflow#46415

In the old signature, dttm is used by Airflow as a positional argument, so the argument name in the function declaration may not be dttm. In the new signature, however, the argument name must be ti_key (although it may be non-keyword-only; self, operator, ti_key should be considered new signature). So the deprecated signature detection rule should be exactly three positional arguments, the third one not named ti_key.

Old Signature

class MyOperatorLink(BaseOperatorLink):
    def get_link(self, operator, dttm):
        ...

New Signature

class MyOperatorLink(BaseOperatorLink):
    def get_link(self, operator, *, ti_key):
        ...

This PR extends AIR303 to detect deprecated get_link method signatures in BaseOperatorLink subclasses, which is a method definition. BaseOperatorLink is a base class can be extended by the user, and the get_link method is called by Airflow. As the way Airflow internally call this method change, we need to flag the user's implementation if the method definition is not match with the new signature (@Lee-W , please correct me if I understand it wrong, whenever you have time, thanks). The rule detects deprecated signatures where:

The class inherits from BaseOperatorLink (from airflow.models or airflow.sdk)
The method is named get_link
There are exactly 3 positional parameters
The 3rd parameter is not named as ti_key

Test Plan

The test cases are added to AIR303.py and the test snapshot is updated.

sjyangkevin · 2026-01-24T03:30:57Z

crates/ruff_linter/src/rules/airflow/helpers.rs

+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) enum FunctionSignatureChange {
+    /// Carries a message describing the function signature change.
+    Message(&'static str),
+}
+


The original definition in function_signature_change_in_3.rs becomes redundant as these enum items only encapsulate a message. Therefore, combine these into a single Message item, and move it to helpers.rs. This enum will be dedicated used for AIR303, or function signature change in future version of Airflow.

#[derive(Clone, Debug, Eq, PartialEq)] pub(crate) enum FunctionSignatureChangeType { /// Function signature changed to only accept keyword arguments. KeywordOnly { message: &'static str }, /// Function signature changed to not accept certain positional arguments. PositionalArgumentChange { message: &'static str }, }

I'm totally fine with this change if y'all prefer it, but I think it would also be fine to keep the old version if you think you'll want to differentiate between the variants later. You could always add a message method to avoid having to destructure the enum everywhere you use it. For example:

impl FunctionSignatureChangeType { fn message(&self) -> &'static str { match self { Self::KeywordOnly { message } | Self::PositionalArgumentChange { message } => message, } } }

Alternatively, I think you could just use a &'static str for the message field instead of having this enum type at all?

sjyangkevin · 2026-01-24T03:33:59Z

crates/ruff_linter/src/rules/airflow/helpers.rs

+
+/// This is a helper function to check if the given function definition is a method
+/// that inherits from a base class.
+pub(crate) fn is_method_in_subclass<F>(


This helper function generalizes the function is_execute_method_inherits_from_airflow_operator defined in the removal_in_3.rs. The function is checking if a subclass of airflow's BaseOperator implements a execute function. In AIR303, we need to check if a subclass of airflow's BaseOperatorLink implements a get_link function. Therefore, this is something reusable across multiple rules. I parameterized the method name and the match condition, to reduce redundant definition for similar checks.

Nice, I like this! One nit, it might make sense to have the Fn type take a &QualifiedName instead of the less type-safe &[&str]. Just an idea, though.

crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs

astral-sh-bot · 2026-01-24T03:37:05Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py

sjyangkevin · 2026-01-28T05:01:47Z

crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs

+        let is_valid_signature = match positional_count {
+            // check valid signature `def get_link(self, operator, *, ti_key)`
+            2 => parameters
+                .kwonlyargs
+                .iter()
+                .any(|p| p.name().as_str() == "ti_key"),
+            // check valid signature `def get_link(self, operator, ti_key)`
+            3 => parameters
+                .posonlyargs
+                .iter()
+                .chain(parameters.args.iter())
+                .nth(2)
+                .is_some_and(|p| p.name().as_str() == "ti_key"),
+            _ => false,
+        };
+
+        if !is_valid_signature {
+            checker.report_diagnostic(
+                Airflow3IncompatibleFunctionSignature {
+                    function_name: "get_link".to_string(),
+                    change: FunctionSignatureChange::Message(
+                        "Use `def get_link(self, operator, *, ti_key)` or `def get_link(self, operator, ti_key)` as the method signature.",
+                    ),
+                },
+                function_def.name.range(),


updated the logic to check for valid signature pattern, which is either def get_link(self, operator, *, ti_key) or ``def get_link(self, operator, ti_key)`, and the diagnostic will be raised when the method signature is evaluated to invalid.

The name of the first two positional parameters doesn't matter for the rule. If there are 2 positional parameters (i.e., self, operator), check if the keyword-only parameter is named ti_key. If there are 3 positional parameters, check if the third one is named ti_key.

The message and test cases are updated accordingly.

Lee-W · 2026-01-29T08:24:57Z

crates/ruff_linter/src/rules/airflow/rules/function_signature_change_in_3.rs

+        let positional_count = parameters.posonlyargs.len() + parameters.args.len();
+
+        let is_valid_signature = match positional_count {
+            // check valid signature `def get_link(self, operator, *, ti_key)`


what would happen if we use def get_link(self, operator, ti_key)

In this case, I think we have 3 positional parameters, and it will not match the first condition which check if the number of positional parameters is 2. It will proceed to the next condition and check if there are 3 positional parameters and iterate through it see if the 3rd one is named ti_key. I’ve added a test case for this but will also do a double check. Thanks!

sjyangkevin · 2026-02-03T04:58:57Z

Hi @Lee-W , @ntBre , I've added some more test cases for the new rule. I wonder if you can have a look when you get chance, and would appreciate if you have any feedback on the implementation for checking function signature. Thanks!

ntBre

Thank you! This looks good to me if @Lee-W is happy with it. I just had a couple of small suggestions for your consideration.

crates/ruff_linter/src/rules/airflow/helpers.rs

ntBre · 2026-02-04T16:59:15Z

crates/ruff_linter/src/rules/airflow/helpers.rs

+
+/// This is a helper function to check if the given function definition is a method
+/// that inherits from a base class.
+pub(crate) fn is_method_in_subclass<F>(


Nice, I like this! One nit, it might make sense to have the Fn type take a &QualifiedName instead of the less type-safe &[&str]. Just an idea, though.

ntBre · 2026-02-04T17:04:22Z

crates/ruff_linter/src/rules/airflow/helpers.rs

+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) enum FunctionSignatureChange {
+    /// Carries a message describing the function signature change.
+    Message(&'static str),
+}
+


I'm totally fine with this change if y'all prefer it, but I think it would also be fine to keep the old version if you think you'll want to differentiate between the variants later. You could always add a message method to avoid having to destructure the enum everywhere you use it. For example:

impl FunctionSignatureChangeType { fn message(&self) -> &'static str { match self { Self::KeywordOnly { message } | Self::PositionalArgumentChange { message } => message, } } }

Alternatively, I think you could just use a &'static str for the message field instead of having this enum type at all?

… name and any_qualified_base_class

sjyangkevin · 2026-02-05T05:17:29Z

crates/ruff_linter/src/rules/airflow/helpers.rs

+    F: Fn(QualifiedName) -> bool,
+{
+    if function_def.name.as_str() != method_name {
+        return false;
+    }
+
+    let ScopeKind::Class(class_def) = semantic.current_scope().kind else {
+        return false;
+    };
+
+    any_qualified_base_class(class_def, semantic, &is_base_class)


updated the helper to let Fn type take QualifiedName instead of &[&str], and use any_qualified_base_class for base class check. I got help from AI tool for it, wonder if it is the proper way to implement, still try to understand it.

That looks great!

Lee-W

Just checked the code and comments again. LGTM. Thanks!

sjyangkevin commented Jan 24, 2026

View reviewed changes

sjyangkevin mentioned this pull request Jan 24, 2026

Airflow 2 to 3 auto migration rules apache/airflow#41641

Closed

2 tasks

Lee-W reviewed Jan 28, 2026

View reviewed changes

crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py Show resolved Hide resolved

sjyangkevin force-pushed the air303-BaseOperatorLink-get_link branch from 2c7f7d2 to 78dccc5 Compare January 28, 2026 04:56

sjyangkevin commented Jan 28, 2026

View reviewed changes

sjyangkevin requested a review from Lee-W January 28, 2026 05:04

ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jan 28, 2026

Lee-W reviewed Jan 29, 2026

View reviewed changes

sjyangkevin force-pushed the air303-BaseOperatorLink-get_link branch from 1177bca to 20daae9 Compare February 3, 2026 04:51

sjyangkevin requested a review from Lee-W February 3, 2026 04:59

ntBre approved these changes Feb 4, 2026

View reviewed changes

sjyangkevin added 5 commits February 5, 2026 00:10

flag change in BaseOperatorLink.get_link

b04e96c

update rule and test cases

b84bb3c

add blank line at end of test file

fefb763

enhance test cases

01f4707

improve the implementation for is_method_in_subclass to use qualified…

c11b27c

… name and any_qualified_base_class

sjyangkevin force-pushed the air303-BaseOperatorLink-get_link branch from 20daae9 to c11b27c Compare February 5, 2026 05:10

sjyangkevin commented Feb 5, 2026

View reviewed changes

Lee-W reviewed Feb 6, 2026

View reviewed changes

ntBre merged commit c2ffad5 into astral-sh:main Feb 6, 2026
41 checks passed

BrewTestBot mentioned this pull request Feb 13, 2026

ruff 0.15.1 Homebrew/homebrew-core#267345

Merged

Conversation

sjyangkevin commented Jan 24, 2026

Summary

Test Plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

astral-sh-bot bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

Uh oh!

sjyangkevin Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjyangkevin commented Feb 3, 2026

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjyangkevin Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lee-W left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

astral-sh-bot bot commented Jan 24, 2026 •

edited

Loading

`ruff-ecosystem` results

sjyangkevin Jan 28, 2026 •

edited

Loading

sjyangkevin Feb 5, 2026 •

edited

Loading