gh-124294: str.format() raises misleading error when using negative indices

[makukha]

Fixes #124294.

TypeError is raised instead of ValueError for backward compatibility with current behaviour.
Detailed error message includes invalid string index value to avoid ambiguity.
Without these details, user might be confused by error pointing to the wrong place.

>>> '{x[-1]}'.format(x=[0,1,2])
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    '{x[-1]}'.format(x=[0,1,2])
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^
TypeError: Index must be a sequence of digits, string '-1' is not allowed

• Issue: str.format() raises misleading error when using negative indices #124294

[ghost]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[makukha]

@ericvsmith I think I've found the right place for the check.

[rruuaanng]

Maybe you should added NEWS, Used to describe your changes.

[skirpichev]

And you, probably, should add a test, right?

[skirpichev]

This is something, that already fails with TypeError in the main. I think you should test actual error messages with assertRaises's msg argument or using assertRaisesRegex().

[skirpichev]

Suggested change

	if (str != NULL)
	if (str)

Also beware PEP 7: "braces are required everywhere, even where C permits them to be omitted".

[ericvsmith]

I still am not convinced this is a good idea. I don't think it's worth complicating things to get a slightly better error message. For example, is changing all possible errors from SubString_new_object actually correct? What about out of memory errors?

[rruuaanng]

Maybe you could delete this comment since the user won't see it anyway.

[makukha]

@ericvsmith I can't say if it's worth, but this issue has quite a long history already:

• 2009: str.format(list) with negative index doesn't work in Python (Stack Overflow)
• 2010: Should str.format allow negative indexes when used for __getitem__ access? (bpo-7951)
• 2010: String format() has problems parsing numeric indexes #53231
• 2022: The str.format feature — "Accessing arguments’ items" failed, when using negative indices. #95846
• 2024: Why is there "TypeError: string indices must be integers" when using negative indices or slices in string formatting? (Stack Overflow)
• 2024: str.format() raises misleading error when using negative indices #124294

As long as str.format() behaviour and documentation for negative indices (and slices) is (IMO) misleading, this list will grow unless something is changed. I understand that current behaviour is there for a long time, and fixing this bug (?) would become a breaking change, but at least people could get informative message to start digging into docs.

I'm not totally satisfied with proposed message

Index must be a sequence of digits, string '-1' is not allowed

The index value could be omitted to not complicate things, as you suggested (no intermediate str object will be created at runtime), giving

Sequence index can only contain digits 0 to 9

Or there is a better wording?

[ericvsmith]

This code works on 3.12, but fails with your patch:

class F:
    def __getitem__(self, key):
        return f'key:{key}'

f = F()

print(f['foo'])
print('{f[foo]}'.format(f=f))

With 3.12, this gives:

key:foo
key:foo

With this PR, it gives:

key:foo
Traceback (most recent call last):
  File "str-format-negative.py", line 8, in <module>
    print('{f[foo]}'.format(f=f))
          ~~~~~~~~~~~~~~~~~^^^^^
TypeError: Index must be a sequence of digits, string 'foo' is not allowed

[makukha]

@ericvsmith now I see how naive was my attempt. The only solution could be parsing negative integers from the very beginning, that is already impossible. Thank you for your clarification and hints which I didn't take seriously.

[ericvsmith]

No problem, @makukha! I happen to be very familiar with this code, and it's not reasonable for everyone to know all the ins and outs. I'm glad you learned something, and please don't be discouraged from continuing to contribute. If you agree, I'll close this, and hopefully people can find it and refer to it in the future.

But if you want to leave it open and continue to try and find a way to improve this (anything is possible!), then that's okay, too.

[makukha]

@ericvsmith, being realistic, I think this PR should be closed. Thank you for your warm attitude!