Enable autofix for annotations within 'simple' string literals (#3657)

This commit is contained in:
Charlie Marsh 2023-03-22 12:45:51 -04:00 committed by GitHub
parent 8593739f88
commit 3a8e98341b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 307 additions and 148 deletions

View file

@ -30,8 +30,17 @@ def f(x: "List[str]") -> None:
...
list = "abc"
def f(x: List[str]) -> None:
def f(x: r"List[str]") -> None:
...
def f(x: "List[str]") -> None:
...
def f(x: """List[str]""") -> None:
...
def f(x: "Li" "st[str]") -> None:
...

View file

@ -6,7 +6,6 @@ use log::error;
use nohash_hasher::IntMap;
use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_common::cformat::{CFormatError, CFormatErrorType};
use rustpython_parser as parser;
use rustpython_parser::ast::{
Arg, Arguments, Comprehension, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext,
ExprKind, KeywordData, Located, Location, Operator, Pattern, PatternKind, Stmt, StmtKind,
@ -19,14 +18,15 @@ use ruff_python_ast::helpers::{
binding_range, extract_handled_exceptions, to_module_path, Exceptions,
};
use ruff_python_ast::operations::{extract_all_names, AllNamesFlags};
use ruff_python_ast::relocate::relocate_expr;
use ruff_python_ast::scope::{
Binding, BindingId, BindingKind, ClassDef, ExecutionContext, FunctionDef, Lambda, Scope,
ScopeId, ScopeKind, ScopeStack,
};
use ruff_python_ast::source_code::{Indexer, Locator, Stylist};
use ruff_python_ast::types::{Node, Range, RefEquality};
use ruff_python_ast::typing::{match_annotated_subscript, Callable, SubscriptKind};
use ruff_python_ast::typing::{
match_annotated_subscript, parse_type_annotation, Callable, SubscriptKind,
};
use ruff_python_ast::visitor::{walk_excepthandler, walk_pattern, Visitor};
use ruff_python_ast::{
branch_detection, cast, helpers, operations, str, typing, visibility, visitor,
@ -2142,7 +2142,8 @@ where
}
fn visit_expr(&mut self, expr: &'b Expr) {
if !(self.ctx.in_deferred_type_definition || self.ctx.in_deferred_string_type_definition)
if !self.ctx.in_deferred_type_definition
&& self.ctx.in_deferred_string_type_definition.is_none()
&& self.ctx.in_type_definition
&& self.ctx.annotations_future_enabled
{
@ -4158,7 +4159,7 @@ impl<'a> Checker<'a> {
self.ctx.bindings[*index].mark_used(scope_id, Range::from(expr), context);
if self.ctx.bindings[*index].kind.is_annotation()
&& !self.ctx.in_deferred_string_type_definition
&& self.ctx.in_deferred_string_type_definition.is_none()
&& !self.ctx.in_deferred_type_definition
{
continue;
@ -4542,20 +4543,19 @@ impl<'a> Checker<'a> {
where
'b: 'a,
{
let mut stacks = vec![];
let mut stacks = Vec::with_capacity(self.deferred.string_type_definitions.len());
self.deferred.string_type_definitions.reverse();
while let Some((range, expression, (in_annotation, in_type_checking_block), deferral)) =
while let Some((range, value, (in_annotation, in_type_checking_block), deferral)) =
self.deferred.string_type_definitions.pop()
{
if let Ok(mut expr) = parser::parse_expression(expression, "<filename>") {
if let Ok((expr, kind)) = parse_type_annotation(value, range, self.locator) {
if in_annotation && self.ctx.annotations_future_enabled {
if self.settings.rules.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, expression, range);
pyupgrade::rules::quoted_annotation(self, value, range);
}
}
relocate_expr(&mut expr, range);
allocator.push(expr);
stacks.push(((in_annotation, in_type_checking_block), deferral));
stacks.push((kind, (in_annotation, in_type_checking_block), deferral));
} else {
if self
.settings
@ -4564,14 +4564,14 @@ impl<'a> Checker<'a> {
{
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::ForwardAnnotationSyntaxError {
body: expression.to_string(),
body: value.to_string(),
},
range,
));
}
}
}
for (expr, ((in_annotation, in_type_checking_block), (scopes, parents))) in
for (expr, (kind, (in_annotation, in_type_checking_block), (scopes, parents))) in
allocator.iter().zip(stacks)
{
self.ctx.scope_stack = scopes;
@ -4579,9 +4579,9 @@ impl<'a> Checker<'a> {
self.ctx.in_annotation = in_annotation;
self.ctx.in_type_checking_block = in_type_checking_block;
self.ctx.in_type_definition = true;
self.ctx.in_deferred_string_type_definition = true;
self.ctx.in_deferred_string_type_definition = Some(kind);
self.visit_expr(expr);
self.ctx.in_deferred_string_type_definition = false;
self.ctx.in_deferred_string_type_definition = None;
self.ctx.in_type_definition = false;
}
}

View file

@ -61,10 +61,10 @@ expression: diagnostics
fixable: false
location:
row: 58
column: 3
column: 4
end_location:
row: 58
column: 8
column: 7
fix: ~
parent: ~
- kind:
@ -139,10 +139,10 @@ expression: diagnostics
fixable: false
location:
row: 115
column: 8
column: 9
end_location:
row: 115
column: 23
column: 22
fix: ~
parent: ~
- kind:
@ -152,10 +152,10 @@ expression: diagnostics
fixable: false
location:
row: 123
column: 13
column: 14
end_location:
row: 123
column: 18
column: 17
fix: ~
parent: ~
- kind:
@ -165,10 +165,10 @@ expression: diagnostics
fixable: false
location:
row: 123
column: 20
column: 21
end_location:
row: 123
column: 25
column: 24
fix: ~
parent: ~

View file

@ -9,49 +9,49 @@ expression: diagnostics
fixable: false
location:
row: 11
column: 9
end_location:
row: 11
column: 16
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 18
column: 16
end_location:
row: 18
column: 23
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 24
column: 12
end_location:
row: 24
column: 19
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 30
column: 10
end_location:
row: 30
row: 11
column: 15
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 18
column: 17
end_location:
row: 18
column: 22
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 24
column: 13
end_location:
row: 24
column: 18
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 30
column: 11
end_location:
row: 30
column: 16
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 18
column: 26
column: 27
end_location:
row: 18
column: 30
column: 29
fix: ~
parent: ~
- kind:
@ -22,10 +22,10 @@ expression: diagnostics
fixable: false
location:
row: 23
column: 12
column: 13
end_location:
row: 23
column: 17
column: 16
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 20
column: 26
column: 27
end_location:
row: 20
column: 30
column: 29
fix: ~
parent: ~
- kind:
@ -22,10 +22,10 @@ expression: diagnostics
fixable: false
location:
row: 25
column: 12
column: 13
end_location:
row: 25
column: 17
column: 16
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 5
column: 11
column: 12
end_location:
row: 5
column: 18
column: 17
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 11
column: 8
column: 9
end_location:
row: 11
column: 13
column: 12
fix: ~
parent: ~
- kind:
@ -22,10 +22,10 @@ expression: diagnostics
fixable: false
location:
row: 11
column: 15
column: 16
end_location:
row: 11
column: 22
column: 21
fix: ~
parent: ~

View file

@ -9,25 +9,25 @@ expression: diagnostics
fixable: false
location:
row: 4
column: 9
column: 10
end_location:
row: 4
column: 15
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 9
column: 11
end_location:
row: 9
column: 16
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
suggestion: ~
fixable: false
location:
row: 9
column: 10
end_location:
row: 9
column: 17
fix: ~
parent: ~
- kind:
name: UndefinedName
body: "Undefined name `Model`"
@ -35,10 +35,10 @@ expression: diagnostics
fixable: false
location:
row: 14
column: 14
column: 15
end_location:
row: 14
column: 21
column: 20
fix: ~
parent: ~
- kind:
@ -48,10 +48,10 @@ expression: diagnostics
fixable: false
location:
row: 19
column: 30
column: 31
end_location:
row: 19
column: 37
column: 36
fix: ~
parent: ~
- kind:
@ -61,10 +61,10 @@ expression: diagnostics
fixable: false
location:
row: 24
column: 18
column: 19
end_location:
row: 24
column: 25
column: 24
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 5
column: 29
column: 30
end_location:
row: 5
column: 41
column: 40
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 11
column: 20
column: 21
end_location:
row: 11
column: 31
column: 30
fix: ~
parent: ~
- kind:
@ -22,10 +22,10 @@ expression: diagnostics
fixable: false
location:
row: 12
column: 25
column: 26
end_location:
row: 12
column: 36
column: 35
fix: ~
parent: ~
- kind:
@ -35,10 +35,10 @@ expression: diagnostics
fixable: false
location:
row: 13
column: 20
column: 21
end_location:
row: 13
column: 31
column: 30
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 6
column: 34
column: 35
end_location:
row: 6
column: 38
column: 37
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 7
column: 13
column: 14
end_location:
row: 7
column: 20
column: 19
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 26
column: 15
column: 16
end_location:
row: 26
column: 20
column: 19
fix: ~
parent: ~
- kind:
@ -22,10 +22,10 @@ expression: diagnostics
fixable: false
location:
row: 33
column: 15
column: 16
end_location:
row: 33
column: 20
column: 19
fix: ~
parent: ~

View file

@ -9,10 +9,10 @@ expression: diagnostics
fixable: false
location:
row: 26
column: 15
column: 16
end_location:
row: 26
column: 20
column: 19
fix: ~
parent: ~

View file

@ -3,6 +3,7 @@ use rustpython_parser::ast::Expr;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;
use ruff_python_ast::typing::AnnotationKind;
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
@ -40,7 +41,11 @@ pub fn use_pep585_annotation(checker: &mut Checker, expr: &Expr) {
.resolve_call_path(expr)
.and_then(|call_path| call_path.last().copied())
{
let fixable = !checker.ctx.in_deferred_string_type_definition;
let fixable = checker
.ctx
.in_deferred_string_type_definition
.as_ref()
.map_or(true, AnnotationKind::is_simple);
let mut diagnostic = Diagnostic::new(
NonPEP585Annotation {
name: binding.to_string(),

View file

@ -4,6 +4,7 @@ use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_expr;
use ruff_python_ast::types::Range;
use ruff_python_ast::typing::AnnotationKind;
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
@ -99,7 +100,11 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s
};
// Avoid fixing forward references.
let fixable = !checker.ctx.in_deferred_string_type_definition;
let fixable = checker
.ctx
.in_deferred_string_type_definition
.as_ref()
.map_or(true, AnnotationKind::is_simple);
match typing_member {
TypingMember::Optional => {

View file

@ -85,15 +85,22 @@ expression: diagnostics
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: ~
fixable: false
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 29
column: 9
column: 10
end_location:
row: 29
column: 20
fix: ~
column: 14
fix:
content: list
location:
row: 29
column: 10
end_location:
row: 29
column: 14
parent: ~
- kind:
name: NonPEP585Annotation
@ -101,11 +108,71 @@ expression: diagnostics
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 36
row: 33
column: 11
end_location:
row: 33
column: 15
fix:
content: list
location:
row: 33
column: 11
end_location:
row: 33
column: 15
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 37
column: 10
end_location:
row: 37
column: 14
fix:
content: list
location:
row: 37
column: 10
end_location:
row: 37
column: 14
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: "Replace `List` with `list`"
fixable: true
location:
row: 41
column: 12
end_location:
row: 41
column: 16
fix:
content: list
location:
row: 41
column: 12
end_location:
row: 41
column: 16
parent: ~
- kind:
name: NonPEP585Annotation
body: "Use `list` instead of `List` for type annotations"
suggestion: ~
fixable: false
location:
row: 45
column: 9
end_location:
row: 36
column: 13
row: 45
column: 23
fix: ~
parent: ~

View file

@ -145,41 +145,62 @@ expression: diagnostics
- kind:
name: NonPEP604Annotation
body: "Use `X | Y` for type annotations"
suggestion: ~
fixable: false
suggestion: "Convert to `X | Y`"
fixable: true
location:
row: 30
column: 9
column: 10
end_location:
row: 30
column: 47
fix: ~
column: 46
fix:
content: "str | int | Union[float, bytes]"
location:
row: 30
column: 10
end_location:
row: 30
column: 46
parent: ~
- kind:
name: NonPEP604Annotation
body: "Use `X | Y` for type annotations"
suggestion: ~
fixable: false
suggestion: "Convert to `X | Y`"
fixable: true
location:
row: 30
column: 9
column: 26
end_location:
row: 30
column: 47
fix: ~
column: 45
fix:
content: float | bytes
location:
row: 30
column: 26
end_location:
row: 30
column: 45
parent: ~
- kind:
name: NonPEP604Annotation
body: "Use `X | Y` for type annotations"
suggestion: ~
fixable: false
suggestion: "Convert to `X | Y`"
fixable: true
location:
row: 34
column: 9
column: 10
end_location:
row: 34
column: 33
fix: ~
column: 32
fix:
content: str | int
location:
row: 34
column: 10
end_location:
row: 34
column: 32
parent: ~
- kind:
name: NonPEP604Annotation

View file

@ -14,6 +14,7 @@ use crate::scope::{
ScopeStack, Scopes,
};
use crate::types::{CallPath, RefEquality};
use crate::typing::AnnotationKind;
use crate::visibility::{module_visibility, Modifier, VisibleScope};
#[allow(clippy::struct_excessive_bools)]
@ -41,7 +42,7 @@ pub struct Context<'a> {
pub visible_scope: VisibleScope,
pub in_annotation: bool,
pub in_type_definition: bool,
pub in_deferred_string_type_definition: bool,
pub in_deferred_string_type_definition: Option<AnnotationKind>,
pub in_deferred_type_definition: bool,
pub in_exception_handler: bool,
pub in_literal: bool,
@ -79,7 +80,7 @@ impl<'a> Context<'a> {
},
in_annotation: false,
in_type_definition: false,
in_deferred_string_type_definition: false,
in_deferred_string_type_definition: None,
in_deferred_type_definition: false,
in_exception_handler: false,
in_literal: false,
@ -311,7 +312,7 @@ impl<'a> Context<'a> {
pub const fn execution_context(&self) -> ExecutionContext {
if self.in_type_checking_block
|| self.in_annotation
|| self.in_deferred_string_type_definition
|| self.in_deferred_string_type_definition.is_some()
{
ExecutionContext::Typing
} else {

View file

@ -1,8 +1,14 @@
use rustpython_parser::ast::{Expr, ExprKind};
use anyhow::Result;
use rustpython_parser as parser;
use rustpython_parser::ast::{Expr, ExprKind, Location};
use ruff_python_stdlib::typing::{PEP_585_BUILTINS_ELIGIBLE, PEP_593_SUBSCRIPTS, SUBSCRIPTS};
use crate::context::Context;
use crate::relocate::relocate_expr;
use crate::source_code::Locator;
use crate::str;
use crate::types::Range;
pub enum Callable {
ForwardRef,
@ -66,3 +72,48 @@ pub fn is_pep585_builtin(expr: &Expr, context: &Context) -> bool {
PEP_585_BUILTINS_ELIGIBLE.contains(&call_path.as_slice())
})
}
#[derive(is_macro::Is)]
pub enum AnnotationKind {
/// The annotation is defined as part a simple string literal,
/// e.g. `x: "List[int]" = []`. Annotations within simple literals
/// can be accurately located. For example, we can underline specific
/// expressions within the annotation and apply automatic fixes, which is
/// not possible for complex string literals.
Simple,
/// The annotation is defined as part of a complex string literal, such as
/// a literal containing an implicit concatenation or escaped characters,
/// e.g. `x: "List" "[int]" = []`. These are comparatively rare, but valid.
Complex,
}
/// Parse a type annotation from a string.
pub fn parse_type_annotation(
value: &str,
range: Range,
locator: &Locator,
) -> Result<(Expr, AnnotationKind)> {
let expression = locator.slice(range);
let body = str::raw_contents(expression);
if body == value {
// The annotation is considered "simple" if and only if the raw representation (e.g.,
// `List[int]` within "List[int]") exactly matches the parsed representation. This
// isn't the case, e.g., for implicit concatenations, or for annotations that contain
// escaped quotes.
let leading_quote = str::leading_quote(expression).unwrap();
let expr = parser::parse_expression_located(
body,
"<filename>",
Location::new(
range.location.row(),
range.location.column() + leading_quote.len(),
),
)?;
Ok((expr, AnnotationKind::Simple))
} else {
// Otherwise, consider this a "complex" annotation.
let mut expr = parser::parse_expression(value, "<filename>")?;
relocate_expr(&mut expr, range);
Ok((expr, AnnotationKind::Complex))
}
}