fix[lang]: disallow absolute relative imports #4268
Conversation
sandbubbles
changed the title
vyper/semantics/analysis/module.py
Outdated
| @@ -789,11 +789,7 @@ def _add_import( | |||
| # load an InterfaceT or ModuleInfo from an import. | |||
| # raises FileNotFoundError | |||
| def _load_import(self, node: vy_ast.VyperNode, level: int, module_str: str, alias: str) -> Any: | |||
There was a problem hiding this comment.
i guess we can fuse load_import_helper right?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4268 +/- ##
===========================================
- Coverage 91.40% 51.84% -39.57%
===========================================
Files 112 112
Lines 15927 15921 -6
Branches 2694 2695 +1
===========================================
- Hits 14558 8254 -6304
- Misses 935 7055 +6120
- Partials 434 612 +178 ☔ View full report in Codecov by Sentry. |
| vyper.compiler.compile_code( | ||
| code, resolved_path=input_bundle.search_paths[0] / "code.vy", input_bundle=input_bundle | ||
| ) |
There was a problem hiding this comment.
why is this change needed?
There was a problem hiding this comment.
Because it imports by a relative path, it needs the current path of the file. By default it is left to unknown (UNKNOWN_CONTRACT_NAME), so then when we ask for the current directory (by .parent) it gets evaluated to "." instead of the temporary directory vreated by the test. But i think you are right that i added it somewhere unnecessarily.
|
@charles-cooper, have you considered the concept of packages? because we don't have it, we allow relative imports that can point even outside the project's directory |
|
|
||
| assert ( | ||
| compile_code( | ||
| code, resolved_path=(input_bundle.search_paths[0]) / subdirs, input_bundle=input_bundle |
There was a problem hiding this comment.
shouldn't the resolved path end with a file? like code for the 2nd case should be in a/b, right? in which case ..a resolves to a/a and we should get file not found?
yeah i know, those will raise a compiler panic if you try to create an input bundle. i think it might be useful to introduce packages, but it needs some design work, and not sure how useful it would really be. |
i have from .... import lib1
exports: lib1.__interface__so the import targets a file which is in the parent of the project's dir (from which vyper's run) .. and it compiles fine |
vyper/semantics/analysis/imports.py
Outdated
| def _load_file(self, path: PathLike, level: int): | ||
| if level == 0: | ||
| self.input_bundle.search_paths = self.absolute_search_paths | ||
| else: | ||
| ast = self.graph.current_module | ||
| current_search_path = Path(ast.resolved_path).parent | ||
| self.input_bundle.search_paths = [current_search_path] | ||
|
|
||
| with self.input_bundle.poke_search_path(self_search_path): | ||
| return self._load_import_helper(node, level, module_str, alias) | ||
| return self.input_bundle.load_file(path) |
There was a problem hiding this comment.
nit: can we move this after _load_import to match the other functions, which are laid out in reverse dependency order
vyper/semantics/analysis/imports.py
Outdated
| else: | ||
| ast = self.graph.current_module | ||
| current_search_path = Path(ast.resolved_path).parent | ||
| self.input_bundle.search_paths = [current_search_path] |
There was a problem hiding this comment.
i think this corrupts the input bundle's search path, which might be important if anybody tries to access the search path after compilation (for instance as is done in output_bundle.py). i think we need to restore the original .search_paths after load_file is called.
i tried to create an output bundle (vyper -f archive) with this setup and it led to compiler panic |
What I did
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture