Skip to content

Commit e224675

Browse files
committed
feat(AIR301): warn if airflow.lineage.hook.HookLineageCollector.create_asset has positional arguments
1 parent fa12fd0 commit e224675

File tree

5 files changed

+90
-12
lines changed

5 files changed

+90
-12
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,9 @@
9696
# airflow.secrets.local_filesystem
9797
lfb = LocalFilesystemBackend()
9898
lfb.get_connections()
99+
100+
101+
hlc.create_asset("there")
102+
hlc.create_asset("should", "be", "no", "posarg")
103+
hlc.create_asset(name="but", uri="kwargs are ok")
104+
hlc.create_asset()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ pub(crate) enum Replacement {
2121
Message(&'static str),
2222
// The attribute name of a class has been changed.
2323
AttrName(&'static str),
24+
// The attribute name of a class has been changed with extra Message.
25+
AttrNameWithMessage {
26+
attr_name: &'static str,
27+
message: &'static str,
28+
},
2429
// Symbols updated in Airflow 3 with replacement
2530
// e.g., `airflow.datasets.Dataset` to `airflow.sdk.Asset`
2631
Rename {

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

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,12 @@ impl Violation for Airflow3Removal {
5858
} = self;
5959
match replacement {
6060
Replacement::None
61-
| Replacement::AttrName(_)
6261
| Replacement::Message(_)
62+
| Replacement::AttrName(_)
63+
| Replacement::AttrNameWithMessage {
64+
attr_name: _,
65+
message: _,
66+
}
6367
| Replacement::Rename { module: _, name: _ }
6468
| Replacement::SourceModuleMoved { module: _, name: _ } => {
6569
format!("`{deprecated}` is removed in Airflow 3.0")
@@ -71,8 +75,11 @@ impl Violation for Airflow3Removal {
7175
let Airflow3Removal { replacement, .. } = self;
7276
match replacement {
7377
Replacement::None => None,
74-
Replacement::AttrName(name) => Some(format!("Use `{name}` instead")),
7578
Replacement::Message(message) => Some((*message).to_string()),
79+
Replacement::AttrName(name) => Some(format!("Use `{name}` instead")),
80+
Replacement::AttrNameWithMessage { attr_name, message } => {
81+
Some(format!("Use `{attr_name}` instead; {message}"))
82+
}
7683
Replacement::Rename { module, name } => {
7784
Some(format!("Use `{name}` from `{module}` instead."))
7885
}
@@ -98,7 +105,7 @@ pub(crate) fn airflow_3_removal_expr(checker: &Checker, expr: &Expr) {
98105
if let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) {
99106
check_call_arguments(checker, &qualified_name, arguments);
100107
}
101-
check_method(checker, call_expr);
108+
check_method(checker, call_expr, arguments);
102109
check_context_key_usage_in_call(checker, call_expr);
103110
}
104111
Expr::Attribute(attribute_expr @ ExprAttribute { range, .. }) => {
@@ -467,7 +474,7 @@ fn is_kwarg_parameter(semantic: &SemanticModel, name: &ExprName) -> bool {
467474
/// manager = DatasetManager()
468475
/// manager.register_datsaet_change()
469476
/// ```
470-
fn check_method(checker: &Checker, call_expr: &ExprCall) {
477+
fn check_method(checker: &Checker, call_expr: &ExprCall, arguments: &Arguments) {
471478
let Expr::Attribute(ExprAttribute { attr, value, .. }) = &*call_expr.func else {
472479
return;
473480
};
@@ -486,10 +493,28 @@ fn check_method(checker: &Checker, call_expr: &ExprCall) {
486493
_ => return,
487494
},
488495
["airflow", "lineage", "hook", "HookLineageCollector"] => match attr.as_str() {
489-
"create_dataset" => Replacement::AttrName("create_asset"),
490496
"add_input_dataset" => Replacement::AttrName("add_input_asset"),
491497
"add_output_dataset" => Replacement::AttrName("add_output_asset"),
492498
"collected_datasets" => Replacement::AttrName("collected_assets"),
499+
"create_dataset" => {
500+
if arguments.len() > 0 {
501+
Replacement::AttrNameWithMessage {
502+
attr_name: "create_asset",
503+
message: "Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error",
504+
}
505+
} else {
506+
Replacement::AttrName("create_asset")
507+
}
508+
}
509+
"create_asset" => {
510+
if arguments.len() > 0 {
511+
Replacement::Message(
512+
"Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error",
513+
)
514+
} else {
515+
return;
516+
}
517+
}
493518
_ => return,
494519
},
495520
["airflow", "providers_manager", "ProvidersManager"] => match attr.as_str() {

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,12 @@ impl Violation for Airflow3SuggestedUpdate {
5454
} = self;
5555
match replacement {
5656
Replacement::None
57-
| Replacement::AttrName(_)
5857
| Replacement::Message(_)
58+
| Replacement::AttrName(_)
59+
| Replacement::AttrNameWithMessage {
60+
attr_name: _,
61+
message: _,
62+
}
5963
| Replacement::Rename { module: _, name: _ }
6064
| Replacement::SourceModuleMoved { module: _, name: _ } => {
6165
format!(
@@ -70,8 +74,11 @@ impl Violation for Airflow3SuggestedUpdate {
7074
let Airflow3SuggestedUpdate { replacement, .. } = self;
7175
match replacement {
7276
Replacement::None => None,
73-
Replacement::AttrName(name) => Some(format!("Use `{name}` instead")),
7477
Replacement::Message(message) => Some((*message).to_string()),
78+
Replacement::AttrName(name) => Some(format!("Use `{name}` instead")),
79+
Replacement::AttrNameWithMessage { attr_name, message } => {
80+
Some(format!("Use `{attr_name}` instead; {message}"))
81+
}
7582
Replacement::Rename { module, name } => {
7683
Some(format!("Use `{name}` from `{module}` instead."))
7784
}

crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR301_AIR301_class_attribute.py.snap

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -631,8 +631,43 @@ AIR301 [*] `get_connections` is removed in Airflow 3.0
631631
| ^^^^^^^^^^^^^^^
632632
|
633633
help: Use `get_connection` instead
634-
95 |
635-
96 | # airflow.secrets.local_filesystem
636-
97 | lfb = LocalFilesystemBackend()
637-
- lfb.get_connections()
638-
98 + lfb.get_connection()
634+
95 |
635+
96 | # airflow.secrets.local_filesystem
636+
97 | lfb = LocalFilesystemBackend()
637+
- lfb.get_connections()
638+
98 + lfb.get_connection()
639+
99 |
640+
100 |
641+
101 | hlc.create_asset("there")
642+
643+
AIR301 `create_asset` is removed in Airflow 3.0
644+
--> AIR301_class_attribute.py:101:5
645+
|
646+
101 | hlc.create_asset("there")
647+
| ^^^^^^^^^^^^
648+
102 | hlc.create_asset("should", "be", "no", "posarg")
649+
103 | hlc.create_asset(name="but", uri="kwargs are ok")
650+
|
651+
help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error
652+
653+
AIR301 `create_asset` is removed in Airflow 3.0
654+
--> AIR301_class_attribute.py:102:5
655+
|
656+
101 | hlc.create_asset("there")
657+
102 | hlc.create_asset("should", "be", "no", "posarg")
658+
| ^^^^^^^^^^^^
659+
103 | hlc.create_asset(name="but", uri="kwargs are ok")
660+
104 | hlc.create_asset()
661+
|
662+
help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error
663+
664+
AIR301 `create_asset` is removed in Airflow 3.0
665+
--> AIR301_class_attribute.py:103:5
666+
|
667+
101 | hlc.create_asset("there")
668+
102 | hlc.create_asset("should", "be", "no", "posarg")
669+
103 | hlc.create_asset(name="but", uri="kwargs are ok")
670+
| ^^^^^^^^^^^^
671+
104 | hlc.create_asset()
672+
|
673+
help: Calling ``HookLineageCollector.create_asset`` with positional argument should raise an error

0 commit comments

Comments
 (0)