Emit error PTH118 for os.sep.join()

[dosisod]

The following shows 2 similar snippets of code, but only one is picked up by Ruff currently:

import os

os.path.join("hello", "world")   # PTH118
os.sep.join(("hello", "world"))  # no error

os.sep.join() acts the same as os.path.join(), just written differently. Searching on grep.app shows that this is a somewhat common idiom.

Running:

$ ruff --version
ruff 0.0.278

$ ruff file.py
file.py:3:1: PTH118 `os.path.join()` should be replaced by `Path` with `/` operator
Found 1 error.

Expected:

An error for both os.sep.join and os.path.join.

[sbrugman]

I can take this one (also working on some other pathlib ports from refurb)

Nice catch, and also interesting to learn about grep.app. I've been using the GitHub search functionality so far

[dosisod]

I've found that GitHub search doesn't find exactly for what I'm looking for while grep.app does, in addition to providing case-insensitive and regex searching. It's also great for gauging how impactful a new rule might be, and finding potential edge-cases.

And thank you for taking the time to port parts of Refurb into Ruff! I've added a lot of checks since #1348 was opened, and I've been meaning to leave a comment with an updated list but haven't had the time. I'll find some time this weekend to update the list!

[charliermarsh]

Any idea why folks are using os.sep.join in lieu of os.path.join? I just want to make sure there's not a subtle distinction that we're overlooking.

[sbrugman]

os.sep.join is a string join on the os.sep character. os.path.join also handles some path logic. os.sep.join may be faster (so)

For the Pathlib rules this shouldn't matter. The overhead of path objects in is neglectable unless it's in a loop (and this is mentioned in our docs in #5815)

[sbrugman]

Just realised that we should also detect the opposite: .split(os.sep) is also frequently used

This should be a new rule, the requires a combination of Path.name and Path.parent, or Path.parts

[dosisod]

One thing to note is that os.path.join() will drop falsey arguments, but os.sep.join() won't:

>>> import os
>>> os.path.join("hello", "world")
'hello/world'
>>> os.sep.join(("hello", "", "world"))
'hello//world'

This isn't an issue when using it for the Path() constructor, but might be an issue if generalized to all uses of os.path.join():

>>> from pathlib import Path
>>> Path("hello", "", "world") == Path("hello", "world")
True