Skip to content

Commit

Permalink
Remove nested f-string flag (#5966)
Browse files Browse the repository at this point in the history 
## Summary

Not worth taking up a slot in the semantic model flags.
  • Loading branch information
@charliermarsh
charliermarsh authored Jul 22, 2023
1 parent f5a2fb5 commit 86b6a3e
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 102 deletions.
6 changes: 1 addition & 5 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3934,11 +3934,7 @@ where
}
}
Expr::JoinedStr(_) => {
self.semantic.flags |= if self.semantic.in_f_string() {
SemanticModelFlags::NESTED_F_STRING
} else {
SemanticModelFlags::F_STRING
};
self.semantic.flags |= SemanticModelFlags::F_STRING;
visitor::walk_expr(self, expr);
}
_ => visitor::walk_expr(self, expr),
Expand Down
162 changes: 86 additions & 76 deletions crates/ruff/src/rules/pyupgrade/rules/native_literals.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ pub(crate) fn native_literals(
return;
}

// There's no way to rewrite, e.g., `f"{f'{str()}'}"` within a nested f-string.
if checker.semantic().in_nested_f_string() {
return;
}

if !matches!(id.as_str(), "str" | "bytes") {
return;
}
Expand All @@ -94,80 +89,95 @@ pub(crate) fn native_literals(
return;
}

let Some(arg) = args.get(0) else {
let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
// There's no way to rewrite, e.g., `f"{f'{str()}'}"` within a nested f-string.
if checker.semantic().in_f_string() {
if checker
.semantic()
.expr_ancestors()
.filter(|expr| expr.is_joined_str_expr())
.count()
> 1
{
return;
}
}

match args.get(0) {
None => {
let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
} else {
LiteralType::Bytes
},
},
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
let constant = if id == "bytes" {
Constant::Bytes(vec![])
} else {
LiteralType::Bytes
Constant::Str(String::new())
};
let content = checker.generator().constant(&constant);
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
content,
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
Some(arg) => {
// Look for `str("")`.
if id == "str"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Str(_),
..
}),
)
{
return;
}

// Look for `bytes(b"")`
if id == "bytes"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Bytes(_),
..
}),
)
{
return;
}

// Skip implicit string concatenations.
let arg_code = checker.locator.slice(arg.range());
if is_implicit_concatenation(arg_code) {
return;
}

let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
} else {
LiteralType::Bytes
},
},
},
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
let constant = if id == "bytes" {
Constant::Bytes(vec![])
} else {
Constant::Str(String::new())
};
let content = checker.generator().constant(&constant);
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
content,
expr.range(),
)));
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
arg_code.to_string(),
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
checker.diagnostics.push(diagnostic);
return;
};

// Look for `str("")`.
if id == "str"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Str(_),
..
}),
)
{
return;
}

// Look for `bytes(b"")`
if id == "bytes"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Bytes(_),
..
}),
)
{
return;
}

// Skip implicit string concatenations.
let arg_code = checker.locator.slice(arg.range());
if is_implicit_concatenation(arg_code) {
return;
}

let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
} else {
LiteralType::Bytes
},
},
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
arg_code.to_string(),
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
28 changes: 7 additions & 21 deletions crates/ruff_python_semantic/src/model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1094,12 +1094,6 @@ impl<'a> SemanticModel<'a> {
/// Return `true` if the context is in an f-string.
pub const fn in_f_string(&self) -> bool {
self.flags.contains(SemanticModelFlags::F_STRING)
|| self.flags.contains(SemanticModelFlags::NESTED_F_STRING)
}

/// Return `true` if the context is in a nested f-string.
pub const fn in_nested_f_string(&self) -> bool {
self.flags.contains(SemanticModelFlags::NESTED_F_STRING)
}

/// Return `true` if the context is in boolean test.
Expand Down Expand Up @@ -1304,14 +1298,6 @@ bitflags! {
/// ```
const F_STRING = 1 << 7;

/// The context is in a nested f-string.
///
/// For example, the context could be visiting `x` in:
/// ```python
/// f'{f"{x}"}'
/// ```
const NESTED_F_STRING = 1 << 8;

/// The context is in a boolean test.
///
/// For example, the context could be visiting `x` in:
Expand All @@ -1322,7 +1308,7 @@ bitflags! {
///
/// The implication is that the actual value returned by the current expression is
/// not used, only its truthiness.
const BOOLEAN_TEST = 1 << 9;
const BOOLEAN_TEST = 1 << 8;

/// The context is in a `typing::Literal` annotation.
///
Expand All @@ -1331,15 +1317,15 @@ bitflags! {
/// def f(x: Literal["A", "B", "C"]):
/// ...
/// ```
const LITERAL = 1 << 10;
const LITERAL = 1 << 9;

/// The context is in a subscript expression.
///
/// For example, the context could be visiting `x["a"]` in:
/// ```python
/// x["a"]["b"]
/// ```
const SUBSCRIPT = 1 << 11;
const SUBSCRIPT = 1 << 10;

/// The context is in a type-checking block.
///
Expand All @@ -1351,7 +1337,7 @@ bitflags! {
/// if TYPE_CHECKING:
/// x: int = 1
/// ```
const TYPE_CHECKING_BLOCK = 1 << 12;
const TYPE_CHECKING_BLOCK = 1 << 11;

/// The context has traversed past the "top-of-file" import boundary.
///
Expand All @@ -1364,7 +1350,7 @@ bitflags! {
///
/// x: int = 1
/// ```
const IMPORT_BOUNDARY = 1 << 13;
const IMPORT_BOUNDARY = 1 << 12;

/// The context has traversed past the `__future__` import boundary.
///
Expand All @@ -1379,7 +1365,7 @@ bitflags! {
///
/// Python considers it a syntax error to import from `__future__` after
/// any other non-`__future__`-importing statements.
const FUTURES_BOUNDARY = 1 << 14;
const FUTURES_BOUNDARY = 1 << 13;

/// `__future__`-style type annotations are enabled in this context.
///
Expand All @@ -1391,7 +1377,7 @@ bitflags! {
/// def f(x: int) -> int:
/// ...
/// ```
const FUTURE_ANNOTATIONS = 1 << 15;
const FUTURE_ANNOTATIONS = 1 << 14;
}
}

Expand Down

0 comments on commit 86b6a3e

Please sign in to comment.