aragon / radspec

🤘 Radspec is a safe interpreter for Ethereum's NatSpec
MIT License
141 stars 36 forks source link

Add support for multiple return values #79

Closed onbjerg closed 5 years ago

onbjerg commented 5 years ago

This PR closes #1 by adding support for multiple return values for calls.

It adds two new tokens (LEFT_BRACKET and RIGHT_BRACKET) and no new AST nodes. It is fully backwards compatible with the single return type syntax.

Example

Imagine that this string describes a function that removes an entry from a registry. The string calls a function, getEntry(uint256): (string title, address author), that returns information about a specific entry in a registry:

Remove entry "`self.getEntry(entry): (string, [address])`" authored by `self.getEntry(entry): (string, [address])`

Output:

Remove entry "Foo" authored by 0x0000000000000000000000000000000000000003

Design Rationale

The syntax for describing a call with multiple return values is a type list (e.g. (address, string)) where one of the types is marked as the type to use for further evaluation by surrounding it with brackets.

I initially wanted to implement a syntax that is more "JavaScript"-y by mimicing the array access syntax (arr[n] for the nth value in an array), but it would lead to extra parenthesis being needed:

Remove entry "`(self.getEntry(entry): (string, address))[0]`" authored by `(self.getEntry(entry): (string, address))[1]`

I realised I could make the syntax more concise while making the implementation easier by implementing it as the final result in this PR.

Final note

There might be a case for using something other than brackets, e.g. angle brackets, to denote what value to return. For example, using brackets as this PR does might lead to confusion with array syntax in some languages.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-1.6%) to 89.648% when pulling e532957d3f216097d0343895e76e40f277005c55 on onbjerg:multiple-return-values into 35a446dbe22515e5aa12987001b6217a702a0bb8 on aragon:master.

sohkai commented 5 years ago

There might be a case for using something other than brackets, e.g. angle brackets, to denote what value to return.

I would agree with this; perhaps we could use <return> instead? At first read, without context, I thought [return] meant this was optional, due to how JSDoc and etc. handle optional parameters.

onbjerg commented 5 years ago

Sure, I can update the PR to use angled brackets if you decide on that :)

onbjerg commented 5 years ago

@sohkai Added the requested test cases and switched [ and ] for < and >

sohkai commented 5 years ago

@onbjerg Just wanted to check your thoughts on https://github.com/aragon/radspec/pull/79#discussion_r334993442.

I'll merge this and release soon :).

sohkai commented 5 years ago

@onbjerg I've just added a few commits: