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

Fix can_omit_optional_parentheses for expressions with a right most fstring #9124

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

MichaReiser
Copy link
Member

Summary

This PR fixes an issue in our can_omit_optional_parentheses implementation.

The goal of can_omit_optional_parentheses is to return true when the expression starts or ends with a parenthesized expression.

if a + [
	parentheses,
	are,
	not,
	required,
	here
]: pass

However, our implementation incorrectly traversed into fstrings and other expressions that end with a token. We should skip these
(similar to other parenthesized expressions) because they're not at the end of the expression (they're wrapped).

A silly example, but it should do it for illustrating what it is about

if a + f"parentheses are required because the list isn't at the very end, the closing quote is {[1, 2]}": 
	pass

Test Plan

Added test. The similarity for our old compatibility check remains unchanged. The ecosystem only shows one intentional change.

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@MichaReiser MichaReiser added bug Something isn't working formatter Related to the formatter labels Dec 14, 2023
Copy link
Contributor

ruff-ecosystem results

Formatter (stable)

ℹ️ ecosystem check detected format changes. (+5 -4 lines in 1 file in 41 projects)

reflex-dev/reflex (+5 -4 lines across 1 file)

tests/test_event.py~L239

     assert spec.args[0][1].equals(Var.create_safe("testkey"))
     assert spec.args[1][0].equals(Var.create_safe("options"))
     assert spec.args[1][1].equals(Var.create_safe(options))
-    assert format.format_event(
-        spec
-    ) == f'Event("_remove_cookie", {{key:`testkey`,options:{json.dumps(options)}}})'
+    assert (
+        format.format_event(spec)
+        == f'Event("_remove_cookie", {{key:`testkey`,options:{json.dumps(options)}}})'
+    )
 
 
 def test_clear_local_storage():

Formatter (preview)

ℹ️ ecosystem check detected format changes. (+5 -4 lines in 1 file in 41 projects)

reflex-dev/reflex (+5 -4 lines across 1 file)

ruff format --preview

tests/test_event.py~L239

     assert spec.args[0][1].equals(Var.create_safe("testkey"))
     assert spec.args[1][0].equals(Var.create_safe("options"))
     assert spec.args[1][1].equals(Var.create_safe(options))
-    assert format.format_event(
-        spec
-    ) == f'Event("_remove_cookie", {{key:`testkey`,options:{json.dumps(options)}}})'
+    assert (
+        format.format_event(spec)
+        == f'Event("_remove_cookie", {{key:`testkey`,options:{json.dumps(options)}}})'
+    )
 
 
 def test_clear_local_storage():

@MichaReiser MichaReiser merged commit 7256b88 into main Dec 14, 2023
17 checks passed
@MichaReiser MichaReiser deleted the fix-can-omit-optional-parentheses-f-strings branch December 14, 2023 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants