From 8e4b65eb5f584a3c85fb84e50a25630e773452a8 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Wed, 4 Dec 2024 12:59:48 +0800 Subject: [PATCH] feat(AIR303): initial air303 --- .../resources/test/fixtures/airflow/AIR303.py | 3 + .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/airflow/mod.rs | 1 + .../src/rules/airflow/rules/mod.rs | 2 + .../airflow/rules/moved_to_provider_in_3.rs | 98 +++++++++++++++++++ ...les__airflow__tests__AIR303_AIR303.py.snap | 11 +++ ruff.schema.json | 1 + 8 files changed, 120 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py create mode 100644 crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs create mode 100644 crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py b/crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py new file mode 100644 index 0000000000000..edc16b0e25e55 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/airflow/AIR303.py @@ -0,0 +1,3 @@ +from airflow.www.security import FabAirflowSecurityManagerOverride + +FabAirflowSecurityManagerOverride diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index b86389c74d8d4..9b640284ace14 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -223,6 +223,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::Airflow3Removal) { airflow::rules::removed_in_3(checker, expr); } + if checker.enabled(Rule::Airflow3MoveToProvider) { + airflow::rules::moved_to_provider_in_3(checker, expr); + } // Ex) List[...] if checker.any_enabled(&[ diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 7a87494341276..708a6bd1214fb 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1044,6 +1044,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Airflow, "001") => (RuleGroup::Stable, rules::airflow::rules::AirflowVariableNameTaskIdMismatch), (Airflow, "301") => (RuleGroup::Preview, rules::airflow::rules::AirflowDagNoScheduleArgument), (Airflow, "302") => (RuleGroup::Preview, rules::airflow::rules::Airflow3Removal), + (Airflow, "303") => (RuleGroup::Preview, rules::airflow::rules::Airflow3MoveToProvider), // perflint (Perflint, "101") => (RuleGroup::Stable, rules::perflint::rules::UnnecessaryListCast), diff --git a/crates/ruff_linter/src/rules/airflow/mod.rs b/crates/ruff_linter/src/rules/airflow/mod.rs index 1869b0888b445..3450ebbd7f218 100644 --- a/crates/ruff_linter/src/rules/airflow/mod.rs +++ b/crates/ruff_linter/src/rules/airflow/mod.rs @@ -16,6 +16,7 @@ mod tests { #[test_case(Rule::AirflowDagNoScheduleArgument, Path::new("AIR301.py"))] #[test_case(Rule::Airflow3Removal, Path::new("AIR302_args.py"))] #[test_case(Rule::Airflow3Removal, Path::new("AIR302_names.py"))] + #[test_case(Rule::Airflow3MoveToProvider, Path::new("AIR303.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/airflow/rules/mod.rs b/crates/ruff_linter/src/rules/airflow/rules/mod.rs index e5d1c83fb6dd8..033395cb751d8 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/mod.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/mod.rs @@ -1,7 +1,9 @@ pub(crate) use dag_schedule_argument::*; +pub(crate) use moved_to_provider_in_3::*; pub(crate) use removal_in_3::*; pub(crate) use task_variable_name::*; mod dag_schedule_argument; +mod moved_to_provider_in_3; mod removal_in_3; mod task_variable_name; diff --git a/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs new file mode 100644 index 0000000000000..6044f0d9d831b --- /dev/null +++ b/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs @@ -0,0 +1,98 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::{Expr, ExprAttribute}; +use ruff_python_semantic::Modules; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +#[derive(Debug, Eq, PartialEq)] +enum Replacement { + Message(String, String, String), +} + +/// ## What it does +/// Checks for uses of deprecated Airflow functions and values. +/// +/// ## Why is this bad? +/// Airflow 3.0 removed various deprecated functions, members, and other +/// values. Some have more modern replacements. Others are considered too niche +/// and not worth to be maintained in Airflow. +/// +/// ## Example +/// ```python +/// from airflow.utils.dates import days_ago +/// +/// +/// yesterday = days_ago(today, 1) +/// ``` +/// +/// Use instead: +/// ```python +/// from datetime import timedelta +/// +/// +/// yesterday = today - timedelta(days=1) +/// ``` +#[derive(ViolationMetadata)] +pub(crate) struct Airflow3MoveToProvider { + deprecated: String, + replacement: Replacement, +} + +impl Violation for Airflow3MoveToProvider { + #[derive_message_formats] + fn message(&self) -> String { + let Airflow3MoveToProvider { + deprecated, + replacement, + } = self; + match replacement { + Replacement::Message(name, provider, provider_version) => { + format!("`{deprecated}` is removed in Airflow 3.0; use `{name}` instead (in `{provider}=={provider_version}`)") + } + } + } +} + +fn moved_to_provider(checker: &mut Checker, expr: &Expr, ranged: impl Ranged) { + let result = + checker + .semantic() + .resolve_qualified_name(expr) + .and_then(|qualname| match qualname.segments() { + ["airflow", "www", "security", "FabAirflowSecurityManagerOverride"] => Some(( + qualname.to_string(), + Replacement::Message( + "airflow.providers.fab.auth_manager.security_manager.override.FabAirflowSecurityManagerOverride".to_string(), + "apache-airflow-providers-fab".to_string(), + "1.0.0".to_string() + ), + )), + _ => None, + }); + if let Some((deprecated, replacement)) = result { + checker.diagnostics.push(Diagnostic::new( + Airflow3MoveToProvider { + deprecated, + replacement, + }, + ranged.range(), + )); + } +} + +/// AIR303 +pub(crate) fn moved_to_provider_in_3(checker: &mut Checker, expr: &Expr) { + if !checker.semantic().seen_module(Modules::AIRFLOW) { + return; + } + + match expr { + Expr::Attribute(ExprAttribute { attr: ranged, .. }) => { + moved_to_provider(checker, expr, ranged); + } + ranged @ Expr::Name(_) => moved_to_provider(checker, expr, ranged), + _ => {} + } +} diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303.py.snap new file mode 100644 index 0000000000000..2993e8ed68daf --- /dev/null +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR303_AIR303.py.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff_linter/src/rules/airflow/mod.rs +snapshot_kind: text +--- +AIR303.py:3:1: AIR303 `airflow.www.security.FabAirflowSecurityManagerOverride` is removed in Airflow 3.0; use `airflow.providers.fab.auth_manager.security_manager.override.FabAirflowSecurityManagerOverride` instead (in `apache-airflow-providers-fab==1.0.0`) + | +1 | from airflow.www.security import FabAirflowSecurityManagerOverride +2 | +3 | FabAirflowSecurityManagerOverride + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AIR303 + | diff --git a/ruff.schema.json b/ruff.schema.json index 85e2c08b914cd..7c75bb14d9e6d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2798,6 +2798,7 @@ "AIR30", "AIR301", "AIR302", + "AIR303", "ALL", "ANN", "ANN0",