Skip to content

Commit

Permalink
Add support for top-level quoted annotations in RUF013
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jun 21, 2023
1 parent 621e9ac commit a8fc4ec
Show file tree
Hide file tree
Showing 4 changed files with 318 additions and 162 deletions.
61 changes: 44 additions & 17 deletions crates/ruff/resources/test/fixtures/ruff/RUF013_0.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@ def f(arg: object = None):
pass


def f(arg: int = None): # RUF011
def f(arg: int = None): # RUF013
pass


def f(arg: str = None): # RUF011
def f(arg: str = None): # RUF013
pass


def f(arg: typing.List[str] = None): # RUF011
def f(arg: typing.List[str] = None): # RUF013
pass


def f(arg: Tuple[str] = None): # RUF011
def f(arg: Tuple[str] = None): # RUF013
pass


Expand Down Expand Up @@ -64,15 +64,15 @@ def f(arg: Union[int, str, Any] = None):
pass


def f(arg: Union = None): # RUF011
def f(arg: Union = None): # RUF013
pass


def f(arg: Union[int, str] = None): # RUF011
def f(arg: Union[int, str] = None): # RUF013
pass


def f(arg: typing.Union[int, str] = None): # RUF011
def f(arg: typing.Union[int, str] = None): # RUF013
pass


Expand All @@ -91,11 +91,11 @@ def f(arg: int | float | str | None = None):
pass


def f(arg: int | float = None): # RUF011
def f(arg: int | float = None): # RUF013
pass


def f(arg: int | float | str | bytes = None): # RUF011
def f(arg: int | float | str | bytes = None): # RUF013
pass


Expand All @@ -110,11 +110,11 @@ def f(arg: Literal[1, 2, None, 3] = None):
pass


def f(arg: Literal[1, "foo"] = None): # RUF011
def f(arg: Literal[1, "foo"] = None): # RUF013
pass


def f(arg: typing.Literal[1, "foo", True] = None): # RUF011
def f(arg: typing.Literal[1, "foo", True] = None): # RUF013
pass


Expand All @@ -133,11 +133,11 @@ def f(arg: Annotated[Any, ...] = None):
pass


def f(arg: Annotated[int, ...] = None): # RUF011
def f(arg: Annotated[int, ...] = None): # RUF013
pass


def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF011
def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013
pass


Expand All @@ -153,9 +153,9 @@ def f(


def f(
arg1: int = None, # RUF011
arg2: Union[int, float] = None, # RUF011
arg3: Literal[1, 2, 3] = None, # RUF011
arg1: int = None, # RUF013
arg2: Union[int, float] = None, # RUF013
arg3: Literal[1, 2, 3] = None, # RUF013
):
pass

Expand Down Expand Up @@ -183,5 +183,32 @@ def f(arg: Union[Annotated[int, ...], Annotated[Optional[float], ...]] = None):
pass


def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF011
def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013
pass


# Quoted


def f(arg: "int" = None): # RUF013
pass


def f(arg: "str" = None): # RUF013
pass


def f(arg: "st" "r" = None): # RUF013
pass


def f(arg: "Optional[int]" = None):
pass


def f(arg: Union["int", "str"] = None): # False negative
pass


def f(arg: Union["int", "None"] = None):
pass
75 changes: 56 additions & 19 deletions crates/ruff/src/rules/ruff/rules/implicit_optional.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use anyhow::Result;
use ruff_text_size::TextRange;
use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Constant, Expr, Operator, Ranged};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::typing::parse_type_annotation;
use ruff_python_semantic::SemanticModel;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -65,14 +66,16 @@ pub struct ImplicitOptional {
conversion_type: ConversionType,
}

impl AlwaysAutofixableViolation for ImplicitOptional {
impl Violation for ImplicitOptional {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
format!("PEP 484 prohibits implicit `Optional`")
}

fn autofix_title(&self) -> String {
format!("Convert to `{}`", self.conversion_type)
fn autofix_title(&self) -> Option<String> {
Some(format!("Convert to `{}`", self.conversion_type))
}
}

Expand Down Expand Up @@ -142,6 +145,7 @@ enum TypingTarget<'a> {
None,
Any,
Object,
ForwardReference,
Optional,
Union(Vec<&'a Expr>),
Literal(Vec<&'a Expr>),
Expand Down Expand Up @@ -175,6 +179,10 @@ impl<'a> TypingTarget<'a> {
value: Constant::None,
..
}) => Some(TypingTarget::None),
Expr::Constant(ast::ExprConstant {
value: Constant::Str(_),
..
}) => Some(TypingTarget::ForwardReference),
_ => semantic.resolve_call_path(expr).and_then(|call_path| {
if semantic.match_typing_call_path(&call_path, "Any") {
Some(TypingTarget::Any)
Expand All @@ -196,8 +204,8 @@ impl<'a> TypingTarget<'a> {
| TypingTarget::Object => true,
TypingTarget::Literal(elements) => elements.iter().any(|element| {
let Some(new_target) = TypingTarget::try_from_expr(element, semantic) else {
return false;
};
return false;
};
// Literal can only contain `None`, a literal value, other `Literal`
// or an enum value.
match new_target {
Expand All @@ -208,22 +216,24 @@ impl<'a> TypingTarget<'a> {
}),
TypingTarget::Union(elements) => elements.iter().any(|element| {
let Some(new_target) = TypingTarget::try_from_expr(element, semantic) else {
return false;
};
return false;
};
match new_target {
TypingTarget::None => true,
_ => new_target.contains_none(semantic),
}
}),
TypingTarget::Annotated(element) => {
let Some(new_target) = TypingTarget::try_from_expr(element, semantic) else {
return false;
};
return false;
};
match new_target {
TypingTarget::None => true,
_ => new_target.contains_none(semantic),
}
}
// TODO(charlie): Add support for nested forward references (e.g., `Union["A", "B"]`).
TypingTarget::ForwardReference => true,
}
}
}
Expand Down Expand Up @@ -305,7 +315,7 @@ fn generate_fix(checker: &Checker, conversion_type: ConversionType, expr: &Expr)
}
}

/// RUF011
/// RUF013
pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) {
for ArgWithDefault {
def,
Expand All @@ -326,15 +336,42 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) {
let Some(annotation) = &def.annotation else {
continue
};
let Some(expr) = type_hint_explicitly_allows_none(annotation, checker.semantic()) else {
continue;
};
let conversion_type = checker.settings.target_version.into();

let mut diagnostic = Diagnostic::new(ImplicitOptional { conversion_type }, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| generate_fix(checker, conversion_type, expr));
if let Expr::Constant(ast::ExprConstant {
range,
value: Constant::Str(value),
kind: _,
}) = annotation.as_ref()
{
// Quoted annotation.
if let Ok((annotation, kind)) = parse_type_annotation(value, *range, checker.locator) {
let Some(expr) = type_hint_explicitly_allows_none(&annotation, checker.semantic()) else {
continue;
};
let conversion_type = checker.settings.target_version.into();

let mut diagnostic =
Diagnostic::new(ImplicitOptional { conversion_type }, expr.range());
if checker.patch(diagnostic.kind.rule()) {
if kind.is_simple() {
diagnostic.try_set_fix(|| generate_fix(checker, conversion_type, expr));
}
}
checker.diagnostics.push(diagnostic);
}
} else {
// Unquoted annotation.
let Some(expr) = type_hint_explicitly_allows_none(annotation, checker.semantic()) else {
continue;
};
let conversion_type = checker.settings.target_version.into();

let mut diagnostic =
Diagnostic::new(ImplicitOptional { conversion_type }, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| generate_fix(checker, conversion_type, expr));
}
checker.diagnostics.push(diagnostic);
}
checker.diagnostics.push(diagnostic);
}
}

0 comments on commit a8fc4ec

Please sign in to comment.