-
Notifications
You must be signed in to change notification settings - Fork 882
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
Fix PYI011 and add auto-fix #3492
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,25 @@ | ||
use rustpython_parser::ast::{Arguments, Constant, Expr, ExprKind, Operator, Unaryop}; | ||
|
||
use ruff_diagnostics::{Diagnostic, Violation}; | ||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::types::Range; | ||
|
||
use crate::checkers::ast::Checker; | ||
use crate::registry::AsRule; | ||
|
||
#[violation] | ||
pub struct TypedArgumentSimpleDefaults; | ||
|
||
/// PYI011 | ||
impl Violation for TypedArgumentSimpleDefaults { | ||
impl AlwaysAutofixableViolation for TypedArgumentSimpleDefaults { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
format!("Only simple default values allowed for typed arguments") | ||
} | ||
|
||
fn autofix_title(&self) -> String { | ||
"Replace default value by `...`".to_string() | ||
} | ||
} | ||
|
||
#[violation] | ||
|
@@ -111,7 +116,7 @@ fn is_valid_default_value_with_annotation(default: &Expr, checker: &Checker) -> | |
if let ExprKind::Attribute { .. } = &operand.node { | ||
if checker | ||
.ctx | ||
.resolve_call_path(default) | ||
.resolve_call_path(operand) | ||
.map_or(false, |call_path| { | ||
ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS.iter().any(|target| { | ||
// reject `-math.nan` | ||
|
@@ -188,10 +193,18 @@ pub fn typed_argument_simple_defaults(checker: &mut Checker, args: &Arguments) { | |
{ | ||
if arg.node.annotation.is_some() { | ||
if !is_valid_default_value_with_annotation(default, checker) { | ||
checker.diagnostics.push(Diagnostic::new( | ||
TypedArgumentSimpleDefaults, | ||
Range::from(default), | ||
)); | ||
let mut diagnostic = | ||
Diagnostic::new(TypedArgumentSimpleDefaults, Range::from(default)); | ||
|
||
if checker.patch(diagnostic.kind.rule()) { | ||
diagnostic.amend(Fix::replacement( | ||
"...".to_string(), | ||
default.location, | ||
default.end_location.unwrap(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is done in a lot of places in the code.
|
||
)); | ||
} | ||
|
||
checker.diagnostics.push(diagnostic); | ||
} | ||
} | ||
} | ||
|
@@ -207,10 +220,18 @@ pub fn typed_argument_simple_defaults(checker: &mut Checker, args: &Arguments) { | |
{ | ||
if kwarg.node.annotation.is_some() { | ||
if !is_valid_default_value_with_annotation(default, checker) { | ||
checker.diagnostics.push(Diagnostic::new( | ||
TypedArgumentSimpleDefaults, | ||
Range::from(default), | ||
)); | ||
let mut diagnostic = | ||
Diagnostic::new(TypedArgumentSimpleDefaults, Range::from(default)); | ||
|
||
if checker.patch(diagnostic.kind.rule()) { | ||
diagnostic.amend(Fix::replacement( | ||
"...".to_string(), | ||
default.location, | ||
default.end_location.unwrap(), | ||
)); | ||
} | ||
|
||
checker.diagnostics.push(diagnostic); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment will be removed.
I am working on another PR to add a method to aggregate comments from
Expression
to keep them.