Skip to content

Commit

Permalink
Ignore stub file assignments to value-requiring targets (#4030)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Apr 19, 2023
1 parent 10d5415 commit cc8b5a5
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 26 deletions.
34 changes: 34 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI015.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,37 @@
# We shouldn't emit Y015 within functions
def f():
field26: list[int] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]


# We shouldn't emit Y015 for __slots__ or __match_args__
class Class1:
__slots__ = (
'_one',
'_two',
'_three',
'_four',
'_five',
'_six',
'_seven',
'_eight',
'_nine',
'_ten',
'_eleven',
)

__match_args__ = (
'one',
'two',
'three',
'four',
'five',
'six',
'seven',
'eight',
'nine',
'ten',
'eleven',
)

# We shouldn't emit Y015 for __all__
__all__ = ["Class1"]
34 changes: 34 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI015.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,37 @@ field25 = 5 * 5 # Y015 Only simple default values are allowed for assignments
# We shouldn't emit Y015 within functions
def f():
field26: list[int] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]


# We shouldn't emit Y015 for __slots__ or __match_args__
class Class1:
__slots__ = (
'_one',
'_two',
'_three',
'_four',
'_five',
'_six',
'_seven',
'_eight',
'_nine',
'_ten',
'_eleven',
)

__match_args__ = (
'one',
'two',
'three',
'four',
'five',
'six',
'seven',
'eight',
'nine',
'ten',
'eleven',
)

# We shouldn't emit Y015 for __all__
__all__ = ["Class1"]
8 changes: 3 additions & 5 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1842,7 +1842,7 @@ where
flake8_pyi::rules::prefix_type_params(self, value, targets);
}
if self.settings.rules.enabled(Rule::AssignmentDefaultInStub) {
flake8_pyi::rules::assignment_default_in_stub(self, value, None);
flake8_pyi::rules::assignment_default_in_stub(self, targets, value);
}
}
}
Expand Down Expand Up @@ -1882,10 +1882,8 @@ where
if self.settings.rules.enabled(Rule::AssignmentDefaultInStub) {
// Ignore assignments in function bodies; those are covered by other rules.
if !self.ctx.scopes().any(|scope| scope.kind.is_function()) {
flake8_pyi::rules::assignment_default_in_stub(
self,
value,
Some(annotation),
flake8_pyi::rules::annotated_assignment_default_in_stub(
self, target, value, annotation,
);
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ pub use pass_in_class_body::{pass_in_class_body, PassInClassBody};
pub use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
pub use prefix_type_params::{prefix_type_params, UnprefixedTypeParam};
pub use simple_defaults::{
argument_simple_defaults, assignment_default_in_stub, typed_argument_simple_defaults,
ArgumentDefaultInStub, AssignmentDefaultInStub, TypedArgumentDefaultInStub,
annotated_assignment_default_in_stub, argument_simple_defaults, assignment_default_in_stub,
typed_argument_simple_defaults, ArgumentDefaultInStub, AssignmentDefaultInStub,
TypedArgumentDefaultInStub,
};
pub use type_comment_in_stub::{type_comment_in_stub, TypeCommentInStub};
pub use unrecognized_platform::{
Expand Down
78 changes: 59 additions & 19 deletions crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::registry::AsRule;
#[violation]
pub struct TypedArgumentDefaultInStub;

/// PYI011
impl AlwaysAutofixableViolation for TypedArgumentDefaultInStub {
#[derive_message_formats]
fn message(&self) -> String {
Expand All @@ -26,7 +25,6 @@ impl AlwaysAutofixableViolation for TypedArgumentDefaultInStub {
#[violation]
pub struct ArgumentDefaultInStub;

/// PYI014
impl AlwaysAutofixableViolation for ArgumentDefaultInStub {
#[derive_message_formats]
fn message(&self) -> String {
Expand All @@ -41,7 +39,6 @@ impl AlwaysAutofixableViolation for ArgumentDefaultInStub {
#[violation]
pub struct AssignmentDefaultInStub;

/// PYI015
impl AlwaysAutofixableViolation for AssignmentDefaultInStub {
#[derive_message_formats]
fn message(&self) -> String {
Expand Down Expand Up @@ -225,7 +222,7 @@ fn is_valid_default_value_with_annotation(
/// Returns `true` if an [`Expr`] appears to be `TypeVar`, `TypeVarTuple`, `NewType`, or `ParamSpec`
/// call.
fn is_type_var_like_call(context: &Context, expr: &Expr) -> bool {
let ExprKind::Call {func, ..} = &expr.node else {
let ExprKind::Call { func, .. } = &expr.node else {
return false;
};
context.resolve_call_path(func).map_or(false, |call_path| {
Expand All @@ -239,6 +236,20 @@ fn is_type_var_like_call(context: &Context, expr: &Expr) -> bool {
})
}

/// Returns `true` if this is a "special" assignment which must have a value (e.g., an assignment to
/// `__all__`).
fn is_special_assignment(context: &Context, target: &Expr) -> bool {
if let ExprKind::Name { id, .. } = &target.node {
match id.as_str() {
"__all__" => context.scope().kind.is_module(),
"__match_args__" | "__slots__" => context.scope().kind.is_class(),
_ => false,
}
} else {
false
}
}

/// PYI011
pub fn typed_argument_simple_defaults(checker: &mut Checker, args: &Arguments) {
if !args.defaults.is_empty() {
Expand Down Expand Up @@ -354,26 +365,55 @@ pub fn argument_simple_defaults(checker: &mut Checker, args: &Arguments) {
}

/// PYI015
pub fn assignment_default_in_stub(checker: &mut Checker, value: &Expr, annotation: Option<&Expr>) {
if annotation.map_or(false, |annotation| {
checker.ctx.match_typing_expr(annotation, "TypeAlias")
}) {
pub fn assignment_default_in_stub(checker: &mut Checker, targets: &[Expr], value: &Expr) {
if targets.len() == 1 && is_special_assignment(&checker.ctx, &targets[0]) {
return;
}
if is_type_var_like_call(&checker.ctx, value) {
return;
}
if !is_valid_default_value_with_annotation(value, checker, true) {
let mut diagnostic = Diagnostic::new(AssignmentDefaultInStub, Range::from(value));

if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::replacement(
"...".to_string(),
value.location,
value.end_location.unwrap(),
));
}
if is_valid_default_value_with_annotation(value, checker, true) {
return;
}

let mut diagnostic = Diagnostic::new(AssignmentDefaultInStub, Range::from(value));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::replacement(
"...".to_string(),
value.location,
value.end_location.unwrap(),
));
}
checker.diagnostics.push(diagnostic);
}

/// PYI015
pub fn annotated_assignment_default_in_stub(
checker: &mut Checker,
target: &Expr,
value: &Expr,
annotation: &Expr,
) {
if checker.ctx.match_typing_expr(annotation, "TypeAlias") {
return;
}
if is_special_assignment(&checker.ctx, target) {
return;
}
if is_type_var_like_call(&checker.ctx, value) {
return;
}
if is_valid_default_value_with_annotation(value, checker, true) {
return;
}

checker.diagnostics.push(diagnostic);
let mut diagnostic = Diagnostic::new(AssignmentDefaultInStub, Range::from(value));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::replacement(
"...".to_string(),
value.location,
value.end_location.unwrap(),
));
}
checker.diagnostics.push(diagnostic);
}

0 comments on commit cc8b5a5

Please sign in to comment.