Implemented new check use-maxsplit-arg

[yushao2]

Steps

• Add a ChangeLog entry describing what your PR does.
• If it's a new feature or an important bug fix, add a What's New entry in
  doc/whatsnew/<current release.rst>.
• Write a good description on what the PR does.

Description

Implemented a new checker, consider-using-str-partition, which checks for instances where
only the first or last element of a str.split() is accessed

Type of Changes

	Type
✓	✨ New feature

Related Issue

Closes #4440

Notes:

It seems like the check has difficulty detecting if something is a string or not when iterating over an iterable of strings that is returned from a function. See:

#4469 (comment)

#4469 (comment)

#4469 (comment)

That being said, I'm not too sure how to resolve these specific cases, or if the checker is alright as it is -- I'm just worried that it might be a bit too "weak" for the cases mentioned. Thanks!

[yushao2]

Leaving this as draft as a moment because I'm having difficulty with a particular test case:

import os
path = "/home/"
dirs = os.listdirs(path)
extensions = [d.split(".")[-1] for d in dirs if "." in d]   # should fire an error

It seems like the d in this case can't be inferred as a string, and the .split() is also uninferrable -- however we know that os.listdirs returns a list of string.

Therefore I'm somewhat stuck at the moment -- will experiment around and see if I could fix it

[coveralls]

Coverage decreased (-0.003%) to 91.723% when pulling 7e640f5 on yushao2:checkers-4440 into 55d3cb7 on PyCQA:master.

[yushao2]

Leaving this as draft as a moment because I'm having difficulty with a particular test case:

import os
path = "/home/"
dirs = os.listdirs(path)
extensions = [d.split(".")[-1] for d in dirs if "." in d]   # should fire an error

It seems like the d in this case can't be inferred as a string, and the .split() is also uninferrable -- however we know that os.listdirs returns a list of string.

Therefore I'm somewhat stuck at the moment -- will experiment around and see if I could fix it

It seems like some output of functions might not be infer-able, during my debugging process I found this and it seems like the return type information is not available for os.listdir():

I'm not entirely sure if there's a utility function to check, but for now I will be changing this PR to be ready for review

[yushao2]

Leaving this as draft as a moment because I'm having difficulty with a particular test case:

import os
path = "/home/"
dirs = os.listdirs(path)
extensions = [d.split(".")[-1] for d in dirs if "." in d]   # should fire an error

It seems like the d in this case can't be inferred as a string, and the .split() is also uninferrable -- however we know that os.listdirs returns a list of string.
Therefore I'm somewhat stuck at the moment -- will experiment around and see if I could fix it

It seems like some output of functions might not be infer-able, during my debugging process I found this and it seems like the return type information is not available for os.listdir():

I'm not entirely sure if there's a utility function to check, but for now I will be changing this PR to be ready for review

Further investigation further confirming difficulties with determining type of iterable returned from functions:

list_of_strs = ["a", "b", "c", "d", "e", "f"]
for s in list_of_strs:
    print(s.split(" ")[0])  # [consider-using-str-partition]
    print(s.split(" ")[-1])  # [consider-using-str-partition]
    print(s.split(" ")[-2])

my_dict = {"a":0, "b": 0, "c": 0, "d": 0, "e": 0, "f":0}
for s in my_dict:
    print(s.split(" ")[0])  # [consider-using-str-partition]  (NOT FIRED)
    print(s.split(" ")[-1])  # [consider-using-str-partition] (NOT FIRED)
    print(s.split(" ")[-2])

[cdce8p]

Some initial comments.

--

import os
path = "/home/"
dirs = os.listdirs(path)
extensions = [d.split(".")[-1] for d in dirs if "." in d]   # should fire an error

It seems like the d in this case can't be inferred as a string, and the .split() is also uninferrable -- however we know that os.listdirs returns a list of string.

Therefore I'm somewhat stuck at the moment -- will experiment around and see if I could fix it

I won't worry too much about this. It's probably best to just ignore everything that can't easily be inferred.

[yushao2]

@cdce8p could I get another initial review on this? Have re-implemented as discussed -- but still in the midst of performing some refactoring (moving some code into utils)

[Pierre-Sassoulas]

Very nice checker, and the non-opinionated performance checks are always a pleasure to merge. Thanks a lot for handling all the comments during the review. The design on this one got better and better thanks to your perseverance. Probably the main feature in 2.8.3 !

[yushao2]

Very nice checker, and the non-opinionated performance checks are always a pleasure to merge. Thanks a lot for handling all the comments during the review. The design on this one got better and better thanks to your perseverance. Probably the main feature in 2.8.3 !

thanks for the kind words :)

[cdce8p]

It looks so good.. Unfortunately I noticed one small issue during the final test run. (See comment below)

FYI: I also pushed a small commit with formatting changes. It wouldn't have been worth it to mention these changes individually.

[cdce8p]

They might be a bit contrived but I found two more cases that won't work properly:

"1,2,3".split('split')[-1]
"1,2,3".rsplit('rsplit')[0]

With the current logic, the separator will be replaced instead of the function name.

Some ideas

• Use node.func.as_string() and call rsplit on it instead
• I didn't know that myself, but whereas utils.get_argument_from_call(node, 0, "sep").value returns the unescaped separator, you can use utils.get_argument_from_call(node, 0, "sep").as_string()

[cdce8p]

It's done! Tremendous work you did here @yushao2 🥇
The review process wasn't the easiest but you still did a really good job getting this one over the finish line. The next release will be awesome in no small part thanks to you 🐬

Will have to look at #4485 now I guess 😉 Can't be that you only contributed two new checkers for one release.