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.

Sign up for GitHub

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

Jump to bottom

Use typing_extensions.TypeAlias for PYI026 fixes on pre-3.10 #6696

Merged
merged 1 commit into from
Aug 19, 2023
Merged

Use typing_extensions.TypeAlias for PYI026 fixes on pre-3.10 #6696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 32 additions & 30 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,46 +30,50 @@ mod tests {
#[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.pyi"))]
#[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))]
#[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))]
#[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.py"))]
#[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.pyi"))]
#[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))]
#[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))]
#[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))]
#[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))]
#[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.py"))]
#[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.pyi"))]
#[test_case(Rule::NonSelfReturnType, Path::new("PYI034.py"))]
#[test_case(Rule::NonSelfReturnType, Path::new("PYI034.pyi"))]
#[test_case(Rule::FutureAnnotationsInStub, Path::new("PYI044.py"))]
#[test_case(Rule::FutureAnnotationsInStub, Path::new("PYI044.pyi"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))]
#[test_case(Rule::NoReturnArgumentAnnotationInStub, Path::new("PYI050.py"))]
#[test_case(Rule::NoReturnArgumentAnnotationInStub, Path::new("PYI050.pyi"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.py"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.pyi"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.py"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.pyi"))]
#[test_case(Rule::NonSelfReturnType, Path::new("PYI034.py"))]
#[test_case(Rule::NonSelfReturnType, Path::new("PYI034.pyi"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.py"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.pyi"))]
#[test_case(Rule::PassInClassBody, Path::new("PYI012.py"))]
#[test_case(Rule::PassInClassBody, Path::new("PYI012.pyi"))]
#[test_case(Rule::PassStatementStubBody, Path::new("PYI009.py"))]
#[test_case(Rule::PassStatementStubBody, Path::new("PYI009.pyi"))]
#[test_case(Rule::PatchVersionComparison, Path::new("PYI004.py"))]
#[test_case(Rule::PatchVersionComparison, Path::new("PYI004.pyi"))]
#[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.py"))]
#[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.pyi"))]
#[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.py"))]
#[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.pyi"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.py"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.pyi"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.py"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.pyi"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.py"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.pyi"))]
#[test_case(Rule::StrOrReprDefinedInStub, Path::new("PYI029.py"))]
#[test_case(Rule::StrOrReprDefinedInStub, Path::new("PYI029.pyi"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.py"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.pyi"))]
#[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.py"))]
#[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.pyi"))]
#[test_case(Rule::StubBodyMultipleStatements, Path::new("PYI048.py"))]
#[test_case(Rule::StubBodyMultipleStatements, Path::new("PYI048.pyi"))]
#[test_case(Rule::TSuffixedTypeAlias, Path::new("PYI043.py"))]
#[test_case(Rule::TSuffixedTypeAlias, Path::new("PYI043.pyi"))]
#[test_case(Rule::FutureAnnotationsInStub, Path::new("PYI044.py"))]
#[test_case(Rule::FutureAnnotationsInStub, Path::new("PYI044.pyi"))]
#[test_case(Rule::PatchVersionComparison, Path::new("PYI004.py"))]
#[test_case(Rule::PatchVersionComparison, Path::new("PYI004.pyi"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.py"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))]
#[test_case(Rule::TypeCommentInStub, Path::new("PYI033.py"))]
#[test_case(Rule::TypeCommentInStub, Path::new("PYI033.pyi"))]
#[test_case(Rule::TypedArgumentDefaultInStub, Path::new("PYI011.py"))]
Expand All @@ -78,8 +82,12 @@ mod tests {
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025.pyi"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.py"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.pyi"))]
#[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.py"))]
#[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.pyi"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.py"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.pyi"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.py"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.pyi"))]
#[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.py"))]
#[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.pyi"))]
#[test_case(Rule::UnprefixedTypeParam, Path::new("PYI001.py"))]
#[test_case(Rule::UnprefixedTypeParam, Path::new("PYI001.pyi"))]
#[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.py"))]
Expand All @@ -88,24 +96,18 @@ mod tests {
#[test_case(Rule::UnrecognizedPlatformName, Path::new("PYI008.pyi"))]
#[test_case(Rule::UnrecognizedVersionInfoCheck, Path::new("PYI003.py"))]
#[test_case(Rule::UnrecognizedVersionInfoCheck, Path::new("PYI003.pyi"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.py"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.py"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))]
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))]
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))]
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))]
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.pyi"))]
#[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.py"))]
#[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.pyi"))]
#[test_case(Rule::UnusedPrivateTypeAlias, Path::new("PYI047.py"))]
#[test_case(Rule::UnusedPrivateTypeAlias, Path::new("PYI047.pyi"))]
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))]
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.pyi"))]
#[test_case(Rule::UnusedPrivateTypedDict, Path::new("PYI049.py"))]
#[test_case(Rule::UnusedPrivateTypedDict, Path::new("PYI049.pyi"))]
#[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.py"))]
#[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.pyi"))]
#[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.py"))]
#[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.pyi"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.py"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand All @@ -116,15 +118,15 @@ mod tests {
Ok(())
}

#[test_case(Path::new("PYI019.py"))]
#[test_case(Path::new("PYI019.pyi"))]
fn custom_type_var_return_type(path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", "PYI019", path.to_string_lossy());
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.py"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))]
fn py38(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("py38_{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("flake8_pyi").join(path).as_path(),
&settings::Settings {
target_version: PythonVersion::Py312,
..settings::Settings::for_rules(vec![Rule::CustomTypeVarReturnType])
target_version: PythonVersion::Py38,
..settings::Settings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Expand Down
24 changes: 24 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub(crate) use quoted_annotation_in_stub::*;
pub(crate) use redundant_literal_union::*;
pub(crate) use redundant_numeric_union::*;
pub(crate) use simple_defaults::*;
use std::fmt;
pub(crate) use str_or_repr_defined_in_stub::*;
pub(crate) use string_or_bytes_too_long::*;
pub(crate) use stub_body_multiple_statements::*;
Expand Down Expand Up @@ -69,3 +70,26 @@ mod unrecognized_platform;
mod unrecognized_version_info;
mod unsupported_method_call_on_all;
mod unused_private_type_definition;

// TODO(charlie): Replace this with a common utility for selecting the appropriate source
// module for a given `typing` member.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum TypingModule {
Typing,
TypingExtensions,
}

impl TypingModule {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be more useful to generalize this into a generic backport import helper.

IE. something that would automatically downgrade typing.TypeAlias. Alternatively, I wonder if you could use the pyupgrade import rules logic here and always apply that rule, but only to this specific import. It would either be a NOOP or automatically fix the import if the import was moved to the typing module.

That also has the added benefit of only having to maintain the version when a new import was introduced in one place (the pyupgrade rules).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic could even be folded into the helper which inserts the import if you do it pyupgrade way.

Copy link
Contributor

@Skylion007 Skylion007 Aug 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Note that, in some cases, it may be preferable to continue importing members from typing_extensions even after they're added to the Python standard library, as typing_extensions can backport bugfixes and optimizations from later Python versions. This rule thus avoids flagging imports from typing_extensions in such cases.

Seems like more reason to use the helper from this rule here: https://beta.ruff.rs/docs/rules/deprecated-import/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea, I will look into it separately. There are a few other places where this would be useful, like PYI019 which suggests typing.Self.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup agreed. (I did check if that applied to TypeAlias and it does not.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a TODO, I may look into it tonight.

fn as_str(self) -> &'static str {
match self {
TypingModule::Typing => "typing",
TypingModule::TypingExtensions => "typing_extensions",
}
}
}

impl fmt::Display for TypingModule {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
fmt.write_str(self.as_str())
}
}
31 changes: 22 additions & 9 deletions crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::CallPath;
use ruff_python_ast::{
self as ast, Arguments, Constant, Expr, Operator, ParameterWithDefault, Parameters, Ranged,
Stmt, UnaryOp,
};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::CallPath;
use ruff_python_semantic::{ScopeKind, SemanticModel};
use ruff_source_file::Locator;

use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::registry::AsRule;
use crate::rules::flake8_pyi::rules::TypingModule;
use crate::settings::types::PythonVersion;

#[violation]
pub struct TypedArgumentDefaultInStub;
Expand Down Expand Up @@ -124,19 +125,24 @@ impl Violation for UnassignedSpecialVariableInStub {
/// ```
#[violation]
pub struct TypeAliasWithoutAnnotation {
module: TypingModule,
name: String,
value: String,
}

impl AlwaysAutofixableViolation for TypeAliasWithoutAnnotation {
#[derive_message_formats]
fn message(&self) -> String {
let TypeAliasWithoutAnnotation { name, value } = self;
format!("Use `typing.TypeAlias` for type alias, e.g., `{name}: typing.TypeAlias = {value}`")
let TypeAliasWithoutAnnotation {
module,
name,
value,
} = self;
format!("Use `{module}.TypeAlias` for type alias, e.g., `{name}: TypeAlias = {value}`")
}

fn autofix_title(&self) -> String {
"Add `typing.TypeAlias` annotation".to_string()
"Add `TypeAlias` annotation".to_string()
}
}

Expand Down Expand Up @@ -606,7 +612,7 @@ pub(crate) fn unassigned_special_variable_in_stub(
));
}

/// PIY026
/// PYI026
pub(crate) fn type_alias_without_annotation(checker: &mut Checker, value: &Expr, targets: &[Expr]) {
let [target] = targets else {
return;
Expand All @@ -620,8 +626,15 @@ pub(crate) fn type_alias_without_annotation(checker: &mut Checker, value: &Expr,
return;
}

let module = if checker.settings.target_version >= PythonVersion::Py310 {
TypingModule::Typing
} else {
TypingModule::TypingExtensions
};

let mut diagnostic = Diagnostic::new(
TypeAliasWithoutAnnotation {
module,
name: id.to_string(),
value: checker.generator().expr(value),
},
Expand All @@ -630,7 +643,7 @@ pub(crate) fn type_alias_without_annotation(checker: &mut Checker, value: &Expr,
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("typing", "TypeAlias"),
&ImportRequest::import(module.as_str(), "TypeAlias"),
target.start(),
checker.semantic(),
)?;
Expand Down
20 changes: 10 additions & 10 deletions ...uff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI026_PYI026.pyi.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI026.pyi:3:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `NewAny: typing.TypeAlias = Any`
PYI026.pyi:3:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `NewAny: TypeAlias = Any`
|
1 | from typing import Literal, Any
2 |
Expand All @@ -10,7 +10,7 @@ PYI026.pyi:3:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `NewAny:
4 | OptionalStr = typing.Optional[str]
5 | Foo = Literal["foo"]
|
= help: Add `typing.TypeAlias` annotation
= help: Add `TypeAlias` annotation

ℹ Suggested fix
1 |-from typing import Literal, Any
Expand All @@ -22,15 +22,15 @@ PYI026.pyi:3:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `NewAny:
5 5 | Foo = Literal["foo"]
6 6 | IntOrStr = int | str

PYI026.pyi:4:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `OptionalStr: typing.TypeAlias = typing.Optional[str]`
PYI026.pyi:4:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `OptionalStr: TypeAlias = typing.Optional[str]`
|
3 | NewAny = Any
4 | OptionalStr = typing.Optional[str]
| ^^^^^^^^^^^ PYI026
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
|
= help: Add `typing.TypeAlias` annotation
= help: Add `TypeAlias` annotation

ℹ Suggested fix
1 |-from typing import Literal, Any
Expand All @@ -43,7 +43,7 @@ PYI026.pyi:4:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Optiona
6 6 | IntOrStr = int | str
7 7 | AliasNone = None

PYI026.pyi:5:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Foo: typing.TypeAlias = Literal["foo"]`
PYI026.pyi:5:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Foo: TypeAlias = Literal["foo"]`
|
3 | NewAny = Any
4 | OptionalStr = typing.Optional[str]
Expand All @@ -52,7 +52,7 @@ PYI026.pyi:5:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Foo: ty
6 | IntOrStr = int | str
7 | AliasNone = None
|
= help: Add `typing.TypeAlias` annotation
= help: Add `TypeAlias` annotation

ℹ Suggested fix
1 |-from typing import Literal, Any
Expand All @@ -66,15 +66,15 @@ PYI026.pyi:5:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Foo: ty
7 7 | AliasNone = None
8 8 |

PYI026.pyi:6:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `IntOrStr: typing.TypeAlias = int | str`
PYI026.pyi:6:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `IntOrStr: TypeAlias = int | str`
|
4 | OptionalStr = typing.Optional[str]
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
| ^^^^^^^^ PYI026
7 | AliasNone = None
|
= help: Add `typing.TypeAlias` annotation
= help: Add `TypeAlias` annotation

ℹ Suggested fix
1 |-from typing import Literal, Any
Expand All @@ -89,7 +89,7 @@ PYI026.pyi:6:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `IntOrSt
8 8 |
9 9 | NewAny: typing.TypeAlias = Any

PYI026.pyi:7:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `AliasNone: typing.TypeAlias = None`
PYI026.pyi:7:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `AliasNone: TypeAlias = None`
|
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
Expand All @@ -98,7 +98,7 @@ PYI026.pyi:7:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `AliasNo
8 |
9 | NewAny: typing.TypeAlias = Any
|
= help: Add `typing.TypeAlias` annotation
= help: Add `TypeAlias` annotation

ℹ Suggested fix
1 |-from typing import Literal, Any
Expand Down
4 changes: 4 additions & 0 deletions ...src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__py38_PYI026_PYI026.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

Loading
Loading