From af8bb5fb60c1909469aa75ecd3ab59ecc7d027ec Mon Sep 17 00:00:00 2001 From: Alexander Arlt Date: Tue, 24 Mar 2020 17:44:39 -0500 Subject: [PATCH] Allow natspec comments on state variables. --- Changelog.md | 2 +- docs/control-structures.rst | 6 +- docs/natspec-format.rst | 6 +- docs/security-considerations.rst | 6 +- docs/types/reference-types.rst | 2 +- libsolidity/analysis/DocStringAnalyser.cpp | 49 ++++-- libsolidity/ast/AST.h | 8 +- libsolidity/ast/ASTJsonConverter.cpp | 5 +- libsolidity/ast/ASTJsonImporter.cpp | 2 +- libsolidity/interface/Natspec.cpp | 27 ++-- libsolidity/parsing/Parser.cpp | 11 +- test/libsolidity/ASTJSON/documentation.json | 139 +++++++++++------- test/libsolidity/ASTJSON/documentation.sol | 1 + .../ASTJSON/documentation_legacy.json | 109 +++++++++----- test/libsolidity/SolidityNatspecJSON.cpp | 72 ++++++--- .../docstring_author_title_state_variable.sol | 7 + ..._non_public_state_variable_with_return.sol | 6 + .../docstring_private_state_variable.sol | 7 + .../natspec/docstring_state_variable.sol | 6 +- ...ng_state_variable_too_many_return_tags.sol | 9 ++ .../natspec/docstring_variable.sol | 1 + 21 files changed, 320 insertions(+), 161 deletions(-) create mode 100644 test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol create mode 100644 test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol create mode 100644 test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol create mode 100644 test/libsolidity/syntaxTests/natspec/docstring_state_variable_too_many_return_tags.sol diff --git a/Changelog.md b/Changelog.md index 54b60bcf6..649b2e147 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,7 +7,7 @@ Compiler Features: * Build system: Update the soljson.js build to emscripten 1.39.15 and boost 1.73.0 and include Z3 for integrated SMTChecker support without the callback mechanism. * SMTChecker: Support array ``length``. * SMTChecker: Support array ``push`` and ``pop``. - + * Add support for natspec comments on state variables. Bugfixes: * Optimizer: Fixed a bug in BlockDeDuplicator. diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 9ecaa838b..811e612c0 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -255,9 +255,9 @@ which only need to be created if there is a dispute. contract C { function createDSalted(bytes32 salt, uint arg) public { - /// This complicated expression just tells you how the address - /// can be pre-computed. It is just there for illustration. - /// You actually only need ``new D{salt: salt}(arg)``. + // This complicated expression just tells you how the address + // can be pre-computed. It is just there for illustration. + // You actually only need ``new D{salt: salt}(arg)``. address predictedAddress = address(uint(keccak256(abi.encodePacked( byte(0xff), address(this), diff --git a/docs/natspec-format.rst b/docs/natspec-format.rst index 18305961e..a0596869a 100644 --- a/docs/natspec-format.rst +++ b/docs/natspec-format.rst @@ -85,10 +85,10 @@ Tag =========== =============================================================================== ============================= ``@title`` A title that should describe the contract/interface contract, interface ``@author`` The name of the author contract, interface, function -``@notice`` Explain to an end user what this does contract, interface, function -``@dev`` Explain to a developer any extra details contract, interface, function +``@notice`` Explain to an end user what this does contract, interface, function, public state variable +``@dev`` Explain to a developer any extra details contract, interface, function, state variable ``@param`` Documents a parameter just like in doxygen (must be followed by parameter name) function -``@return`` Documents the return variables of a contract's function function +``@return`` Documents the return variables of a contract's function function, public state variable =========== =============================================================================== ============================= If your function returns multiple values, like ``(int quotient, int remainder)`` diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index 772fa7c1d..f73cb0dbf 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -63,7 +63,7 @@ complete contract): // THIS CONTRACT CONTAINS A BUG - DO NOT USE contract Fund { - /// Mapping of ether shares of the contract. + /// @dev Mapping of ether shares of the contract. mapping(address => uint) shares; /// Withdraw your share. function withdraw() public { @@ -87,7 +87,7 @@ as it uses ``call`` which forwards all remaining gas by default: // THIS CONTRACT CONTAINS A BUG - DO NOT USE contract Fund { - /// Mapping of ether shares of the contract. + /// @dev Mapping of ether shares of the contract. mapping(address => uint) shares; /// Withdraw your share. function withdraw() public { @@ -106,7 +106,7 @@ outlined further below: pragma solidity >=0.4.11 <0.7.0; contract Fund { - /// Mapping of ether shares of the contract. + /// @dev Mapping of ether shares of the contract. mapping(address => uint) shares; /// Withdraw your share. function withdraw() public { diff --git a/docs/types/reference-types.rst b/docs/types/reference-types.rst index 5a59997c4..2db374959 100644 --- a/docs/types/reference-types.rst +++ b/docs/types/reference-types.rst @@ -415,7 +415,7 @@ Array slices are useful to ABI-decode secondary data passed in function paramete pragma solidity >=0.6.0 <0.7.0; contract Proxy { - /// Address of the client contract managed by proxy i.e., this contract + /// @dev Address of the client contract managed by proxy i.e., this contract address client; constructor(address _client) public { diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index 9385838a9..51f60f790 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -59,9 +59,27 @@ bool DocStringAnalyser::visit(FunctionDefinition const& _function) bool DocStringAnalyser::visit(VariableDeclaration const& _variable) { - if (_variable.isStateVariable() && _variable.isPartOfExternalInterface()) - handleDeclaration(_variable, _variable, _variable.annotation()); - return true; + if (_variable.isStateVariable()) + { + static set const validPublicTags = set{"dev", "notice", "return", "title", "author"}; + if (_variable.isPublic()) + parseDocStrings(_variable, _variable.annotation(), validPublicTags, "public state variables"); + else + { + parseDocStrings(_variable, _variable.annotation(), validPublicTags, "non-public state variables"); + if (_variable.annotation().docTags.count("notice") > 0) + m_errorReporter.warning( + 9098_error, _variable.documentation()->location(), + "Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly." + ); + } + if (_variable.annotation().docTags.count("title") > 0 || _variable.annotation().docTags.count("author") > 0) + m_errorReporter.warning( + 4822_error, _variable.documentation()->location(), + "Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0." + ); + } + return false; } bool DocStringAnalyser::visit(ModifierDefinition const& _modifier) @@ -124,16 +142,6 @@ void DocStringAnalyser::handleCallable( checkParameters(_callable, _node, _annotation); } -void DocStringAnalyser::handleDeclaration( - Declaration const&, - StructurallyDocumented const& _node, - StructurallyDocumentedAnnotation& _annotation -) -{ - static set const validTags = set{"author", "dev", "notice", "return", "param"}; - parseDocStrings(_node, _annotation, validTags, "state variables"); -} - void DocStringAnalyser::parseDocStrings( StructurallyDocumented const& _node, StructurallyDocumentedAnnotation& _annotation, @@ -161,7 +169,20 @@ void DocStringAnalyser::parseDocStrings( if (docTag.first == "return") { returnTagsVisited++; - if (auto* function = dynamic_cast(&_node)) + if (auto* varDecl = dynamic_cast(&_node)) + { + if (!varDecl->isPublic()) + appendError( + _node.documentation()->location(), + "Documentation tag \"@" + docTag.first + "\" is only allowed on public state-variables." + ); + if (returnTagsVisited > 1) + appendError( + _node.documentation()->location(), + "Documentation tag \"@" + docTag.first + "\" is only allowed once on state-variables." + ); + } + else if (auto* function = dynamic_cast(&_node)) { string content = docTag.second.content; string firstWord = content.substr(0, content.find_first_of(" \t")); diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index aacd88507..721e76791 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -882,7 +882,7 @@ public: ASTPointer const& _name, ASTPointer _value, Visibility _visibility, - ASTPointer const& _documentation, + ASTPointer const _documentation = nullptr, bool _isStateVar = false, bool _isIndexed = false, Mutability _mutability = Mutability::Mutable, @@ -890,9 +890,9 @@ public: Location _referenceLocation = Location::Unspecified ): Declaration(_id, _location, _name, _visibility), - StructurallyDocumented(_documentation), - m_typeName(_type), - m_value(_value), + StructurallyDocumented(std::move(_documentation)), + m_typeName(std::move(_type)), + m_value(std::move(_value)), m_isStateVariable(_isStateVar), m_isIndexed(_isIndexed), m_mutability(_mutability), diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp index bfdb92e11..8da1dddd7 100644 --- a/libsolidity/ast/ASTJsonConverter.cpp +++ b/libsolidity/ast/ASTJsonConverter.cpp @@ -390,10 +390,9 @@ bool ASTJsonConverter::visit(VariableDeclaration const& _node) make_pair("typeDescriptions", typePointerToJson(_node.annotation().type, true)) }; if (_node.isStateVariable() && _node.isPublic()) - { attributes.emplace_back("functionSelector", _node.externalIdentifierHex()); - attributes.emplace_back("documentation", _node.documentation() ? toJson(*_node.documentation()) : Json::nullValue); - } + if (_node.isStateVariable() && _node.documentation()) + attributes.emplace_back("documentation", toJson(*_node.documentation())); if (m_inEvent) attributes.emplace_back("indexed", _node.isIndexed()); if (!_node.annotation().baseFunctions.empty()) diff --git a/libsolidity/ast/ASTJsonImporter.cpp b/libsolidity/ast/ASTJsonImporter.cpp index f96e247cc..97e3ba964 100644 --- a/libsolidity/ast/ASTJsonImporter.cpp +++ b/libsolidity/ast/ASTJsonImporter.cpp @@ -441,7 +441,7 @@ ASTPointer ASTJsonImporter::createVariableDeclaration(Json: make_shared(member(_node, "name").asString()), nullOrCast(member(_node, "value")), visibility(_node), - _node["documentation"].isNull() ? nullptr : createDocumentation(member(_node, "documentation")), + _node["documentation"].isNull() ? nullptr : createDocumentation(member(_node, "documentation")), memberAsBool(_node, "stateVariable"), _node.isMember("indexed") ? memberAsBool(_node, "indexed") : false, mutability, diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index ef842c8ef..25dcd794e 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -40,7 +40,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) auto constructorDefinition(_contractDef.constructor()); if (constructorDefinition) { - string value = extractDoc(constructorDefinition->annotation().docTags, "notice"); + string const value = extractDoc(constructorDefinition->annotation().docTags, "notice"); if (!value.empty()) // add the constructor, only if we have any documentation to add methods["constructor"] = Json::Value(value); @@ -64,8 +64,9 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) methods[it.second->externalSignature()] = user; } } - if (auto var = dynamic_cast(&it.second->declaration())) + else if (auto var = dynamic_cast(&it.second->declaration())) { + solAssert(var->isStateVariable() && var->isPublic(), ""); string value = extractDoc(var->annotation().docTags, "notice"); if (!value.empty()) { @@ -121,22 +122,22 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) if (!method.empty()) methods[it.second->externalSignature()] = method; } - if (auto var = dynamic_cast(&it.second->declaration())) - { - Json::Value method(devDocumentation(var->annotation().docTags)); - if (!method.empty()) - { -// Json::Value jsonReturn = extractReturnParameterDocs(var->annotation().docTags, *var); + } -// if (!jsonReturn.empty()) -// method["returns"] = jsonReturn; + Json::Value stateVariables(Json::objectValue); + for (VariableDeclaration const* varDecl: _contractDef.stateVariables()) + { + if (auto devDoc = devDocumentation(varDecl->annotation().docTags); !devDoc.empty()) + stateVariables[varDecl->name()] = devDoc; - methods[it.second->externalSignature()] = method; - } - } + solAssert(varDecl->annotation().docTags.count("return") <= 1, ""); + if (varDecl->annotation().docTags.count("return") == 1) + stateVariables[varDecl->name()]["return"] = extractDoc(varDecl->annotation().docTags, "return"); } doc["methods"] = methods; + if (!stateVariables.empty()) + doc["stateVariables"] = stateVariables; return doc; } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 884a0765a..c429ba786 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -686,7 +686,7 @@ ASTPointer Parser::parseVariableDeclaration( ASTNodeFactory nodeFactory = _lookAheadArrayType ? ASTNodeFactory(*this, _lookAheadArrayType) : ASTNodeFactory(*this); ASTPointer type; - ASTPointer documentation = parseStructuredDocumentation(); + ASTPointer const documentation = parseStructuredDocumentation(); if (_lookAheadArrayType) type = _lookAheadArrayType; else @@ -696,8 +696,8 @@ ASTPointer Parser::parseVariableDeclaration( nodeFactory.setEndPositionFromNode(type); } -// if (!_options.isStateVariable && documentation != nullptr) -// fatalParserError("Only state variables can retrieve a docstring."); + if (!_options.isStateVariable && documentation != nullptr) + parserWarning(2837_error, "Only state variables can have a docstring. This will be disallowed in 0.7.0."); if (dynamic_cast(type.get()) && _options.isStateVariable && m_scanner->currentToken() == Token::LBrace) fatalParserError( @@ -813,7 +813,7 @@ ASTPointer Parser::parseVariableDeclaration( identifier, value, visibility, - documentation, + documentation, _options.isStateVariable, isIndexed, mutability, @@ -1579,8 +1579,7 @@ ASTPointer Parser::parseVariableDeclarationStateme ASTPointer(), name, ASTPointer(), - Visibility::Default, - nullptr + Visibility::Default ); } variables.push_back(var); diff --git a/test/libsolidity/ASTJSON/documentation.json b/test/libsolidity/ASTJSON/documentation.json index b4305969e..29e0b17b9 100644 --- a/test/libsolidity/ASTJSON/documentation.json +++ b/test/libsolidity/ASTJSON/documentation.json @@ -87,10 +87,10 @@ { "C": [ - 20 + 23 ] }, - "id": 21, + "id": 24, "license": null, "nodeType": "SourceUnit", "nodes": @@ -102,90 +102,129 @@ "contractKind": "contract", "documentation": null, "fullyImplemented": true, - "id": 20, + "id": 23, "linearizedBaseContracts": [ - 20 + 23 ], "name": "C", "nodeType": "ContractDefinition", "nodes": [ { - "anonymous": false, + "constant": false, "documentation": { "id": 7, "nodeType": "StructuredDocumentation", - "src": "15:26:3", - "text": "Some comment on Evt." + "src": "15:32:3", + "text": "Some comment on state var." }, + "functionSelector": "c19d93fb", "id": 9, - "name": "Evt", - "nodeType": "EventDefinition", - "parameters": + "mutability": "mutable", + "name": "state", + "nodeType": "VariableDeclaration", + "overrides": null, + "scope": 23, + "src": "48:17:3", + "stateVariable": true, + "storageLocation": "default", + "typeDescriptions": + { + "typeIdentifier": "t_uint256", + "typeString": "uint256" + }, + "typeName": { "id": 8, - "nodeType": "ParameterList", - "parameters": [], - "src": "51:2:3" + "name": "uint", + "nodeType": "ElementaryTypeName", + "src": "48:4:3", + "typeDescriptions": + { + "typeIdentifier": "t_uint256", + "typeString": "uint256" + } }, - "src": "42:12:3" + "value": null, + "visibility": "public" }, { - "body": - { - "id": 13, - "nodeType": "Block", - "src": "99:6:3", - "statements": - [ - { - "id": 12, - "nodeType": "PlaceholderStatement", - "src": "101:1:3" - } - ] - }, + "anonymous": false, "documentation": { "id": 10, "nodeType": "StructuredDocumentation", - "src": "57:26:3", - "text": "Some comment on mod." + "src": "69:26:3", + "text": "Some comment on Evt." }, - "id": 14, - "name": "mod", - "nodeType": "ModifierDefinition", - "overrides": null, + "id": 12, + "name": "Evt", + "nodeType": "EventDefinition", "parameters": { "id": 11, "nodeType": "ParameterList", "parameters": [], - "src": "96:2:3" + "src": "105:2:3" }, - "src": "84:21:3", + "src": "96:12:3" + }, + { + "body": + { + "id": 16, + "nodeType": "Block", + "src": "153:6:3", + "statements": + [ + { + "id": 15, + "nodeType": "PlaceholderStatement", + "src": "155:1:3" + } + ] + }, + "documentation": + { + "id": 13, + "nodeType": "StructuredDocumentation", + "src": "111:26:3", + "text": "Some comment on mod." + }, + "id": 17, + "name": "mod", + "nodeType": "ModifierDefinition", + "overrides": null, + "parameters": + { + "id": 14, + "nodeType": "ParameterList", + "parameters": [], + "src": "150:2:3" + }, + "src": "138:21:3", "virtual": false, "visibility": "internal" }, { "body": { - "id": 18, + "id": 21, "nodeType": "Block", - "src": "155:2:3", + "src": "209:2:3", "statements": [] }, "documentation": { - "id": 15, + "id": 18, "nodeType": "StructuredDocumentation", - "src": "108:25:3", + "src": "162:25:3", "text": "Some comment on fn." }, "functionSelector": "a4a2c40b", - "id": 19, + "id": 22, "implemented": true, "kind": "function", "modifiers": [], @@ -194,29 +233,29 @@ "overrides": null, "parameters": { - "id": 16, + "id": 19, "nodeType": "ParameterList", "parameters": [], - "src": "145:2:3" + "src": "199:2:3" }, "returnParameters": { - "id": 17, + "id": 20, "nodeType": "ParameterList", "parameters": [], - "src": "155:0:3" + "src": "209:0:3" }, - "scope": 20, - "src": "134:23:3", + "scope": 23, + "src": "188:23:3", "stateMutability": "nonpayable", "virtual": false, "visibility": "public" } ], - "scope": 21, - "src": "0:159:3" + "scope": 24, + "src": "0:213:3" } ], - "src": "0:160:3" + "src": "0:214:3" } ] diff --git a/test/libsolidity/ASTJSON/documentation.sol b/test/libsolidity/ASTJSON/documentation.sol index 5c5fe7d2b..089f81e0b 100644 --- a/test/libsolidity/ASTJSON/documentation.sol +++ b/test/libsolidity/ASTJSON/documentation.sol @@ -11,6 +11,7 @@ contract C {} // ---- SOURCE: c contract C { + /** Some comment on state var.*/ uint public state; /** Some comment on Evt.*/ event Evt(); /** Some comment on mod.*/ modifier mod() { _; } /** Some comment on fn.*/ function fn() public {} diff --git a/test/libsolidity/ASTJSON/documentation_legacy.json b/test/libsolidity/ASTJSON/documentation_legacy.json index 70a4d8cf4..bf085443c 100644 --- a/test/libsolidity/ASTJSON/documentation_legacy.json +++ b/test/libsolidity/ASTJSON/documentation_legacy.json @@ -6,7 +6,7 @@ { "C": [ - 20 + 23 ] }, "license": null @@ -30,13 +30,54 @@ "fullyImplemented": true, "linearizedBaseContracts": [ - 20 + 23 ], "name": "C", - "scope": 21 + "scope": 24 }, "children": [ + { + "attributes": + { + "constant": false, + "functionSelector": "c19d93fb", + "mutability": "mutable", + "name": "state", + "overrides": null, + "scope": 23, + "stateVariable": true, + "storageLocation": "default", + "type": "uint256", + "value": null, + "visibility": "public" + }, + "children": + [ + { + "attributes": + { + "name": "uint", + "type": "uint256" + }, + "id": 8, + "name": "ElementaryTypeName", + "src": "48:4:3" + }, + { + "attributes": + { + "text": "Some comment on state var." + }, + "id": 7, + "name": "StructuredDocumentation", + "src": "15:32:3" + } + ], + "id": 9, + "name": "VariableDeclaration", + "src": "48:17:3" + }, { "attributes": { @@ -50,9 +91,9 @@ { "text": "Some comment on Evt." }, - "id": 7, + "id": 10, "name": "StructuredDocumentation", - "src": "15:26:3" + "src": "69:26:3" }, { "attributes": @@ -63,14 +104,14 @@ ] }, "children": [], - "id": 8, + "id": 11, "name": "ParameterList", - "src": "51:2:3" + "src": "105:2:3" } ], - "id": 9, + "id": 12, "name": "EventDefinition", - "src": "42:12:3" + "src": "96:12:3" }, { "attributes": @@ -87,9 +128,9 @@ { "text": "Some comment on mod." }, - "id": 10, + "id": 13, "name": "StructuredDocumentation", - "src": "57:26:3" + "src": "111:26:3" }, { "attributes": @@ -100,27 +141,27 @@ ] }, "children": [], - "id": 11, + "id": 14, "name": "ParameterList", - "src": "96:2:3" + "src": "150:2:3" }, { "children": [ { - "id": 12, + "id": 15, "name": "PlaceholderStatement", - "src": "101:1:3" + "src": "155:1:3" } ], - "id": 13, + "id": 16, "name": "Block", - "src": "99:6:3" + "src": "153:6:3" } ], - "id": 14, + "id": 17, "name": "ModifierDefinition", - "src": "84:21:3" + "src": "138:21:3" }, { "attributes": @@ -135,7 +176,7 @@ ], "name": "fn", "overrides": null, - "scope": 20, + "scope": 23, "stateMutability": "nonpayable", "virtual": false, "visibility": "public" @@ -147,9 +188,9 @@ { "text": "Some comment on fn." }, - "id": 15, + "id": 18, "name": "StructuredDocumentation", - "src": "108:25:3" + "src": "162:25:3" }, { "attributes": @@ -160,9 +201,9 @@ ] }, "children": [], - "id": 16, + "id": 19, "name": "ParameterList", - "src": "145:2:3" + "src": "199:2:3" }, { "attributes": @@ -173,9 +214,9 @@ ] }, "children": [], - "id": 17, + "id": 20, "name": "ParameterList", - "src": "155:0:3" + "src": "209:0:3" }, { "attributes": @@ -186,22 +227,22 @@ ] }, "children": [], - "id": 18, + "id": 21, "name": "Block", - "src": "155:2:3" + "src": "209:2:3" } ], - "id": 19, + "id": 22, "name": "FunctionDefinition", - "src": "134:23:3" + "src": "188:23:3" } ], - "id": 20, + "id": 23, "name": "ContractDefinition", - "src": "0:159:3" + "src": "0:213:3" } ], - "id": 21, + "id": 24, "name": "SourceUnit", - "src": "0:160:3" + "src": "0:214:3" } diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index c3f96d874..7ae794a83 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -204,43 +204,75 @@ BOOST_AUTO_TEST_CASE(dev_and_user_no_doc) checkNatspec(sourceCode, "test", userNatspec, true); } -BOOST_AUTO_TEST_CASE(dev_state_variable) +BOOST_AUTO_TEST_CASE(public_state_variable) { char const* sourceCode = R"( contract test { - /// @dev Public accessor for the internal state - /// @notice Keeps track of the internal state + /// @notice example of notice + /// @dev example of dev + /// @return returns state uint public state; } )"; - char const* natspec = "{" - "\"methods\":{" - " \"state()\":{ \n" - " \"details\": \"Public accessor for the internal state\",\n" - " }\n" - "}}"; + char const* devDoc = R"R( + { + "methods" : {}, + "stateVariables" : + { + "state" : + { + "details" : "example of dev", + "return" : "returns state" + } + } + } + )R"; + checkNatspec(sourceCode, "test", devDoc, false); - checkNatspec(sourceCode, "test", natspec, false); + char const* userDoc = R"R( + { + "methods" : + { + "state()" : + { + "notice": "example of notice" + } + } + } + )R"; + checkNatspec(sourceCode, "test", userDoc, true); } -BOOST_AUTO_TEST_CASE(user_state_variable) +BOOST_AUTO_TEST_CASE(private_state_variable) { char const* sourceCode = R"( contract test { - /// @notice Keeps track of the internal state - uint public state; + /// @dev example of dev + uint private state; } )"; - char const* natspec = "{" - "\"methods\":{" - " \"state()\":{ \n" - " \"notice\": \"Keeps track of the internal state\",n" - " }\n" - "}}"; + char const* devDoc = R"( + { + "methods" : {}, + "stateVariables" : + { + "state" : + { + "details" : "example of dev" + } + } + } + )"; + checkNatspec(sourceCode, "test", devDoc, false); - checkNatspec(sourceCode, "test", natspec, true); + char const* userDoc = R"( + { + "methods":{} + } + )"; + checkNatspec(sourceCode, "test", userDoc, true); } BOOST_AUTO_TEST_CASE(dev_desc_after_nl) diff --git a/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol new file mode 100644 index 000000000..78aaf920c --- /dev/null +++ b/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol @@ -0,0 +1,7 @@ +contract C { + /// @title title + /// @author author + uint private state; +} +// ---- +// Warning: (17-56): Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol b/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol new file mode 100644 index 000000000..2b079ee7f --- /dev/null +++ b/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol @@ -0,0 +1,6 @@ +contract test { + /// @return returns something + uint private state; +} +// ---- +// DocstringParsingError: (18-47): Documentation tag "@return" is only allowed on public state-variables. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol new file mode 100644 index 000000000..780501230 --- /dev/null +++ b/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol @@ -0,0 +1,7 @@ +contract C { + /// @notice example of notice + /// @dev example of dev + uint private state; +} +// ---- +// Warning: (17-74): Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_state_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_state_variable.sol index 1ee27b62b..fd63f73df 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_state_variable.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_state_variable.sol @@ -1,9 +1,5 @@ contract C { - /// @title example of title - /// @author example of author /// @notice example of notice /// @dev example of dev - /// @param example of param - /// @return example of return uint public state; -} \ No newline at end of file +} diff --git a/test/libsolidity/syntaxTests/natspec/docstring_state_variable_too_many_return_tags.sol b/test/libsolidity/syntaxTests/natspec/docstring_state_variable_too_many_return_tags.sol new file mode 100644 index 000000000..e57d6f7e4 --- /dev/null +++ b/test/libsolidity/syntaxTests/natspec/docstring_state_variable_too_many_return_tags.sol @@ -0,0 +1,9 @@ +contract test { + /// @notice example of notice + /// @dev example of dev + /// @return returns something + /// @return returns something + uint public state; +} +// ---- +// DocstringParsingError: (18-137): Documentation tag "@return" is only allowed once on state-variables. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_variable.sol index ccbd4c8a5..726eba852 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_variable.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_variable.sol @@ -11,3 +11,4 @@ contract C { } } // ---- +// Warning: (290-295): Only state variables can have a docstring. This will be disallowed in 0.7.0.