Mailing List Archive

gh-117381: Improve error messages for ntpath.commonpath() (GH-117382)
https://github.com/python/cpython/commit/2ec6bb4111d2c03c1cac02b27c74beee7e5a2a05
commit: 2ec6bb4111d2c03c1cac02b27c74beee7e5a2a05
branch: main
author: Nice Zombies <nineteendo19d0@gmail.com>
committer: serhiy-storchaka <storchaka@gmail.com>
date: 2024-04-03T16:10:09+03:00
summary:

gh-117381: Improve error messages for ntpath.commonpath() (GH-117382)

files:
A Misc/NEWS.d/next/Core and Builtins/2024-03-29-21-43-19.gh-issue-117381.fT0JFM.rst
M Lib/ntpath.py
M Lib/test/test_ntpath.py

diff --git a/Lib/ntpath.py b/Lib/ntpath.py
index 0650f14f89f10b..f9f6c78566e8ed 100644
--- a/Lib/ntpath.py
+++ b/Lib/ntpath.py
@@ -857,9 +857,6 @@ def commonpath(paths):
drivesplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths]
split_paths = [p.split(sep) for d, r, p in drivesplits]

- if len({r for d, r, p in drivesplits}) != 1:
- raise ValueError("Can't mix absolute and relative paths")
-
# Check that all drive letters or UNC paths match. The check is made only
# now otherwise type errors for mixing strings and bytes would not be
# caught.
@@ -867,6 +864,12 @@ def commonpath(paths):
raise ValueError("Paths don't have the same drive")

drive, root, path = splitroot(paths[0].replace(altsep, sep))
+ if len({r for d, r, p in drivesplits}) != 1:
+ if drive:
+ raise ValueError("Can't mix absolute and relative paths")
+ else:
+ raise ValueError("Can't mix rooted and not-rooted paths")
+
common = path.split(sep)
common = [.c for c in common if c and c != curdir]

diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py
index c816f99e7e9f1b..31156130fcc747 100644
--- a/Lib/test/test_ntpath.py
+++ b/Lib/test/test_ntpath.py
@@ -866,46 +866,47 @@ def test_commonpath(self):
def check(paths, expected):
tester(('ntpath.commonpath(%r)' % paths).replace('\\\\', '\\'),
expected)
- def check_error(exc, paths):
- self.assertRaises(exc, ntpath.commonpath, paths)
- self.assertRaises(exc, ntpath.commonpath,
- [os.fsencode(p) for p in paths])
+ def check_error(paths, expected):
+ self.assertRaisesRegex(ValueError, expected, ntpath.commonpath, paths)
+ self.assertRaisesRegex(ValueError, expected, ntpath.commonpath, paths[::-1])
+ self.assertRaisesRegex(ValueError, expected, ntpath.commonpath,
+ [os.fsencode(p) for p in paths])
+ self.assertRaisesRegex(ValueError, expected, ntpath.commonpath,
+ [os.fsencode(p) for p in paths[::-1]])

self.assertRaises(TypeError, ntpath.commonpath, None)
self.assertRaises(ValueError, ntpath.commonpath, [])
self.assertRaises(ValueError, ntpath.commonpath, iter([]))
- check_error(ValueError, ['C:\\Program Files', 'Program Files'])
- check_error(ValueError, ['C:\\Program Files', 'C:Program Files'])
- check_error(ValueError, ['\\Program Files', 'Program Files'])
- check_error(ValueError, ['Program Files', 'C:\\Program Files'])
-
- check(['C:\\Program Files'], 'C:\\Program Files')
- check(['C:\\Program Files', 'C:\\Program Files'], 'C:\\Program Files')
- check(['C:\\Program Files\\', 'C:\\Program Files'],
- 'C:\\Program Files')
- check(['C:\\Program Files\\', 'C:\\Program Files\\'],
- 'C:\\Program Files')
- check(['C:\\\\Program Files', 'C:\\Program Files\\\\'],
- 'C:\\Program Files')
- check(['C:\\.\\Program Files', 'C:\\Program Files\\.'],
- 'C:\\Program Files')
- check(['C:\\', 'C:\\bin'], 'C:\\')
- check(['C:\\Program Files', 'C:\\bin'], 'C:\\')
- check(['C:\\Program Files', 'C:\\Program Files\\Bar'],
- 'C:\\Program Files')
- check(['C:\\Program Files\\Foo', 'C:\\Program Files\\Bar'],
- 'C:\\Program Files')
- check(['C:\\Program Files', 'C:\\Projects'], 'C:\\')
- check(['C:\\Program Files\\', 'C:\\Projects'], 'C:\\')
-
- check(['C:\\Program Files\\Foo', 'C:/Program Files/Bar'],
- 'C:\\Program Files')
- check(['C:\\Program Files\\Foo', 'c:/program files/bar'],
- 'C:\\Program Files')
- check(['c:/program files/bar', 'C:\\Program Files\\Foo'],
- 'c:\\program files')
-
- check_error(ValueError, ['C:\\Program Files', 'D:\\Program Files'])
+
+ # gh-117381: Logical error messages
+ check_error(['C:\\Foo', 'C:Foo'], "Can't mix absolute and relative paths")
+ check_error(['C:\\Foo', '\\Foo'], "Paths don't have the same drive")
+ check_error(['C:\\Foo', 'Foo'], "Paths don't have the same drive")
+ check_error(['C:Foo', '\\Foo'], "Paths don't have the same drive")
+ check_error(['C:Foo', 'Foo'], "Paths don't have the same drive")
+ check_error(['\\Foo', 'Foo'], "Can't mix rooted and not-rooted paths")
+
+ check(['C:\\Foo'], 'C:\\Foo')
+ check(['C:\\Foo', 'C:\\Foo'], 'C:\\Foo')
+ check(['C:\\Foo\\', 'C:\\Foo'], 'C:\\Foo')
+ check(['C:\\Foo\\', 'C:\\Foo\\'], 'C:\\Foo')
+ check(['C:\\\\Foo', 'C:\\Foo\\\\'], 'C:\\Foo')
+ check(['C:\\.\\Foo', 'C:\\Foo\\.'], 'C:\\Foo')
+ check(['C:\\', 'C:\\baz'], 'C:\\')
+ check(['C:\\Bar', 'C:\\baz'], 'C:\\')
+ check(['C:\\Foo', 'C:\\Foo\\Baz'], 'C:\\Foo')
+ check(['C:\\Foo\\Bar', 'C:\\Foo\\Baz'], 'C:\\Foo')
+ check(['C:\\Bar', 'C:\\Baz'], 'C:\\')
+ check(['C:\\Bar\\', 'C:\\Baz'], 'C:\\')
+
+ check(['C:\\Foo\\Bar', 'C:/Foo/Baz'], 'C:\\Foo')
+ check(['C:\\Foo\\Bar', 'c:/foo/baz'], 'C:\\Foo')
+ check(['c:/foo/bar', 'C:\\Foo\\Baz'], 'c:\\foo')
+
+ # gh-117381: Logical error messages
+ check_error(['C:\\Foo', 'D:\\Foo'], "Paths don't have the same drive")
+ check_error(['C:\\Foo', 'D:Foo'], "Paths don't have the same drive")
+ check_error(['C:Foo', 'D:Foo'], "Paths don't have the same drive")

check(['spam'], 'spam')
check(['spam', 'spam'], 'spam')
@@ -919,20 +920,16 @@ def check_error(exc, paths):

check([''], '')
check(['', 'spam\\alot'], '')
- check_error(ValueError, ['', '\\spam\\alot'])
-
- self.assertRaises(TypeError, ntpath.commonpath,
- [b'C:\\Program Files', 'C:\\Program Files\\Foo'])
- self.assertRaises(TypeError, ntpath.commonpath,
- [b'C:\\Program Files', 'Program Files\\Foo'])
- self.assertRaises(TypeError, ntpath.commonpath,
- [b'Program Files', 'C:\\Program Files\\Foo'])
- self.assertRaises(TypeError, ntpath.commonpath,
- ['C:\\Program Files', b'C:\\Program Files\\Foo'])
- self.assertRaises(TypeError, ntpath.commonpath,
- ['C:\\Program Files', b'Program Files\\Foo'])
- self.assertRaises(TypeError, ntpath.commonpath,
- ['Program Files', b'C:\\Program Files\\Foo'])
+
+ # gh-117381: Logical error messages
+ check_error(['', '\\spam\\alot'], "Can't mix rooted and not-rooted paths")
+
+ self.assertRaises(TypeError, ntpath.commonpath, [b'C:\\Foo', 'C:\\Foo\\Baz'])
+ self.assertRaises(TypeError, ntpath.commonpath, [b'C:\\Foo', 'Foo\\Baz'])
+ self.assertRaises(TypeError, ntpath.commonpath, [b'Foo', 'C:\\Foo\\Baz'])
+ self.assertRaises(TypeError, ntpath.commonpath, ['C:\\Foo', b'C:\\Foo\\Baz'])
+ self.assertRaises(TypeError, ntpath.commonpath, ['C:\\Foo', b'Foo\\Baz'])
+ self.assertRaises(TypeError, ntpath.commonpath, ['Foo', b'C:\\Foo\\Baz'])

@unittest.skipIf(is_emscripten, "Emscripten cannot fstat unnamed files.")
def test_sameopenfile(self):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-03-29-21-43-19.gh-issue-117381.fT0JFM.rst b/Misc/NEWS.d/next/Core and Builtins/2024-03-29-21-43-19.gh-issue-117381.fT0JFM.rst
new file mode 100644
index 00000000000000..88b6c32e971e72
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2024-03-29-21-43-19.gh-issue-117381.fT0JFM.rst
@@ -0,0 +1 @@
+Fix error message for :func:`ntpath.commonpath`.

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-leave@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: list-python-checkins@lists.gossamer-threads.com