flake8-string-format icon indicating copy to clipboard operation
flake8-string-format copied to clipboard

P103 in regex

Open jayvdb opened this issue 10 years ago • 5 comments

link_regex = re.compile(r'\[\[(?P<title>[^\]|[<>{}]*)(\|.*?)?\]\]')

produces

./pywikibot/__init__.py:643:25: P103 other string does contain unindexed parameters

jayvdb avatar Oct 18 '15 02:10 jayvdb

This is intentional. As I'm using ast to detect strings it's not possible to me to detect raw-strings and it's basically impossible to know that re.compile won't do any funny str.format usage. For example:

class C(object):
    def compile(self, s):
        return s.format(42)
re = C()
link_regex = re.compile(r'\[\[(?P<title>[^\]|[<>{}]*)(\|.*?)?\]\]')

And while you may argue that re.compile is in most cases not using str.format there could be other cases where it's forwarding the string to re.compile and then it's not a re.compile() call directly.

So while I'd prefer to not detect these strings, I don't think there is any sensible solution without implementing #noqa.

xZise avatar Oct 18 '15 11:10 xZise

A whitelist including re.compile would be useful. Anyone replacing re.compile would need to handle {} properly before using str.format.

If for example it was:

__all__ = ('foo', )
link_regex = r'\[\[(?P<title>[^\]|[<>{}]*)(\|.*?)?\]\]'
foo = re.compile(link_regex)

... the plugin could check all uses of link_regex within the module to ensure it is only used in whitelisted function calls.

This would provide users with a nicer workaround than noqa: wrap strange uses of {} with a function which can be added to the whitelist.

jayvdb avatar Oct 18 '15 20:10 jayvdb

Well some wonky whitelist which would ignore the code from the opening post might be reasonably easy. But tracking variables could be quite a bit more complicated. As I said I'm using ast so it won't actually interpret the code… at least not to the point that I know that link_regex in line 3 was defined via line 2. And anyway… that code is still legitimate to P103 as someone might import that module and use link_regex directly.

xZise avatar Oct 19 '15 01:10 xZise

Well, in the case of re: r'{}', these can be escaped (or not?): r'\{\}'. But there is more major issue when unittest wants to test, whether my_func will change the string containing these brackets:

# my_func can for example add closing bracket if it is missing
s='some {} string'
assertEqual(my_func(s), 'some {} string')

This will complain on two lines. How to solve the issue like this?

dvorapa avatar Jan 29 '18 17:01 dvorapa

Personally I think in a case like yours @dvorapa, the P103 should be ignored. It causes many false flags because it is applied to literally every string which isn't used immediately (aka in the format of "...".format(...)). And in cases like yours I'd say that (if it's to annoying) ignoring P103 is a vailable strategy.

But maybe you got an idea to improve that error code so that you can ignore it more constrained. For example spliting it up into different types, that there are different error codes when you assign a string and when you use it as a parameter. But, as evident by your case, they would produce the same false flags.

xZise avatar Jan 30 '18 14:01 xZise