detect if a regex escape is needed in a pytest match statement

[nstarman]

Sometimes re.escape are needed in pytest for match=... arguments.
Then the string is modified and the regex escape is no longer needed.
It would be great if ruff could detect this and remove the unneeded re.escape.

M~WE:

import pytest, re

with pytest.raises(Exception, match=re.escape("regexnotneeded")):
    ...

[MichaReiser]

@InSyncWithFoo was so kind to implement the rule and we looked through the ecosystem changes. After seeing the changes, I don't think we should add this rule to ruff. While it's true that the re.escape call is unnecessary, it does train people not to use re.escape and they might then forget using it in cases where they actually should.

It makes me wonder if we could create ar ule that warns about match parameters that would need escaping. Are there heuristics that would allow us to detect correctly escaped from incorrectly escaped strings?

[nstarman]

Easier for escaped that don't need to be, much harder for not-escaped that should to be. Pytest matches can work with regex, so it would be very hard to figure out if a string that looks like a regex isn't and should be escaped.

[AlexWaygood]

If you have a string passed to pytest.raises() that is:

1. Not a raw string
2. Not surrounded by re.escape() and
3. contains a regex metacharacter

Then I think it's reasonable to say that the string should either be a raw string (if it's meant to be a regex) or should be surrounded by re.escape() (if it's not meant to be a regex). That might possibly be a useful rule to add? What do we think?

[InSyncWithFoo]

Then I think it's reasonable to say that the string should either be a raw string (if it's meant to be a regex) or should be surrounded by re.escape() (if it's not meant to be a regex).

Sounds good to me. This also goes well with unraw-re-pattern.

[nstarman]

And the reverse — if a string is surrounded by re.escape but doesn't have any regex in it?

[AlexWaygood]

And the reverse — if a string is surrounded by re.escape but doesn't have any regex in it?

We already considered and rejected that check in #14746, as Micha stated above in #13705 (comment) I agree with his rationale.

[InSyncWithFoo]

I suppose I can go ahead and implement the new rule then? A PR should be ready sometime later this week.