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

[pyupgrade] Don't introduce invalid syntax when upgrading old-style type aliases with parenthesized multiline values (UP040) #16026

Merged
merged 3 commits into from
Feb 7, 2025
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
15 changes: 15 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,18 @@ class Foo:
Annotated[T, Gt(0)], # preserved comment
], type_params=(T,)
)

T: TypeAlias = (
int
| str
)
Copy link
Contributor

@InSyncWithFoo InSyncWithFoo Feb 7, 2025

Choose a reason for hiding this comment

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

This might be a good test case to add:

T: TypeAlias = (  # This comment should not make the fix unsafe
	int | str
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! However, all fixes for this rule are unsafe (because you can't use PEP-695 type aliases as the second argument in isinstance() calls, whereas you can with old-style type aliases that use TypeAlias)

Copy link
Contributor

Choose a reason for hiding this comment

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

However, all fixes for this rule are unsafe

Aren't they safe in stubs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Aren't they safe in stubs?

hmm, good point. Want to make a followup PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. I'm ready whenever you are.


T: TypeAlias = ( # comment0
# comment1
int # comment2
# comment3
| # comment4
# comment5
str # comment6
# comment7
) # comment8
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use itertools::Itertools;
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::name::Name;
use ruff_python_ast::{
visitor::Visitor, Expr, ExprCall, ExprName, Keyword, StmtAnnAssign, StmtAssign,
};
use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{Expr, ExprCall, ExprName, Keyword, StmtAnnAssign, StmtAssign, StmtRef};
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -183,8 +183,8 @@ pub(crate) fn non_pep695_type_alias_type(checker: &Checker, stmt: &StmtAssign) {
};

checker.report_diagnostic(create_diagnostic(
checker.source(),
stmt.range,
checker,
stmt.into(),
&target_name.id,
value,
&vars,
Expand Down Expand Up @@ -243,8 +243,8 @@ pub(crate) fn non_pep695_type_alias(checker: &Checker, stmt: &StmtAnnAssign) {
}

checker.report_diagnostic(create_diagnostic(
checker.source(),
stmt.range(),
checker,
stmt.into(),
name,
value,
&vars,
Expand All @@ -261,26 +261,30 @@ pub(crate) fn non_pep695_type_alias(checker: &Checker, stmt: &StmtAnnAssign) {

/// Generate a [`Diagnostic`] for a non-PEP 695 type alias or type alias type.
fn create_diagnostic(
source: &str,
stmt_range: TextRange,
checker: &Checker,
stmt: StmtRef,
name: &Name,
value: &Expr,
type_vars: &[TypeVar],
applicability: Applicability,
type_alias_kind: TypeAliasKind,
) -> Diagnostic {
let source = checker.source();
let range_with_parentheses =
parenthesized_range(value.into(), stmt.into(), checker.comment_ranges(), source)
.unwrap_or(value.range());
let content = format!(
"type {name}{type_params} = {value}",
type_params = DisplayTypeVars { type_vars, source },
value = &source[value.range()]
value = &source[range_with_parentheses]
);
let edit = Edit::range_replacement(content, stmt_range);
let edit = Edit::range_replacement(content, stmt.range());
Diagnostic::new(
NonPEP695TypeAlias {
name: name.to_string(),
type_alias_kind,
},
stmt_range,
stmt.range(),
)
.with_fix(Fix::applicable_edit(edit, applicability))
}
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,8 @@ UP040.py:111:1: UP040 [*] Type alias `PositiveList` uses `TypeAliasType` assignm
114 | | ], type_params=(T,)
115 | | )
| |_^ UP040
116 |
117 | T: TypeAlias = (
|
= help: Use the `type` keyword

Expand All @@ -431,3 +433,57 @@ UP040.py:111:1: UP040 [*] Type alias `PositiveList` uses `TypeAliasType` assignm
114 |- ], type_params=(T,)
115 |-)
113 |+ ]
116 114 |
117 115 | T: TypeAlias = (
118 116 | int

UP040.py:117:1: UP040 [*] Type alias `T` uses `TypeAlias` annotation instead of the `type` keyword
|
115 | )
116 |
117 | / T: TypeAlias = (
118 | | int
119 | | | str
120 | | )
| |_^ UP040
121 |
122 | T: TypeAlias = ( # comment0
|
= help: Use the `type` keyword

ℹ Unsafe fix
114 114 | ], type_params=(T,)
115 115 | )
116 116 |
117 |-T: TypeAlias = (
117 |+type T = (
118 118 | int
119 119 | | str
120 120 | )

UP040.py:122:1: UP040 [*] Type alias `T` uses `TypeAlias` annotation instead of the `type` keyword
|
120 | )
121 |
122 | / T: TypeAlias = ( # comment0
123 | | # comment1
124 | | int # comment2
125 | | # comment3
126 | | | # comment4
127 | | # comment5
128 | | str # comment6
129 | | # comment7
130 | | ) # comment8
| |_^ UP040
|
= help: Use the `type` keyword

ℹ Unsafe fix
119 119 | | str
120 120 | )
121 121 |
122 |-T: TypeAlias = ( # comment0
122 |+type T = ( # comment0
123 123 | # comment1
124 124 | int # comment2
125 125 | # comment3
Loading