Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add simplify-split check #291

Closed
wants to merge 2 commits into from

Conversation

jayceslesar
Copy link

Followed the changes detailed from #275 and used print to guide the solution

@dosisod
Copy link
Owner

dosisod commented Sep 26, 2023

Thank you for your contribution! Unfortunately, split(" ") is not the same as split(), as split without an argument will split on all whitespace, not just the space character. For example:

>>> data = "hello there\tworld"

>>> data.split(" ")
['hello', 'there\tworld']

>>> data.split()
['hello', 'there', 'world']

>>> data.split(None)
['hello', 'there', 'world']

>>> "".split(" ")
['']

>>> "".split()
[]

split(None) is the same as split() though, so we could update this to include that instead. If that is the case though, we would be better off adding a str.split(None) entry to FURB120, since that check already detects default arguments in the stdlib.

@jayceslesar
Copy link
Author

@dosisod
Copy link
Owner

dosisod commented Sep 27, 2023

@jayceslesar Yep! It isn't really documented, but basically ... is any argument or type, whereas NoneNode, IntExpr(123), and StringExpr("xyz") correspond to None, 123, and "xyz" respectively.

@jayceslesar
Copy link
Author

@jayceslesar Yep! It isn't really documented, but basically ... is any argument or type, whereas NoneNode, IntExpr(123), and StringExpr("xyz") correspond to None, 123, and "xyz" respectively.

sounds good! I will close this and make a new PR

@jayceslesar
Copy link
Author

jayceslesar commented Nov 14, 2023

@dosisod I did try this for a bit but I dont think that the use_implicit_default check is working because poetry run refurb test/data/err_120.py doesn't even yield any output.

Narrowed it down to the something wrong with the NoneNode definition the only 3 checks not working defined in 120 are

    "builtins.dict.fromkeys": (..., NoneNode),
    "builtins.dict.get": (..., NoneNode),
    "builtins.dict.setdefault": (..., NoneNode),

@dosisod
Copy link
Owner

dosisod commented Nov 14, 2023

Try adding the flag --enable FURB120 to the command. FURB120 is disabled by default, so that's probably why you're not seeing any output.

@jayceslesar
Copy link
Author

jayceslesar commented Nov 14, 2023

yeah my file looks like

"".split(None)

# round(1, 0)

and I am running poetry run refurb --enable FURB120 asdf.py on latest main and I dont see any output when it is just "".split(None) but when I uncomment round(1, 0) I get the 120 message. Even if I use a known check that should be caught of {}.get("asdf", None) I get nothing

@dosisod
Copy link
Owner

dosisod commented Nov 15, 2023

Oh, I see what you mean. Currently FURB120 only detects x.get("key", None), not {}.get("key", None). That's probably why "".split(None) isn't working for you. If you try:

s = ""
s.split(None)

then it should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants