Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix[tool]: include structs in -f interface output #4294

Merged

Conversation

sandbubbles
Copy link
Collaborator

@sandbubbles sandbubbles commented Oct 12, 2024

What I did

fix #4282.

How I did it

How to verify it

Commit message

the `-f interface` output was missing struct definitions; this commit
fixes the issue.

this commit also does some small code touchups in the
`build_interface_output()` function, and makes the number of newlines
between functions, events and structs consistent - to be 2 `\n`
characters.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@charles-cooper charles-cooper changed the title fix: include structs in interface output fix[tool]: include structs in -f interface output Oct 13, 2024
@charles-cooper charles-cooper enabled auto-merge (squash) October 13, 2024 18:52
@charles-cooper charles-cooper merged commit fc9a220 into vyperlang:master Oct 13, 2024
155 checks passed
Copy link

codecov bot commented Oct 13, 2024

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 46.02%. Comparing base (61f7f4e) to head (5e26a3e).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
vyper/compiler/output.py 68.75% 3 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (61f7f4e) and HEAD (5e26a3e). Click for more details.

HEAD has 137 uploads less than BASE
Flag BASE (61f7f4e) HEAD (5e26a3e)
138 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4294       +/-   ##
===========================================
- Coverage   91.36%   46.02%   -45.34%     
===========================================
  Files         112      112               
  Lines       15849    15858        +9     
  Branches     2685     2688        +3     
===========================================
- Hits        14480     7299     -7181     
- Misses        934     8004     +7070     
- Partials      435      555      +120     

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

@pcaversaccio
Copy link
Collaborator

pcaversaccio commented Oct 14, 2024

I quickly tested the PR, and I think it's incomplete. It does not list the imported (via module) structs. Example from 🐍 snekmate:

vyper -f interface src/snekmate/utils/mocks/batch_distributor_mock.vy

outputs

# Functions

@payable
@external
def distribute_ether(data: Batch):
    ...


@external
def distribute_token(token: .../snekmate/venv/Lib/site-packages/vyper/builtins/interfaces/IERC20.vyi, data: Batch):
    ...

The structs (we can debate if we only want to list Batch in this case):

# Structs

struct Transaction:
    recipient: address
    amount: uint256


struct Batch:
    txns: DynArray[Transaction, 255]

are missing in the interface for batch_distributor_mock.vy. I opened an issue here to track it.

Furthermore (not related to this PR), I would like to highlight that for the usage of interfaces as types displayed within -f interface the output is weird (see above def distribute_token(token: .../snekmate/venv/Lib/site-packages/vyper/builtins/interfaces/IERC20.vyi, data: Batch):. I opened an issue here to track it.

@cyberthirst
Copy link
Collaborator

cyberthirst commented Oct 15, 2024

for output structs we output the names of their fields although they are tuples in the abi, is this intended?

# test.vyi
struct Foo:
    x: uint256

def foo() -> Foo:
    ...

-f abi (see "name": "x",)

[
  {
    "stateMutability": "nonpayable",
    "type": "function",
    "name": "foo",
    "inputs": [],
    "outputs": [
      {
        "name": "",
        "type": "tuple",
        "components": [
          {
            "name": "x",
            "type": "uint256"
          }
        ]
      }
    ]
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vyper -f interface doesn't include structs in the generated code
4 participants