jendrikseipp / vulture

Find dead Python code
MIT License
3.48k stars 152 forks source link

Fix formatting of relative paths #246

Closed The-Compiler closed 3 years ago

The-Compiler commented 3 years ago

Prior to vulture 2.2, it was possible to pass an absolute path below the current directory, and the report showed a relative path instead:

With vulture 2.1, inside ~/proj/qutebrowser/git/:

$ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

With vulture 2.3:

$ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

This is due to #226, notably the path.relative_to(os.curdir) in utils.format_path - this won't work properly if the input is absolute:

>>> import os, pathlib
>>> os.getcwd()
'/tmp'
>>> pathlib.Path('/tmp/somefile').relative_to('.')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/pathlib.py", line 928, in relative_to
    raise ValueError("{!r} is not in the subpath of {!r}"
ValueError: '/tmp/somefile' is not in the subpath of '' OR one path is relative and the other is absolute.
>>> pathlib.Path('/tmp/somefile').relative_to(pathlib.Path.cwd())
PosixPath('somefile')

Checklist:

cc @ju-sh

codecov-io commented 3 years ago

Codecov Report

Merging #246 (5fb9d44) into master (d40303b) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #246   +/-   ##
=======================================
  Coverage   99.36%   99.36%           
=======================================
  Files          17       17           
  Lines         631      631           
=======================================
  Hits          627      627           
  Misses          4        4           
Impacted Files Coverage Δ
vulture/utils.py 98.61% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d40303b...5fb9d44. Read the comment docs.

The-Compiler commented 3 years ago

Ah, it looks like vulture turns relative paths absolute internally, so this is even an issue if passed a relative path:

$ vulture qutebrowser/qutebrowser.py
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)
jendrikseipp commented 3 years ago

Good catch. Thanks for taking care of this!