add autofix for E721
#7965
Closed
add autofix for E721
#7965
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diceroll123
commented
Oct 15, 2023
Comment on lines
+104
to
+124
| let right_side: String = match right_argument.unwrap() { | ||
| Expr::Constant(ast::ExprConstant { value, .. }) => match value { | ||
| ast::Constant::Str(..) => "str".to_string(), | ||
| ast::Constant::Bytes(..) => "bytes".to_string(), | ||
| ast::Constant::Int(..) => "int".to_string(), | ||
| ast::Constant::Float(..) => "float".to_string(), | ||
| ast::Constant::Complex { .. } => "complex".to_string(), | ||
| ast::Constant::Bool(..) => "bool".to_string(), | ||
| _ => continue, | ||
| }, | ||
| Expr::FString(ast::ExprFString { .. }) => "str".to_string(), | ||
| Expr::Tuple(ast::ExprTuple { .. }) => "tuple".to_string(), | ||
| Expr::List(ast::ExprList { .. }) => "list".to_string(), | ||
| Expr::Set(ast::ExprSet { .. }) => "set".to_string(), | ||
| Expr::Dict(ast::ExprDict { .. }) => "dict".to_string(), | ||
| Expr::DictComp(_) => "dict".to_string(), | ||
| Expr::BoolOp(_) => "bool".to_string(), | ||
| Expr::ListComp(_) => "list".to_string(), | ||
| Expr::SetComp(_) => "set".to_string(), | ||
| _ => checker.generator().expr(right), | ||
| }; |
There was a problem hiding this comment.
is there a convenience method for this kind of thing?
There was a problem hiding this comment.
Hm there's the more limited
I don't think we've made a broader mapping yet
diceroll123
force-pushed
the
fix-E721
branch
2 times, most recently
from
e827813 to
3d53fcf
Compare
There was a problem hiding this comment.
I think this fix is going to be hard to get right because there are really two valid outcomes: either using isinstance, or comparing the types directly with is. (See #7905 for the revised rule logic which better matches the current pycodestyle behavior.) I'm hesitant to recommend a fix when we don't have enough information about user intent.
|
I'm gonna close this one for now -- sorry for the churn, I appreciate the work but I think it will be hard to get right. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds autofix for
E721. Resolves the right-hand side into a builtin from a constant expression if possible.Notes:
if res == types.IntType:which is not emitted due to an early guard clause on the left-hand side not usingtype(), I have not fixed this due to added complexityTest Plan
cargo testand manually