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

Allow NatSpec comments for variables #3418

Closed
1 task
fulldecent opened this issue Jan 21, 2018 · 12 comments · Fixed by #8532
Closed
1 task

Allow NatSpec comments for variables #3418

fulldecent opened this issue Jan 21, 2018 · 12 comments · Fixed by #8532
Assignees
Labels
language design :rage4: Any changes to the language, e.g. new features
Milestone

Comments

@fulldecent
Copy link
Contributor

fulldecent commented Jan 21, 2018

Test case

pragma solidity ^0.5.0;

/// @title A simulator for Bug Bunny, the most famous Rabbit
/// @author Warned Bros
/// @notice You can use this contract for only the most basic simulation
/// @dev All function calls are currently implement without side effects
contract BugsBunny {
    /// @author Bob Clampett
    /// @notice Determine if Bugs will or will not accept given food
    /// @dev Food must be provided in UTF-8 format
    /// @param _food The name of a food to evaluate (English)
    /// @return true if Bugs will eat it, false otherwise
    function doesEat(string calldata _food) external pure returns (bool) {
        return keccak256(bytes(_food)) == keccak256("carrot");
    }
    
    /// @title example of title
    /// @author example of author
    /// @notice example of notice
    /// @dev example of dev
    /// @param example of param
    /// @return example of return
    string public constant name = "Bugs";
}

Run the command:

solc -o outputDirectory --userdoc --devdoc tmp.sol

Then test output with:

jq .methods outputDirectory/*

Expected outcome

Documentation is output for doesEat and name public functions.

Actual outcome

{
  "doesEat(string)": {
    "author": "Bob Clampett",
    "details": "Food must be provided in UTF-8 format",
    "params": {
      "_food": "The name of a food to evaluate (English)"
    },
    "return": "true if Bugs will eat it, false otherwise"
  }
}
{
  "doesEat(string)": {
    "notice": "Determine if Bugs will or will not accept given food"
  }
}

Follow on work

@axic
Copy link
Member

axic commented Jan 29, 2018

This depends on accepting #3411 first.

@fulldecent
Copy link
Contributor Author

#3411 is closed

@axic axic added the feature label Feb 21, 2018
@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
barakman added a commit to barakman/solidity-docgen that referenced this issue Sep 3, 2019
Since NatSpec currently does NOT apply to public state variables (see ethereum/solidity#3418).
frangio pushed a commit to OpenZeppelin/solidity-docgen that referenced this issue Nov 6, 2019
* Support the documentation of global variables

Following [this feature-request](#43), extending the `solidity-docgen` package to support the documentation of global variables, which are implicitly added by the compiler as getter functions in the contract.

This should allow users to get the natspec-documentation of their global variables added into the `solidity-docgen` output by extending the input template (hbs) file.

For example:
```
{{#if ownVariables}}
# Variables:
{{#ownVariables}}
- [`{{type}} {{name}}`](#{{anchor}})
{{/ownVariables}}
{{/if}}

{{#ownVariables}}
# Variable `{{type}} {{name}}` {#{{anchor~}} }
{{#if natspec.devdoc}}{{natspec.devdoc}}{{else}}No description{{/if}}
{{/ownVariables}}
```
Note that this PR is incomplete, since the `devdoc` of a global variable is always `undefined`.

For this reason, I also had to override function `SolidityContractItem.natspec()` in class `SolidityVariable` with a slightly different implementation (since `this.astNode.documentation === undefined`).

* Fix function `SolidityVariable.type()`

* Throw error in function SolidityVariable.natspec

Since NatSpec currently does NOT apply to public state variables (see ethereum/solidity#3418).

* Add `this.variables` to the array returned by `linkable`

As requested at #44 (comment).

* Update src/solidity.ts

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Add the definition of `interface VariableDeclaration`

As requested at #44 (comment).

* Fix the `VariableDeclaration` interface

Remove the n/a `documentation` field.

* Fix class `SolidityVariable`

Let this class implement `Linkable` instead of extending `SolidityContractItem`.

* add missing semicolons and commas

* rename typeName to type

* add necessary members in variable ast node interface

* remove unused constructor argument

* finish implementation of state variable documentation

* add tests for state variables

* add warning for state variable natspec
@fulldecent
Copy link
Contributor Author

I do not think this depends on #3411 (CLOSED, WONTFIX).

The #3411 only relates to variables in interfaces. But this issue regards variables in contracts.

@bshastry
Copy link
Contributor

bshastry commented Dec 2, 2019

summary from the call: waiting for further clarification from @fulldecent

@fulldecent
Copy link
Contributor Author

Updated issue with test case for 0.6

@fulldecent
Copy link
Contributor Author

Also added PR for documentation (which is blocked against here). #7857

@fulldecent
Copy link
Contributor Author

Related #7834 #7835

@elenadimitrova elenadimitrova added this to the Sprint 1 milestone Jan 27, 2020
@chriseth chriseth modified the milestones: Sprint 1, Sprint 2 Feb 12, 2020
@erak erak self-assigned this Feb 19, 2020
@elenadimitrova elenadimitrova modified the milestones: Sprint 2, Sprint 3 Feb 26, 2020
@erak erak removed their assignment Mar 5, 2020
@erak
Copy link
Collaborator

erak commented Mar 5, 2020

This is up for grabs again. Please see #8399.

@elenadimitrova elenadimitrova modified the milestones: Sprint 3, Sprint 4 Mar 11, 2020
@axic
Copy link
Member

axic commented Apr 25, 2020

Since #3514 was fixed, the (in my opinion) best pattern is using interfaces to define public functions and those have to be NatSpec documented. Interfaces can't have "getters", but getters can implement functions of interfaces. This removes the need for NatSpec documenting internal functions -- probably the bigger questions is defining the goals and use case of NatSpec to properly decide its features.

@fulldecent
Copy link
Contributor Author

Yes agreed. interface is now working great and it is the primary way to document interfaces and that is the primary use for NatSpec.

All is good in the world, there is no need for this feature. Closing.

There is no harm in implementing this feature and it can be opened later if desired but there is no need for it.

@axic
Copy link
Member

axic commented May 21, 2020

Hm, it seems this was a duplicate for #2795.

@Marenz
Copy link
Contributor

Marenz commented Jun 22, 2020

@axic I am not sure why you reopened this, isn't this fixed by #8532

@Marenz Marenz closed this as completed Jun 22, 2020
douglasletz added a commit to douglasletz/pickle-solidity-pro that referenced this issue Sep 15, 2021
* Support the documentation of global variables

Following [this feature-request](OpenZeppelin/solidity-docgen#43), extending the `solidity-docgen` package to support the documentation of global variables, which are implicitly added by the compiler as getter functions in the contract.

This should allow users to get the natspec-documentation of their global variables added into the `solidity-docgen` output by extending the input template (hbs) file.

For example:
```
{{#if ownVariables}}
# Variables:
{{#ownVariables}}
- [`{{type}} {{name}}`](#{{anchor}})
{{/ownVariables}}
{{/if}}

{{#ownVariables}}
# Variable `{{type}} {{name}}` {#{{anchor~}} }
{{#if natspec.devdoc}}{{natspec.devdoc}}{{else}}No description{{/if}}
{{/ownVariables}}
```
Note that this PR is incomplete, since the `devdoc` of a global variable is always `undefined`.

For this reason, I also had to override function `SolidityContractItem.natspec()` in class `SolidityVariable` with a slightly different implementation (since `this.astNode.documentation === undefined`).

* Fix function `SolidityVariable.type()`

* Throw error in function SolidityVariable.natspec

Since NatSpec currently does NOT apply to public state variables (see ethereum/solidity#3418).

* Add `this.variables` to the array returned by `linkable`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Update src/solidity.ts

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Add the definition of `interface VariableDeclaration`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Fix the `VariableDeclaration` interface

Remove the n/a `documentation` field.

* Fix class `SolidityVariable`

Let this class implement `Linkable` instead of extending `SolidityContractItem`.

* add missing semicolons and commas

* rename typeName to type

* add necessary members in variable ast node interface

* remove unused constructor argument

* finish implementation of state variable documentation

* add tests for state variables

* add warning for state variable natspec
Ishanamgai added a commit to Ishanamgai/F-Solidity that referenced this issue Feb 4, 2022
* Support the documentation of global variables

Following [this feature-request](OpenZeppelin/solidity-docgen#43), extending the `solidity-docgen` package to support the documentation of global variables, which are implicitly added by the compiler as getter functions in the contract.

This should allow users to get the natspec-documentation of their global variables added into the `solidity-docgen` output by extending the input template (hbs) file.

For example:
```
{{#if ownVariables}}
# Variables:
{{#ownVariables}}
- [`{{type}} {{name}}`](#{{anchor}})
{{/ownVariables}}
{{/if}}

{{#ownVariables}}
# Variable `{{type}} {{name}}` {#{{anchor~}} }
{{#if natspec.devdoc}}{{natspec.devdoc}}{{else}}No description{{/if}}
{{/ownVariables}}
```
Note that this PR is incomplete, since the `devdoc` of a global variable is always `undefined`.

For this reason, I also had to override function `SolidityContractItem.natspec()` in class `SolidityVariable` with a slightly different implementation (since `this.astNode.documentation === undefined`).

* Fix function `SolidityVariable.type()`

* Throw error in function SolidityVariable.natspec

Since NatSpec currently does NOT apply to public state variables (see ethereum/solidity#3418).

* Add `this.variables` to the array returned by `linkable`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Update src/solidity.ts

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Add the definition of `interface VariableDeclaration`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Fix the `VariableDeclaration` interface

Remove the n/a `documentation` field.

* Fix class `SolidityVariable`

Let this class implement `Linkable` instead of extending `SolidityContractItem`.

* add missing semicolons and commas

* rename typeName to type

* add necessary members in variable ast node interface

* remove unused constructor argument

* finish implementation of state variable documentation

* add tests for state variables

* add warning for state variable natspec
topsuperdev added a commit to topsuperdev/Solidity-Pro that referenced this issue Jun 14, 2022
* Support the documentation of global variables

Following [this feature-request](OpenZeppelin/solidity-docgen#43), extending the `solidity-docgen` package to support the documentation of global variables, which are implicitly added by the compiler as getter functions in the contract.

This should allow users to get the natspec-documentation of their global variables added into the `solidity-docgen` output by extending the input template (hbs) file.

For example:
```
{{#if ownVariables}}
# Variables:
{{#ownVariables}}
- [`{{type}} {{name}}`](#{{anchor}})
{{/ownVariables}}
{{/if}}

{{#ownVariables}}
# Variable `{{type}} {{name}}` {#{{anchor~}} }
{{#if natspec.devdoc}}{{natspec.devdoc}}{{else}}No description{{/if}}
{{/ownVariables}}
```
Note that this PR is incomplete, since the `devdoc` of a global variable is always `undefined`.

For this reason, I also had to override function `SolidityContractItem.natspec()` in class `SolidityVariable` with a slightly different implementation (since `this.astNode.documentation === undefined`).

* Fix function `SolidityVariable.type()`

* Throw error in function SolidityVariable.natspec

Since NatSpec currently does NOT apply to public state variables (see ethereum/solidity#3418).

* Add `this.variables` to the array returned by `linkable`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Update src/solidity.ts

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Add the definition of `interface VariableDeclaration`

As requested at OpenZeppelin/solidity-docgen#44 (comment).

* Fix the `VariableDeclaration` interface

Remove the n/a `documentation` field.

* Fix class `SolidityVariable`

Let this class implement `Linkable` instead of extending `SolidityContractItem`.

* add missing semicolons and commas

* rename typeName to type

* add necessary members in variable ast node interface

* remove unused constructor argument

* finish implementation of state variable documentation

* add tests for state variables

* add warning for state variable natspec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language design :rage4: Any changes to the language, e.g. new features
Projects
None yet
9 participants