|
| 1 | +# Suggestion for PR #137 |
| 2 | + |
| 3 | +The current fix using `rsplit()` is better than the original `split()`, but it can still fail in edge cases where directory names repeat patterns. |
| 4 | + |
| 5 | +## The Problem |
| 6 | +Consider a file path like `/abc/abcd/abc/abcd/file.py` where `root_module = "abc"`: |
| 7 | +- Current approach: `rsplit(f"/abc/", 1)` splits on the last `/abc/`, producing `abcd/file.py` |
| 8 | +- Result: `abc.abcd.file` instead of the correct `abc.abcd.abc.abcd.file` |
| 9 | + |
| 10 | +## More Robust Solution |
| 11 | +Instead of relying on string parsing with the module name, get the actual filesystem location of the root module and remove it from the file path: |
| 12 | + |
| 13 | +```python |
| 14 | +def get_module_path_from_file_path(file_path: str, root_module: str) -> str | None: |
| 15 | + import importlib.util |
| 16 | + |
| 17 | + # Validate inputs |
| 18 | + if ( |
| 19 | + not file_path.endswith(".py") |
| 20 | + or not os.path.exists(file_path) |
| 21 | + or f"/{root_module}/" not in file_path |
| 22 | + ): |
| 23 | + return None |
| 24 | + |
| 25 | + # Get the actual location of the root module |
| 26 | + try: |
| 27 | + spec = importlib.util.find_spec(root_module) |
| 28 | + if spec is None or spec.origin is None: |
| 29 | + return None |
| 30 | + |
| 31 | + module_dir = os.path.dirname(spec.origin) |
| 32 | + # Go up one level if it's an __init__.py |
| 33 | + if os.path.basename(spec.origin) == "__init__.py": |
| 34 | + module_dir = os.path.dirname(module_dir) |
| 35 | + except (ImportError, AttributeError, ValueError): |
| 36 | + return None |
| 37 | + |
| 38 | + # Get relative path from module directory |
| 39 | + try: |
| 40 | + relative_path = os.path.relpath(file_path, module_dir) |
| 41 | + except ValueError: |
| 42 | + # Paths on different drives on Windows |
| 43 | + return None |
| 44 | + |
| 45 | + # Convert to module path |
| 46 | + if relative_path.endswith(".py"): |
| 47 | + relative_path = relative_path[:-3] |
| 48 | + module_path = root_module + "." + relative_path.replace(os.sep, ".") |
| 49 | + |
| 50 | + return module_path |
| 51 | +``` |
| 52 | + |
| 53 | +## Benefits |
| 54 | +- **No string parsing vulnerability** - Works regardless of repeated naming patterns |
| 55 | +- **Uses actual filesystem paths** - More reliable and maintainable |
| 56 | +- **Handles edge cases** - Works with __init__.py files and package structures |
| 57 | +- **More explicit** - The intent is clearer to future maintainers |
| 58 | + |
| 59 | +This approach is immune to all naming collision edge cases! |
0 commit comments