Skip to content
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

Remove nested f-string flag #5966

Merged
merged 1 commit into from
Jul 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions crates/ruff/src/checkers/ast/mod.rs
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
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
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
Loading