Merge pull request #8532 from aarlt/structured-docs-variables-aarlt

Allow NatSpec comments for state variables

libsolidity/analysis/DocStringAnalyser.cpp

@@ -57,6 +57,31 @@ bool DocStringAnalyser::visit(FunctionDefinition const& _function)
 	return true;
 }
 
+bool DocStringAnalyser::visit(VariableDeclaration const& _variable)
+{
+	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)
 {
 	handleCallable(_modifier, _modifier, _modifier.annotation());
@@ -144,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"));

libsolidity/analysis/DocStringAnalyser.h

@@ -46,6 +46,7 @@ public:
 private:
 	bool visit(ContractDefinition const& _contract) override;
 	bool visit(FunctionDefinition const& _function) override;
+	bool visit(VariableDeclaration const& _variable) override;
 	bool visit(ModifierDefinition const& _modifier) override;
 	bool visit(EventDefinition const& _event) override;
 
@@ -67,6 +68,12 @@ private:
 		StructurallyDocumentedAnnotation& _annotation
 	);
 
+	void handleDeclaration(
+		Declaration const& _declaration,
+		StructurallyDocumented const& _node,
+		StructurallyDocumentedAnnotation& _annotation
+	);
+
 	void parseDocStrings(
 		StructurallyDocumented const& _node,
 		StructurallyDocumentedAnnotation& _annotation,

libsolidity/ast/AST.h

@@ -859,7 +859,7 @@ private:
  * Declaration of a variable. This can be used in various places, e.g. in function parameter
  * lists, struct definitions and even function bodies.
  */
-class VariableDeclaration: public Declaration
+class VariableDeclaration: public Declaration, public StructurallyDocumented
 {
 public:
 	enum Location { Unspecified, Storage, Memory, CallData };
@@ -882,6 +882,7 @@ public:
 		ASTPointer<ASTString> const& _name,
 		ASTPointer<Expression> _value,
 		Visibility _visibility,
+		ASTPointer<StructuredDocumentation> const _documentation = nullptr,
 		bool _isStateVar = false,
 		bool _isIndexed = false,
 		Mutability _mutability = Mutability::Mutable,
@@ -889,6 +890,7 @@ public:
 		Location _referenceLocation = Location::Unspecified
 	):
 		Declaration(_id, _location, _name, _visibility),
+		StructurallyDocumented(std::move(_documentation)),
 		m_typeName(std::move(_type)),
 		m_value(std::move(_value)),
 		m_isStateVariable(_isStateVar),

libsolidity/ast/ASTAnnotations.h

@@ -171,7 +171,7 @@ struct ModifierDefinitionAnnotation: CallableDeclarationAnnotation, Structurally
 {
 };
 
-struct VariableDeclarationAnnotation: DeclarationAnnotation
+struct VariableDeclarationAnnotation: DeclarationAnnotation, StructurallyDocumentedAnnotation
 {
 	/// Type of variable (type of identifier referencing this variable).
 	TypePointer type = nullptr;

libsolidity/ast/ASTJsonConverter.cpp

@@ -391,6 +391,8 @@ bool ASTJsonConverter::visit(VariableDeclaration const& _node)
 	};
 	if (_node.isStateVariable() && _node.isPublic())
 		attributes.emplace_back("functionSelector", _node.externalIdentifierHex());
+	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())

libsolidity/ast/ASTJsonImporter.cpp

@@ -441,6 +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")),
 		memberAsBool(_node, "stateVariable"),
 		_node.isMember("indexed") ? memberAsBool(_node, "indexed") : false,
 		mutability,

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);
@@ -52,6 +52,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 
 	for (auto const& it: _contractDef.interfaceFunctions())
 		if (it.second->hasDeclaration())
+		{
 			if (auto const* f = dynamic_cast<FunctionDefinition const*>(&it.second->declaration()))
 			{
 				string value = extractDoc(f->annotation().docTags, "notice");
@@ -63,6 +64,19 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 					methods[it.second->externalSignature()] = user;
 				}
 			}
+			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())
+				{
+					Json::Value user;
+					// since @notice is the only user tag if missing function should not appear
+					user["notice"] = Json::Value(value);
+					methods[it.second->externalSignature()] = user;
+				}
+			}
+		}
 	doc["methods"] = methods;
 
 	return doc;
@@ -110,7 +124,20 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 		}
 	}
 
+	Json::Value stateVariables(Json::objectValue);
+	for (VariableDeclaration const* varDecl: _contractDef.stateVariables())
+	{
+		if (auto devDoc = devDocumentation(varDecl->annotation().docTags); !devDoc.empty())
+			stateVariables[varDecl->name()] = devDoc;
+
+		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;
 }

libsolidity/parsing/Parser.cpp

@@ -686,6 +686,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 	ASTNodeFactory nodeFactory = _lookAheadArrayType ?
 		ASTNodeFactory(*this, _lookAheadArrayType) : ASTNodeFactory(*this);
 	ASTPointer<TypeName> type;
+	ASTPointer<StructuredDocumentation> const documentation = parseStructuredDocumentation();
 	if (_lookAheadArrayType)
 		type = _lookAheadArrayType;
 	else
@@ -695,6 +696,9 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 			nodeFactory.setEndPositionFromNode(type);
 	}
 
+	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(
 			2915_error,
@@ -809,6 +813,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 		identifier,
 		value,
 		visibility,
+		documentation,
 		_options.isStateVariable,
 		isIndexed,
 		mutability,