Slicing of bracketed expressions is broken by removal of the brackets #1575
Labels
T: bug
Something isn't working
Comments
|
I can't reproduce this on master: |
|
@JelleZijlstra Interestingly I can reproduce this on master.
This was my session: (black-rHKUX7ap) R:\Programming\black>echo return ('%x' % value)[-12:] > test.py
(black-rHKUX7ap) R:\Programming\black>type test.py
return ('%x' % value)[-12:]
(black-rHKUX7ap) R:\Programming\black>black test.py --check
error: cannot format test.py: INTERNAL ERROR: Black produced code that is not equivalent to the source. Please report a bug on https://github.com/psf/black/issues. This diff might be helpful: C:\Users\User\AppData\Local\Temp\blk_0tsx_tzh.log
Oh no! 💥 💔 💥
1 file would fail to reformat.
(black-rHKUX7ap) R:\Programming\black>black test.py --fast --check
would reformat test.py
Oh no! 💥 💔 💥
1 file would be reformatted.
(black-rHKUX7ap) R:\Programming\black>black test.py --fast
reformatted test.py
All done! ✨ 🍰 ✨
1 file reformatted.
(black-rHKUX7ap) R:\Programming\black>type test.py
return "%x" % value[-12:]
(black-rHKUX7ap) R:\Programming\black>black -c "return ('%x' % value)[-12:]"
return "%x" % value[-12:]
(black-rHKUX7ap) R:\Programming\black>echo return ('%x' % value)[-12:] | black -
error: cannot format -: INTERNAL ERROR: Black produced code that is not equivalent to the source. Please report a bug on https://github.com/psf/black/issues. This diff might be helpful: C:\Users\User\AppData\Local\Temp\blk_i2fy39_v.log
Oh no! 💥 💔 💥
1 file failed to reformat. Environment:
|
|
Oops, I thought I was on master but was actually in 19.3b1. It does reproduce on master. This is a bad bug; we should fix it before the next release. |
|
Git bisect points to commit 544ea9c as the first bad commit. |
davidszotten
added a commit
to davidszotten/black
that referenced
this issue
Aug 12, 2020
it's not safe to remove brackets that are followed by a `[`
davidszotten
added a commit
to davidszotten/black
that referenced
this issue
Aug 12, 2020
it's not safe to remove brackets that are followed by a `[`
davidszotten
added a commit
to davidszotten/black
that referenced
this issue
Aug 13, 2020
it's not safe to remove brackets that are followed by an operator with lower precedence than PERCENT
JelleZijlstra
pushed a commit
that referenced
this issue
Aug 14, 2020
|
i think we can close this? (possibly pending #1600) |
anderson-dan-w
pushed a commit
to sigopt/black
that referenced
this issue
Aug 31, 2020
* Find project root correctly (psf#1518) Ensure root dir is a common parent of all inputs Fixes psf#1493 * Add pip install from GitHub command to README.md (psf#1529) * Add pip install from GitHub command to README.md * Make it prettier ... * Mozilla uses black too (psf#1531) * add Quora to orgs that use Black (psf#1532) * ISSUE 1533: Fix --config argument description (psf#1534) Change --config argument description to "Read configuration from FILE." The "--config FILE Read configuration from FILE path" * Spelling fix in CONTRIBUTING.md (psf#1547) * Ensure path for finding root is absolute (psf#1550) As Path.resolve() is buggy on windows (see https://bugs.python.org/issue38671) an absolute path is ensured by prepending the Path.cwd() * Update curl command to use stable branch (psf#1543) * pre-commit: show diff on failure on CI (psf#1552) * pre-commit: --show-diff-on-failure * pre-commit: --show-diff-on-failure * docs: Improve pre-commit use (psf#1551) Stable tag wasn't available and crashed when attempting to set initial pre-commit. Also the python version needs to be installed so it would be better to use the generic "python3" command. * Update to accomodate isort 5 release changes. (psf#1559) Isort 5 introduced profiles and ensure_newline_before_comments options. Either needs to be added to work correctly with black. Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> * fix spelling (psf#1567) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> * Remove slow assertion (psf#1592) Partial fix for psf#1581 This assertion produces behavior quadratic in the number of leaves in a line, which is making Black extremely slow on files with very long expressions. On my benchmark file this change makes Black 10x faster. * Add the direnv base directory to the default excludes (psf#1564) Co-authored-by: Chris Rose <offline@offby1.net> * Make --exclude only apply to recursively found files (psf#1591) Ever since --force-exclude was added, --exclude started to touch files that were given to Black through the CLI too. This is not documented behaviour and neither expected as --exclude and --force-exclude now behave the same! Before this commit, get_sources() when encountering a file that was passed explicitly through the CLI would pass a single Path object list to gen_python_files(). This causes bad behaviour since that function doesn't treat the exclude and force_exclude regexes differently. Which is fine for recursively found files, but *not* for files given through the CLI. Now when get_sources() iterates through srcs and encounters a file, it checks if the force_exclude regex matches, if not, then the file will be added to the computed sources set. A new function had to be created since before you can do regex matching, the path must be normalized. The full process of normalizing the path is somewhat long as there is special error handling. I didn't want to duplicate this logic in get_sources() and gen_python_files() so that's why there is a new helper function. * in verbose mode, print stack trace (psf#1594) Make Black failures easier to debug * fix some docstring crashes (psf#1593) Allow removing some trailing whitespace * don't strip brackets before lsqb (psf#1575) (psf#1590) if the string contains a PERCENT, it's not safe to remove brackets that follow and operator with the same or higher precedence than PERCENT * fix unary op detection (psf#1600) * Fix inline code style in README (psf#1608) Refer to `pyproject.toml` in HTML section of README with HTML code tags * Update all dependencies to latest versions * Disable string splitting/merging by default (psf#1609) * put experimental string stuff behind a flag * update tests * don't need an output section if it's the same as the input * Primer: Expect no formatting changes in attrs, hypothesis and poetry with --experimental-string-processing off Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> * Upgrade docs to Sphinx 3+ and add doc build test (psf#1613) * Upgrade docs to Sphinx 3+ * Fix all the warnings... - Fixed bad docstrings - Fixed bad fenced code blocks in documentation - Blocklisted some sections from being generated from the README - Added missing documentation to index.rst - Fixed an invalid autofunction directive in reference/reference_functions.rst - Pin another documentation dependency * Add documentation build test * Reset trailing comma handling * Re-implement magic trailing comma handling: - when a trailing comma is specified in any bracket pair, that signals to Black that this bracket pair needs to be always exploded, e.g. presented as "one item per line"; - this causes some changes to previously formatted code that erroneously left trailing commas embedded into single-line expressions; - internally, Black needs to be able to identify trailing commas that it put itself compared to pre-existing trailing commas. We do this by using/abusing lib2to3's `was_checked` attribute. It's True for internally generated trailing commas and False for pre-existing ones (in fact, for all pre-existing leaves and nodes). Fixes psf#1288 * Reformat docs/conf.py according to the new style * Open file explicitly with UTF-8 so it works on Windows, too * Mark Primer projects that will change formatting * Require Sphinx 3 * Use properly renamed function name in docs * Fix dealing with generated files in docs * Property-based fuzz test * Update the changelog * Make doc generation a little smarter, update doc sections * Add more trailing comma test variants * Run trailing comma tests with TargetVersion.PY38 * Address pre-existing trailing commas when not in the rightmost bracket pair This required some hackery. Long story short, we need to reuse the ability to omit rightmost bracket pairs (which glues them together and splits on something else instead), for use with pre-existing trailing commas. This form of user-controlled formatting is brittle so we have to be careful not to cause a scenario where Black first formats code without trailing commas in one way, and then looks at the same file with pre-existing trailing commas (that it itself put on the previous run) and decides to format the code again. One particular ugly edge case here is handling of optional parentheses. In particular, the long-standing `line_length=1` hack got in the way of pre-existing trailing commas and had to be removed. Instead, a more intelligent but costly solution was put in place: a "second opinion" if the formatting that omits optional parentheses ended up causing lines to be too long. Again, for efficiency purposes, Black reuses Leaf objects from blib2to3 and modifies them in place, which was invalid for having two separate formattings. Line cloning was used to mitigate this. Fixes psf#1619 * Improve docstring re-indentation handling This addresses a few crashers, namely: * producing non-equivalent code due to mangling escaped newlines, * invalid hugging quote characters in the docstring body to the docstring outer triple quotes (causing a quadruple quote which is a syntax error), * lack of handling for docstrings that start on the same line as the `def`, and * invalid stripping of outer triple quotes when the docstring contained a string prefix. As a bonus, tests now also run when string normalization is disabled. * Add links regarding Spotless integration for gradle/maven users (psf#1622) Co-authored-by: Łukasz Langa <lukasz@langa.pl> * Primer update config - enable pytest (psf#1626) Reformatted projects I have acceess to: - aioexabgp - bandersnatch - flake8-bugbear ``` -- primer results 📊 -- 13 / 16 succeeded (81.25%) ✅ 0 / 16 FAILED (0.0%) 💩 - 3 projects disabled by config - 0 projects skipped due to Python version - 0 skipped due to long checkout ``` * Also re-enable pytest ``` -- primer results 📊 -- 14 / 16 succeeded (87.5%) ✅ 0 / 16 FAILED (0.0%) 💩 - 2 projects disabled by config - 0 projects skipped due to Python version - 0 skipped due to long checkout real 2m26.207s user 17m55.404s sys 0m43.061s ``` * v20.8b0 * Treat all trailing commas as pre-existing, as they effectively are On a second pass of Black on the same file, inserted trailing commas are now pre-existing. Doesn't make sense to differentiate between the passes then. * Include mode information for unstable formattings * Add expected failure tests with the unstable formattings * Make dependency on Click 7.0, regex 2020.1.8, and toml 0.10.1 explicit * v20.8b1 Co-authored-by: Lihu Ben-Ezri-Ravin <lbenezriravin@starry.com> Co-authored-by: Cooper Lees <me@cooperlees.com> Co-authored-by: Sylvestre Ledru <sledru@mozilla.com> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> Co-authored-by: Olexiy <alosha969@gmail.com> Co-authored-by: jtpavlock <jtpavlock@gmail.com> Co-authored-by: dhaug-op <56020126+dhaug-op@users.noreply.github.com> Co-authored-by: Steven Maude <StevenMaude@users.noreply.github.com> Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Vinicius Gubiani Ferreira <vini.g.fer@gmail.com> Co-authored-by: Maximilian Cosmo Sitter <48606431+mcsitter@users.noreply.github.com> Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> Co-authored-by: Kaligule <Code@schauderbasis.de> Co-authored-by: Chris Rose <offby1@offby1.net> Co-authored-by: Chris Rose <offline@offby1.net> Co-authored-by: David Szotten <davidszotten@gmail.com> Co-authored-by: Daanyaal Syed <daanyaalasyed@gmail.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl> Co-authored-by: Zac-HD <zac.hatfield.dodds@gmail.com> Co-authored-by: Ned Twigg <ned.twigg@diffplug.com>
This was referenced Mar 12, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Slicing of bracketed expressions is broken by too eager removal of the brackets.
To Reproduce
return ('%x' % value)[-12:]return '%x' % mac[-12:]Expected behavior
Expected to not remove the brackets in this case.
Environment (please complete the following information):
Does this bug also happen on master? To answer this, you have two options:
Yes.
The text was updated successfully, but these errors were encountered: