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

bad-format-character false negative on .format() and f-strings #6085

Open
DudeNr33 opened this issue Apr 1, 2022 · 1 comment
Open

bad-format-character false negative on .format() and f-strings #6085

DudeNr33 opened this issue Apr 1, 2022 · 1 comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@DudeNr33
Copy link
Collaborator

DudeNr33 commented Apr 1, 2022

Bug description

bad-format-character is not raised for new-style string formatting and f-strings.

# pylint: disable=consider-using-f-string, missing-module-docstring
PI = 3.1415
print("Pi is %.2v" % PI)
print("Pi is {:.2v}".format(PI))
print(f"Pi is {PI:.2v}")

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
a.py:3:6: E1300: Unsupported format character 'v' (0x76) at index 9 (bad-format-character)

Expected behavior

bad-format-character is currently only emitted for the old-style string formatting.

It should also cover new style formatting and f-strings.

Pylint version

pylint 2.14.0-dev0
astroid 2.11.2
Python 3.9.6 (default, Jul 28 2021, 21:15:06) 
[Clang 12.0.5 (clang-1205.0.22.9)]

OS / Environment

macOS Big Sur Version 11.6

Additional dependencies

No response

@DudeNr33 DudeNr33 added Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Bug 🪲 False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 1, 2022
@DanielNoord
Copy link
Collaborator

For anybody taking this on, after #6086 has landed we will need to update the documentation for this as well.

@Pierre-Sassoulas Pierre-Sassoulas added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Bug 🪲 labels Jul 13, 2022
charliermarsh added a commit to astral-sh/ruff that referenced this issue Aug 2, 2023
## Summary

Relates to #970.

Add new `bad-format-character` Pylint rule.

I had to make a change in `crates/ruff_python_literal/src/format.rs` to
get a more detailed error in case the format character is not correct. I
chose to do this since most of the format spec parsing functions are
private. It would have required me reimplementing most of the parsing
logic just to know if the format char was correct.

This PR also doesn't reflect current Pylint functionality in two ways.

It supports new format strings correctly, Pylint as of now doesn't. See
pylint-dev/pylint#6085.

In case there are multiple adjacent string literals delimited by
whitespace the index of the wrong format char will relative to the
single string. Pylint will instead reported it relative to the
concatenated string.

Given this:
```
"%s" "%z" % ("hello", "world")
```

Ruff will report this:
```Unsupported format character 'z' (0x7a) at index 1```

Pylint instead:
```Unsupported format character 'z' (0x7a) at index 3```

I believe it's more sensible to report the index relative to the
individual string.

## Test Plan

Added new snapshot and a small test in
`crates/ruff_python_literal/src/format.rs`.

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
3 participants