fix[lang]: disallow absolute relative imports by sandbubbles · Pull Request #4268 · vyperlang/vyper

sandbubbles · 2024-10-01T14:55:02Z

What I did

fix import system allows absolute relative imports #4184

How I did it

Removed implicit relative imports in submodules, eliminating ambiguity in import paths.
Removed the lines that added the current directory to the top of the search paths. As a result, the poke_search_path function may no longer be necessary.

How to verify it

added tests

Commit message

this commit changes the behavior of search paths within
submodules. previously the current module's parent was appended to
the search path in the import recursion. this means that given the
following directory structure, this code is legal:

```
# directory structure:
# subdir/foo.vy
# subdir/bar.vy
```

```
# subdir/foo.vy
import bar
```

which has the same semantics as

```
# subdir/foo.vy
from . import bar
```

this represented a divergence wrt python's import system. in python,
only the second example is allowed.

this commit changes the semantics to match python, so that relative
imports use the current directory as search path, but absolute imports
can only use the top-level search path. (the directory containing the
top-level compilation target is still included in the search path, but
this only happens at the top of the recursion, same as in python -
https://docs.python.org/3/library/sys_path_init.html).

misc/refactor:
- remove dead `add_search_path()` method
- remove now-dead `poke_search_path()`

Description for the changelog

Cute Animal Picture

charles-cooper

yes, i think poke_search_paths is probably dead now. can you check? if so, let's remove it!

charles-cooper · 2024-10-01T15:18:49Z

vyper/semantics/analysis/module.py

@@ -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:


i guess we can fuse load_import_helper right?

vyper/semantics/analysis/module.py

…bsolute search paths

codecov · 2024-10-15T16:32:05Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.46%. Comparing base (ebe26a6) to head (be01034).
Report is 73 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4268      +/-   ##
==========================================
- Coverage   91.12%   88.46%   -2.67%     
==========================================
  Files         115      115              
  Lines       16235    16236       +1     
  Branches     2732     2733       +1     
==========================================
- Hits        14794    14363     -431     
- Misses       1004     1352     +348     
- Partials      437      521      +84

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

vyper/semantics/analysis/imports.py

tests/unit/cli/vyper_json/test_compile_json.py

cyberthirst · 2024-10-17T10:42:48Z

@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

charles-cooper · 2024-11-07T08:53:40Z

@sandbubbles please resolve merge conflicts

charles-cooper · 2024-12-06T02:35:33Z

vyper/semantics/analysis/imports.py

+            res = self.input_bundle.load_file(path)
+
+        if level != 0:
+            self.input_bundle.search_paths += [current_search_path]


Suggested change

self.input_bundle.search_paths += [current_search_path]

self.input_bundle.search_paths.append(current_search_path)

this is kind of weird actually because on line 221, we already set search_paths to [current_search_path]

I think the issue is we need to set it temporarily to only one path, but then we need to add it to search paths in case there is an absolute import that uses it

isn't that handled in the recursion by line 221?
https://github.com/vyperlang/vyper/pull/4268/files/c91f13e870bce8001eeec83dd96b8320c53d4343#diff-dc7dbf75948a3fce0db2cfa0fa819c964a534604d06f72ae88d18e3e3eef78b6R221

i pushed a clean up in a140226

I must have though that example such as the following should work (but its what we were fixing in the first place).

#a.vy from .subdir import b

and

#b.vy from subdir.subsubdir import c

sandbubbles · 2024-12-11T09:58:06Z

This should be ready for rereview

vyper/semantics/analysis/imports.py

tests/functional/syntax/test_import.py

charles-cooper

lgtm, thank you

### 🕓 Changelog This commit updates the version `pragma`s in all Vyper source files to target the latest `master` version `v0.4.1b4`, aligning with the release of Vyper's newest beta version, [`v0.4.1b3`](https://github.com/vyperlang/vyper/releases/tag/v0.4.1b3). Additionally, this PR addresses a disallowed pattern in the `IERC1155MetadataURI.vyi` interface definition by fixing the use of absolute relative imports, which are prohibited in Vyper since PR [#4268](vyperlang/vyper#4268). --------- Signed-off-by: Pascal Marco Caversaccio <[email protected]>

fix absolute relative paths

84f9c70

sandbubbles changed the title ~~fix[lang]: disallow absolute relative paths~~ fix[lang]: disallow absolute relative imports Oct 1, 2024

charles-cooper reviewed Oct 1, 2024

View reviewed changes

sandbubbles added 4 commits October 1, 2024 18:34

lint

0a69fca

fix path from absolute to relative

57f7abd

add test case for correct relative path

0db6330

make relative paths work again

b1dee66

charles-cooper reviewed Oct 3, 2024

View reviewed changes

vyper/semantics/analysis/module.py Outdated Show resolved Hide resolved

sandbubbles added 3 commits October 5, 2024 09:57

fix that relatives paths could break out of current directory

bc7d569

add test for relative paths breaking out of current directory

64ed8c3

Merge branch 'master' into fix/relative-paths

7e3e20a

cyberthirst mentioned this pull request Oct 7, 2024

feat[tool]: separate import resolution pass #4229

Merged

sandbubbles added 8 commits October 15, 2024 10:33

merge master

6207819

add _load_file with modifies search paths to a relative path or all a…

0463a79

…bsolute search paths

add tests

932e09a

add resolved paths to import tests

67cb7d3

lint

052e895

revert back to old load_file

a4edbe9

fuse _load_import with _load_import_helper

32eb376

lint

e20b381

charles-cooper reviewed Oct 15, 2024

View reviewed changes

vyper/semantics/analysis/imports.py Outdated Show resolved Hide resolved

charles-cooper reviewed Oct 15, 2024

View reviewed changes

vyper/semantics/analysis/imports.py Outdated Show resolved Hide resolved

sandbubbles added 3 commits October 15, 2024 19:20

fix typo and remove multiline expr

a843639

remove newly redundant methods

b98ea67

lint

d75c292

charles-cooper reviewed Oct 16, 2024

View reviewed changes

tests/unit/cli/vyper_json/test_compile_json.py Outdated Show resolved Hide resolved

Merge branch 'master' into fix/relative-paths

141ed2f

sandbubbles requested a review from charles-cooper November 7, 2024 08:44

charles-cooper requested a review from cyberthirst November 7, 2024 08:52

merge master

85e7eea

charles-cooper reviewed Dec 6, 2024

View reviewed changes

cyberthirst added the release - must release blocker label Dec 11, 2024

sandbubbles added 2 commits December 11, 2024 10:56

change "+=" to "append"

f0f325a

Merge branch 'master' into fix/relative-paths

c91f13e

sandbubbles requested a review from charles-cooper December 12, 2024 15:19

charles-cooper added this to the v0.4.1 milestone Dec 12, 2024

charles-cooper reviewed Dec 13, 2024

View reviewed changes

vyper/semantics/analysis/imports.py Outdated Show resolved Hide resolved

charles-cooper added 3 commits December 13, 2024 10:00

clean up recursion

a140226

fix lint

99f0af2

clean up test

3359eae

charles-cooper reviewed Dec 13, 2024

View reviewed changes

tests/functional/syntax/test_import.py Show resolved Hide resolved

minor clean up for some tests

f67dee8

charles-cooper reviewed Dec 13, 2024

View reviewed changes

tests/functional/syntax/test_import.py Show resolved Hide resolved

charles-cooper and others added 2 commits December 13, 2024 10:28

some more test cleanup

69ce28e

clean up tests some more

0ad5e7f

cyberthirst approved these changes Dec 16, 2024

View reviewed changes

Merge branch 'master' into fix/relative-paths

be01034

charles-cooper approved these changes Dec 17, 2024

View reviewed changes

charles-cooper enabled auto-merge (squash) December 17, 2024 18:48

charles-cooper merged commit b0ea1b3 into vyperlang:master Dec 17, 2024

This was referenced Dec 18, 2024

Improve error message for absolute relative imports #4408

Closed

🔁 Update 🐍Vyper Version pragma to v0.4.1b4 pcaversaccio/snekmate#293

Merged

	self.input_bundle.search_paths += [current_search_path]
	self.input_bundle.search_paths.append(current_search_path)

Uh oh!

Conversation

sandbubbles commented Oct 1, 2024 • edited by charles-cooper Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How I did it

How to verify it

Commit message

Description for the changelog

Cute Animal Picture

Uh oh!

charles-cooper left a comment

Choose a reason for hiding this comment

Uh oh!

charles-cooper Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cyberthirst commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charles-cooper commented Nov 7, 2024

Uh oh!

charles-cooper Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

charles-cooper Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

sandbubbles Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

charles-cooper Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

charles-cooper Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

sandbubbles Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandbubbles commented Dec 11, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

charles-cooper left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sandbubbles commented Oct 1, 2024 •

edited by charles-cooper

Loading

codecov bot commented Oct 15, 2024 •

edited

Loading

cyberthirst commented Oct 17, 2024 •

edited

Loading

sandbubbles Dec 14, 2024 •

edited

Loading