From b8a6ce43a27e9b79159f214e49cb7aee4bed4c6f Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Mon, 10 Jul 2023 19:19:17 +0100 Subject: [PATCH] Properly ignore bivariate types in `type-name-incorrect-variance` (#5660) ## Summary #5658 didn't actually ignore bivariate types in some all cases (sorry about that). This PR fixes that and adds bivariate types to the test fixture. ## Test Plan `cargo test` --- .../test/fixtures/pylint/type_name_incorrect_variance.py | 9 +++++++++ .../rules/pylint/rules/type_name_incorrect_variance.rs | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/crates/ruff/resources/test/fixtures/pylint/type_name_incorrect_variance.py b/crates/ruff/resources/test/fixtures/pylint/type_name_incorrect_variance.py index 33f68d2a04..f000a2ba0b 100644 --- a/crates/ruff/resources/test/fixtures/pylint/type_name_incorrect_variance.py +++ b/crates/ruff/resources/test/fixtures/pylint/type_name_incorrect_variance.py @@ -57,3 +57,12 @@ T_contra = TypeVar("T_contra", contravariant=True) T_contra = TypeVar("T_contra", covariant=False, contravariant=True) P_contra = ParamSpec("P_contra", contravariant=True) P_contra = ParamSpec("P_contra", covariant=False, contravariant=True) + +# Bivariate types are errors, but not covered by this check. + +T = TypeVar("T", covariant=True, contravariant=True) +P = ParamSpec("P", covariant=True, contravariant=True) +T_co = TypeVar("T_co", covariant=True, contravariant=True) +P_co = ParamSpec("P_co", covariant=True, contravariant=True) +T_contra = TypeVar("T_contra", covariant=True, contravariant=True) +P_contra = ParamSpec("P_contra", covariant=True, contravariant=True) diff --git a/crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs b/crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs index 9af30526ae..8683e09d15 100644 --- a/crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs +++ b/crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs @@ -130,6 +130,7 @@ pub(crate) fn type_name_incorrect_variance(checker: &mut Checker, value: &Expr) .trim_end_matches("_co") .trim_end_matches("_contra"); let replacement_name: String = match variance { + VarVariance::Bivariance => return, // Bivariate type are invalid, so ignore them for this rule. VarVariance::Covariance => format!("{name_root}_co"), VarVariance::Contravariance => format!("{name_root}_contra"), VarVariance::Invariance => name_root.to_string(), @@ -163,6 +164,7 @@ fn variance(covariant: Option<&Expr>, contravariant: Option<&Expr>) -> VarVarian covariant.map(is_const_true), contravariant.map(is_const_true), ) { + (Some(true), Some(true)) => VarVariance::Bivariance, (Some(true), _) => VarVariance::Covariance, (_, Some(true)) => VarVariance::Contravariance, _ => VarVariance::Invariance, @@ -186,6 +188,7 @@ impl fmt::Display for VarKind { #[derive(Debug, PartialEq, Eq, Copy, Clone)] enum VarVariance { + Bivariance, Covariance, Contravariance, Invariance, @@ -194,6 +197,7 @@ enum VarVariance { impl fmt::Display for VarVariance { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match self { + VarVariance::Bivariance => fmt.write_str("bivariance"), VarVariance::Covariance => fmt.write_str("covariance"), VarVariance::Contravariance => fmt.write_str("contravariance"), VarVariance::Invariance => fmt.write_str("invariance"),