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

Improve backport friendliness of quotes in Python 3.12 f-string placeholders #11056

Open
achimnol opened this issue Apr 20, 2024 · 10 comments
Open
Labels
formatter Related to the formatter

Comments

@achimnol
Copy link

achimnol commented Apr 20, 2024

I'm maintaining a codebase with multiple release branches targeting different Python versions by release. The latest one uses Python 3.12 while prior versions use Python 3.10 or 3.11.

The recent update of Ruff has introduced Python 3.12 f-string support and the Ruff v0.4 formatter replaces all single-quotes in placeholders to double-quotes.

Example:
f"a value from dict: {data['key']}" (the previous way to write string literals in f-string placeholdrs)

f"a value from dict: {data["key"]}" (allowed in Python 3.12 but a syntax error in Python 3.11 or older)

The problem is that when I backport this line to a release branch targeting Python 3.11 or older (but using the same Ruff version), Ruff silently ignores the broken syntax. It does not auto-convert the existing codes in Python 3.11, but also does not give errors about the backported codes. Linting will pass, but it will fail when someone executes/imports the code.

In contrast, Black (24.4) loudly fails because the Python parser reports a syntax error.

There could be the following options to resolve the issue:

  • Let Ruff 0.4 targeting Python 3.11 or older report the syntax error loudly, just like Black.
  • Let Ruff 0.4 targeting Python 3.11 or older auto-convert the double-quotes in f-string placeholders back to single-quotes (or vice-versa depending on the quote style).
  • Add an option to keep the single-quotes in f-string placholders intact in Python 3.12 or later.
@achimnol achimnol changed the title Option to preserve single-quotes in placeholders of f-strings to allow backporting Python 3.12 codes to Python 3.11 Improve backport freidliness of quotes in Python 3.12 f-string placeholders Apr 20, 2024
@achimnol achimnol changed the title Improve backport freidliness of quotes in Python 3.12 f-string placeholders Improve backport friendliness of quotes in Python 3.12 f-string placeholders Apr 20, 2024
@MichaReiser
Copy link
Member

Thanks for the detailed report and we're sorry that you're experiencing this. I think backward compatibility could actually be the reason for changing the default to use single quotes in nested f-strings (@dhruvmanila).

We have plans to implement lint rules that allow catching unsupported syntax depending on the configured python version. See #6591

@dhruvmanila
Copy link
Member

Thank you for raising this issue!

I think backward compatibility could actually be the reason for changing the default to use single quotes in nested f-strings (@dhruvmanila).

Yeah, I think that's a strong reason to not change the quotes and it should be the case until the parser supports target version.

@dhruvmanila dhruvmanila added the formatter Related to the formatter label Apr 22, 2024
@dhruvmanila
Copy link
Member

Reading through the suggested solutions, (1) would be done by #6591, I don't think (2) is ideal because that could be complicated.

I think for now it's reasonable to avoid changing the quotes even for 3.12 target version.

Related: #10273, #10184

@achimnol
Copy link
Author

(1) should suffice my needs, because developers could be warned explicitly about the breakage.
(3) (avoiding changing the quotes) would be a good option for us as well.

@dhruvmanila
Copy link
Member

Regarding (1), I'm not sure when it would be picked up but ideally it could be after #1774 and before I start the work on error resilience in the parser.

Regarding (3), I think I'd like avoid adding a config option before finalizing the f-string formatting style. It's currently in preview.

@mvaled
Copy link

mvaled commented May 2, 2024

I consider this a bug if target-python is set to py311 or older.

For instance:

$ cat src/ruffformatter/__init__.py 
def hello() -> str:
    return f"Hello {"a" + "b"}"


$ cat pyproject.toml 
[project]
name = "ruffformatter"
version = "0.1.0"
description = "Add your description here"
dependencies = []
readme = "README.md"
requires-python = ">= 3.8"
license = { text = "MIT" }

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

[tool.rye]
managed = true
dev-dependencies = [
    "ruff==0.4.2",
]

[tool.hatch.metadata]
allow-direct-references = true

[tool.hatch.build.targets.wheel]
packages = ["src/ruffformatter"]

[tool.ruff]
target-version = "py311"

$ rye run ruff format src/
1 file left unchanged

But the file is not syntactically valid in Python 3.11:

$ rye run python --version
Python 3.11.7

$ rye run python src/ruffformatter/__init__.py 
  File "/home/manu/tmp/ruffformatter/src/ruffformatter/__init__.py", line 2
    return f"Hello {"a" + "b"}"
                     ^
SyntaxError: f-string: expecting '}'

@MichaReiser
Copy link
Member

@mvaled What I see from the output is that the formatter didn't change the file. The formatter won't change the quote style when targeting python 3.11 or older. That means it's up to you to use the right quotes.

@mvaled
Copy link

mvaled commented May 2, 2024

@MichaReiser But neither ruff check catches that as an error.

IMO, either ruff format or ruff check should treat that as error. Since Python reports that as syntax error.

@MichaReiser
Copy link
Member

Agree, that's planned in #6591 that @dhruvmanila mentioned above

@charliermarsh
Copy link
Member

Yeah we don't enforce syntax errors right now -- that's known, and isn't a bug but rather a limitation (we just haven't implemented it). What would be a bug is if we changed your code to use syntax that isn't supported by your target version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

No branches or pull requests

5 participants