Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement Pylint typevar-name-incorrect-variance (C0105) #5651

Merged
merged 8 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from typing import ParamSpec, TypeVar

# Errors.

T = TypeVar("T", covariant=True)
T = TypeVar("T", covariant=True, contravariant=False)
T = TypeVar("T", contravariant=True)
T = TypeVar("T", covariant=False, contravariant=True)
P = ParamSpec("P", covariant=True)
P = ParamSpec("P", covariant=True, contravariant=False)
P = ParamSpec("P", contravariant=True)
P = ParamSpec("P", covariant=False, contravariant=True)

T_co = TypeVar("T_co")
T_co = TypeVar("T_co", covariant=False)
T_co = TypeVar("T_co", contravariant=False)
T_co = TypeVar("T_co", covariant=False, contravariant=False)
T_co = TypeVar("T_co", contravariant=True)
T_co = TypeVar("T_co", covariant=False, contravariant=True)
P_co = ParamSpec("P_co")
P_co = ParamSpec("P_co", covariant=False)
P_co = ParamSpec("P_co", contravariant=False)
P_co = ParamSpec("P_co", covariant=False, contravariant=False)
P_co = ParamSpec("P_co", contravariant=True)
P_co = ParamSpec("P_co", covariant=False, contravariant=True)

T_contra = TypeVar("T_contra")
T_contra = TypeVar("T_contra", covariant=False)
T_contra = TypeVar("T_contra", contravariant=False)
T_contra = TypeVar("T_contra", covariant=False, contravariant=False)
T_contra = TypeVar("T_contra", covariant=True)
T_contra = TypeVar("T_contra", covariant=True, contravariant=False)
P_contra = ParamSpec("P_contra")
P_contra = ParamSpec("P_contra", covariant=False)
P_contra = ParamSpec("P_contra", contravariant=False)
P_contra = ParamSpec("P_contra", covariant=False, contravariant=False)
P_contra = ParamSpec("P_contra", covariant=True)
P_contra = ParamSpec("P_contra", covariant=True, contravariant=False)

# Non-errors.

T = TypeVar("T")
T = TypeVar("T", covariant=False)
T = TypeVar("T", contravariant=False)
T = TypeVar("T", covariant=False, contravariant=False)
P = ParamSpec("P")
P = ParamSpec("P", covariant=False)
P = ParamSpec("P", contravariant=False)
P = ParamSpec("P", covariant=False, contravariant=False)

T_co = TypeVar("T_co", covariant=True)
T_co = TypeVar("T_co", covariant=True, contravariant=False)
P_co = ParamSpec("P_co", covariant=True)
P_co = ParamSpec("P_co", covariant=True, contravariant=False)

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)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,9 @@ where
if self.settings.rules.enabled(Rule::TypeParamNameMismatch) {
pylint::rules::type_param_name_mismatch(self, value, targets);
}
if self.settings.rules.enabled(Rule::TypeNameIncorrectVariance) {
pylint::rules::type_name_incorrect_variance(self, value);
}
if self.settings.rules.enabled(Rule::TypeBivariance) {
pylint::rules::type_bivariance(self, value);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pyflakes, "901") => (RuleGroup::Unspecified, rules::pyflakes::rules::RaiseNotImplemented),

// pylint
(Pylint, "C0105") => (RuleGroup::Unspecified, rules::pylint::rules::TypeNameIncorrectVariance),
(Pylint, "C0131") => (RuleGroup::Unspecified, rules::pylint::rules::TypeBivariance),
(Pylint, "C0132") => (RuleGroup::Unspecified, rules::pylint::rules::TypeParamNameMismatch),
(Pylint, "C0205") => (RuleGroup::Unspecified, rules::pylint::rules::SingleStringSlots),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ mod tests {
)]
#[test_case(Rule::TooManyStatements, Path::new("too_many_statements.py"))]
#[test_case(Rule::TypeBivariance, Path::new("type_bivariance.py"))]
#[test_case(
Rule::TypeNameIncorrectVariance,
Path::new("type_name_incorrect_variance.py")
)]
#[test_case(Rule::TypeParamNameMismatch, Path::new("type_param_name_mismatch.py"))]
#[test_case(
Rule::UnexpectedSpecialMethodSignature,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub(crate) use too_many_branches::*;
pub(crate) use too_many_return_statements::*;
pub(crate) use too_many_statements::*;
pub(crate) use type_bivariance::*;
pub(crate) use type_name_incorrect_variance::*;
pub(crate) use type_param_name_mismatch::*;
pub(crate) use unexpected_special_method_signature::*;
pub(crate) use unnecessary_direct_lambda_call::*;
Expand Down Expand Up @@ -87,6 +88,7 @@ mod too_many_branches;
mod too_many_return_statements;
mod too_many_statements;
mod type_bivariance;
mod type_name_incorrect_variance;
mod type_param_name_mismatch;
mod unexpected_special_method_signature;
mod unnecessary_direct_lambda_call;
Expand Down
154 changes: 154 additions & 0 deletions crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
use std::fmt;

use rustpython_parser::ast::{self, Expr, Ranged};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_true;

use crate::checkers::ast::Checker;
use crate::rules::pylint::helpers::type_param_name;

/// ## What it does
/// Checks for type names that do not match the variance of their associated
/// type parameter.
///
/// ## Why is this bad?
/// [PEP 484] recommends the use of the `_co` and `_contra` suffixes for
/// covariant and contravariant type parameters, respectively (while invariant
/// type parameters should not have any such suffix).
///
/// ## Example
/// ```python
/// from typing import TypeVar
///
/// T = TypeVar("T", covariant=True)
/// U = TypeVar("U", contravariant=True)
/// V_co = TypeVar("V_co")
/// ```
///
/// Use instead:
/// ```python
/// from typing import TypeVar
///
/// T_co = TypeVar("T_co", covariant=True)
/// U_contra = TypeVar("U_contra", contravariant=True)
/// V = TypeVar("V")
/// ```
///
/// ## References
/// - [Python documentation: `typing` — Support for type hints](https://docs.python.org/3/library/typing.html)
/// - [PEP 483 – The Theory of Type Hints: Covariance and Contravariance](https://peps.python.org/pep-0483/#covariance-and-contravariance)
/// - [PEP 484 – Type Hints: Covariance and contravariance](https://peps.python.org/pep-0484/#covariance-and-contravariance)
///
/// [PEP 484]: https://www.python.org/dev/peps/pep-0484/
#[violation]
pub struct TypeNameIncorrectVariance {
kind: VarKind,
param_name: String,
}

impl Violation for TypeNameIncorrectVariance {
#[derive_message_formats]
fn message(&self) -> String {
let TypeNameIncorrectVariance { kind, param_name } = self;
format!("`{kind}` name \"{param_name}\" does not match variance")
}
}

/// PLC0105
pub(crate) fn type_name_incorrect_variance(checker: &mut Checker, value: &Expr) {
let Expr::Call(ast::ExprCall {
func,
args,
keywords,
..
}) = value
else {
return;
};

let Some(param_name) = type_param_name(args, keywords) else {
return;
};

let covariant = keywords
.iter()
.find(|keyword| {
keyword
.arg
.as_ref()
.map_or(false, |keyword| keyword.as_str() == "covariant")
})
.map(|keyword| &keyword.value);

let contravariant = keywords
.iter()
.find(|keyword| {
keyword
.arg
.as_ref()
.map_or(false, |keyword| keyword.as_str() == "contravariant")
})
.map(|keyword| &keyword.value);

if !mismatch(param_name, covariant, contravariant) {
return;
}

let Some(kind) = checker
.semantic()
.resolve_call_path(func)
.and_then(|call_path| {
if checker
.semantic()
.match_typing_call_path(&call_path, "ParamSpec")
{
Some(VarKind::ParamSpec)
} else if checker
.semantic()
.match_typing_call_path(&call_path, "TypeVar")
{
Some(VarKind::TypeVar)
} else {
None
}
})
else {
return;
};

checker.diagnostics.push(Diagnostic::new(
TypeNameIncorrectVariance {
kind,
param_name: param_name.to_string(),
},
func.range(),
));
}

/// Returns `true` if the parameter name does not match its type variance.
fn mismatch(param_name: &str, covariant: Option<&Expr>, contravariant: Option<&Expr>) -> bool {
if param_name.ends_with("_co") {
covariant.map_or(true, |covariant| !is_const_true(covariant))
} else if param_name.ends_with("_contra") {
contravariant.map_or(true, |contravariant| !is_const_true(contravariant))
} else {
covariant.map_or(false, is_const_true) || contravariant.map_or(false, is_const_true)
}
}

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum VarKind {
TypeVar,
ParamSpec,
}

impl fmt::Display for VarKind {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
VarKind::TypeVar => fmt.write_str("TypeVar"),
VarKind::ParamSpec => fmt.write_str("ParamSpec"),
}
}
}
Loading