Allow natspec comments on state variables.

This commit is contained in:
Alexander Arlt 2020-03-24 17:44:39 -05:00
parent 7d37ed4531
commit af8bb5fb60
21 changed files with 320 additions and 161 deletions

View File

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

View File

@ -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),

View File

@ -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)``

View File

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

View File

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

View File

@ -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<string> const validPublicTags = set<string>{"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<string> const validTags = set<string>{"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<FunctionDefinition const*>(&_node))
if (auto* varDecl = dynamic_cast<VariableDeclaration const*>(&_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<FunctionDefinition const*>(&_node))
{
string content = docTag.second.content;
string firstWord = content.substr(0, content.find_first_of(" \t"));

View File

@ -882,7 +882,7 @@ public:
ASTPointer<ASTString> const& _name,
ASTPointer<Expression> _value,
Visibility _visibility,
ASTPointer<StructuredDocumentation> const& _documentation,
ASTPointer<StructuredDocumentation> 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),

View File

@ -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())

View File

@ -441,7 +441,7 @@ ASTPointer<VariableDeclaration> ASTJsonImporter::createVariableDeclaration(Json:
make_shared<ASTString>(member(_node, "name").asString()),
nullOrCast<Expression>(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,

View File

@ -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<VariableDeclaration const*>(&it.second->declaration()))
else if (auto var = dynamic_cast<VariableDeclaration const*>(&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<VariableDeclaration const*>(&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;
}

View File

@ -686,7 +686,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
ASTNodeFactory nodeFactory = _lookAheadArrayType ?
ASTNodeFactory(*this, _lookAheadArrayType) : ASTNodeFactory(*this);
ASTPointer<TypeName> type;
ASTPointer<StructuredDocumentation> documentation = parseStructuredDocumentation();
ASTPointer<StructuredDocumentation> const documentation = parseStructuredDocumentation();
if (_lookAheadArrayType)
type = _lookAheadArrayType;
else
@ -696,8 +696,8 @@ ASTPointer<VariableDeclaration> 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<FunctionTypeName*>(type.get()) && _options.isStateVariable && m_scanner->currentToken() == Token::LBrace)
fatalParserError(
@ -813,7 +813,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
identifier,
value,
visibility,
documentation,
documentation,
_options.isStateVariable,
isIndexed,
mutability,
@ -1579,8 +1579,7 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme
ASTPointer<TypeName>(),
name,
ASTPointer<Expression>(),
Visibility::Default,
nullptr
Visibility::Default
);
}
variables.push_back(var);

View File

@ -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"
}
]

View File

@ -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 {}

View File

@ -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"
}

View File

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

View File

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

View File

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

View File

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

View File

@ -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;
}
}

View File

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

View File

@ -11,3 +11,4 @@ contract C {
}
}
// ----
// Warning: (290-295): Only state variables can have a docstring. This will be disallowed in 0.7.0.