Merge pull request #4542 from aarlt/constructor_natspec

Fix: natspec annotations on constructors

5 files changed, 216 insertions, 27 deletions

diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp
index b3fb5258..c1b97def 100644
--- a/libsolidity/analysis/DocStringAnalyser.cpp
+++ b/libsolidity/analysis/DocStringAnalyser.cpp
@@ -48,7 +48,10 @@ bool DocStringAnalyser::visit(ContractDefinition const& _contract)
 
 bool DocStringAnalyser::visit(FunctionDefinition const& _function)
 {
-	handleCallable(_function, _function, _function.annotation());
+	if (_function.isConstructor())
+		handleConstructor(_function, _function, _function.annotation());
+	else
+		handleCallable(_function, _function, _function.annotation());
 	return true;
 }
 
@@ -66,15 +69,11 @@ bool DocStringAnalyser::visit(EventDefinition const& _event)
 	return true;
 }
 
-void DocStringAnalyser::handleCallable(
+void DocStringAnalyser::checkParameters(
 	CallableDeclaration const& _callable,
-	Documented const& _node,
 	DocumentedAnnotation& _annotation
 )
 {
-	static const set<string> validTags = set<string>{"author", "dev", "notice", "return", "param"};
-	parseDocStrings(_node, _annotation, validTags, "functions");
-
 	set<string> validParams;
 	for (auto const& p: _callable.parameters())
 		validParams.insert(p->name());
@@ -89,6 +88,29 @@ void DocStringAnalyser::handleCallable(
 				i->second.paramName +
 				"\" not found in the parameter list of the function."
 			);
+
+}
+
+void DocStringAnalyser::handleConstructor(
+	CallableDeclaration const& _callable,
+	Documented const& _node,
+	DocumentedAnnotation& _annotation
+)
+{
+	static const set<string> validTags = set<string>{"author", "dev", "notice", "param"};
+	parseDocStrings(_node, _annotation, validTags, "constructor");
+	checkParameters(_callable, _annotation);
+}
+
+void DocStringAnalyser::handleCallable(
+	CallableDeclaration const& _callable,
+	Documented const& _node,
+	DocumentedAnnotation& _annotation
+)
+{
+	static const set<string> validTags = set<string>{"author", "dev", "notice", "return", "param"};
+	parseDocStrings(_node, _annotation, validTags, "functions");
+	checkParameters(_callable, _annotation);
 }
 
 void DocStringAnalyser::parseDocStrings(
diff --git a/libsolidity/analysis/DocStringAnalyser.h b/libsolidity/analysis/DocStringAnalyser.h
index 158b4060..5d339428 100644
--- a/libsolidity/analysis/DocStringAnalyser.h
+++ b/libsolidity/analysis/DocStringAnalyser.h
@@ -48,6 +48,17 @@ private:
 	virtual bool visit(ModifierDefinition const& _modifier) override;
 	virtual bool visit(EventDefinition const& _event) override;
 
+	void checkParameters(
+		CallableDeclaration const& _callable,
+		DocumentedAnnotation& _annotation
+	);
+
+	void handleConstructor(
+		CallableDeclaration const& _callable,
+		Documented const& _node,
+		DocumentedAnnotation& _annotation
+	);
+
 	void handleCallable(
 		CallableDeclaration const& _callable,
 		Documented const& _node,
diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp
index 29a5b798..a8716862 100644
--- a/libsolidity/interface/Natspec.cpp
+++ b/libsolidity/interface/Natspec.cpp
@@ -36,6 +36,15 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 	Json::Value doc;
 	Json::Value methods(Json::objectValue);
 
+	auto constructorDefinition(_contractDef.constructor());
+	if (constructorDefinition)
+	{
+		string 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);
+	}
+
 	string notice = extractDoc(_contractDef.annotation().docTags, "notice");
 	if (!notice.empty())
 		doc["notice"] = Json::Value(notice);
@@ -73,33 +82,21 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 	if (!dev.empty())
 		doc["details"] = Json::Value(dev);
 
+	auto constructorDefinition(_contractDef.constructor());
+	if (constructorDefinition) {
+		Json::Value constructor(devDocumentation(constructorDefinition->annotation().docTags));
+		if (!constructor.empty())
+			// add the constructor, only if we have any documentation to add
+			methods["constructor"] = constructor;
+	}
+
 	for (auto const& it: _contractDef.interfaceFunctions())
 	{
 		if (!it.second->hasDeclaration())
 			continue;
-		Json::Value method;
 		if (auto fun = dynamic_cast<FunctionDefinition const*>(&it.second->declaration()))
 		{
-			auto dev = extractDoc(fun->annotation().docTags, "dev");
-			if (!dev.empty())
-				method["details"] = Json::Value(dev);
-
-			auto author = extractDoc(fun->annotation().docTags, "author");
-			if (!author.empty())
-				method["author"] = author;
-
-			auto ret = extractDoc(fun->annotation().docTags, "return");
-			if (!ret.empty())
-				method["return"] = ret;
-
-			Json::Value params(Json::objectValue);
-			auto paramRange = fun->annotation().docTags.equal_range("param");
-			for (auto i = paramRange.first; i != paramRange.second; ++i)
-				params[i->second.paramName] = Json::Value(i->second.content);
-
-			if (!params.empty())
-				method["params"] = params;
-
+			Json::Value method(devDocumentation(fun->annotation().docTags));
 			if (!method.empty())
 				// add the function, only if we have any documentation to add
 				methods[it.second->externalSignature()] = method;
@@ -118,3 +115,31 @@ string Natspec::extractDoc(multimap<string, DocTag> const& _tags, string const&
 		value += i->second.content;
 	return value;
 }
+
+Json::Value Natspec::devDocumentation(std::multimap<std::string, DocTag> const &_tags)
+{
+	Json::Value json(Json::objectValue);
+	auto dev = extractDoc(_tags, "dev");
+	if (!dev.empty())
+		json["details"] = Json::Value(dev);
+
+	auto author = extractDoc(_tags, "author");
+	if (!author.empty())
+		json["author"] = author;
+
+	// for constructors, the "return" node will never exist. invalid tags
+	// will already generate an error within dev::solidity::DocStringAnalyzer.
+	auto ret = extractDoc(_tags, "return");
+	if (!ret.empty())
+		json["return"] = ret;
+
+	Json::Value params(Json::objectValue);
+	auto paramRange = _tags.equal_range("param");
+	for (auto i = paramRange.first; i != paramRange.second; ++i)
+		params[i->second.paramName] = Json::Value(i->second.content);
+
+	if (!params.empty())
+		json["params"] = params;
+
+	return json;
+}
diff --git a/libsolidity/interface/Natspec.h b/libsolidity/interface/Natspec.h
index 6a827d3b..0be4dda2 100644
--- a/libsolidity/interface/Natspec.h
+++ b/libsolidity/interface/Natspec.h
@@ -54,6 +54,12 @@ public:
 private:
 	/// @returns concatenation of all content under the given tag name.
 	static std::string extractDoc(std::multimap<std::string, DocTag> const& _tags, std::string const& _name);
+
+	/// Helper-function that will create a json object with dev specific annotations, if present.
+	/// @param _tags docTags that are used.
+	/// @return      A JSON representation
+	///              of the contract's developer documentation
+	static Json::Value devDocumentation(std::multimap<std::string, DocTag> const &_tags);
 };
 
 } //solidity NS
diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp
index cc44b578..b97df972 100644
--- a/test/libsolidity/SolidityNatspecJSON.cpp
+++ b/test/libsolidity/SolidityNatspecJSON.cpp
@@ -683,6 +683,131 @@ BOOST_AUTO_TEST_CASE(dev_documenting_no_param_description)
 	expectNatspecError(sourceCode);
 }
 
+BOOST_AUTO_TEST_CASE(user_constructor)
+{
+	char const *sourceCode = R"(
+		contract test {
+			/// @notice this is a really nice constructor
+			constructor(uint a, uint second) public { }
+		}
+	)";
+
+	char const *natspec = R"ABCDEF({
+	"methods" : {
+		"constructor" : "this is a really nice constructor"
+	}
+	})ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, true);
+}
+
+BOOST_AUTO_TEST_CASE(user_constructor_and_function)
+{
+	char const *sourceCode = R"(
+		contract test {
+			/// @notice this is a really nice constructor
+			constructor(uint a, uint second) public { }
+			/// another multiplier
+			function mul(uint a, uint second) public returns(uint d) { return a * 7 + second; }
+		}
+	)";
+
+	char const *natspec = R"ABCDEF({
+	"methods" : {
+		"mul(uint256,uint256)" : {
+			"notice" : "another multiplier"
+		},
+		"constructor" : "this is a really nice constructor"
+	}
+	})ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, true);
+}
+
+BOOST_AUTO_TEST_CASE(dev_constructor)
+{
+	char const *sourceCode = R"(
+		contract test {
+			/// @author Alex
+			/// @param a the parameter a is really nice and very useful
+			/// @param second the second parameter is not very useful, it just provides additional confusion
+			constructor(uint a, uint second) public { }
+		}
+	)";
+
+	char const *natspec = R"ABCDEF({
+	"methods" : {
+		"constructor" : {
+			"author" : "Alex",
+			"params" : {
+				"a" : "the parameter a is really nice and very useful",
+				"second" : "the second parameter is not very useful, it just provides additional confusion"
+			}
+		}
+	}
+	})ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, false);
+}
+
+BOOST_AUTO_TEST_CASE(dev_constructor_return)
+{
+	char const* sourceCode = R"(
+		contract test {
+			/// @author Alex
+			/// @param a the parameter a is really nice and very useful
+			/// @param second the second parameter is not very useful, it just provides additional confusion
+			/// @return return should not work within constructors
+			constructor(uint a, uint second) public { }
+		}
+	)";
+
+	expectNatspecError(sourceCode);
+}
+
+BOOST_AUTO_TEST_CASE(dev_constructor_and_function)
+{
+	char const *sourceCode = R"(
+		contract test {
+			/// @author Alex
+			/// @param a the parameter a is really nice and very useful
+			/// @param second the second parameter is not very useful, it just provides additional confusion
+			constructor(uint a, uint second) public { }
+			/// @dev Multiplies a number by 7 and adds second parameter
+			/// @param a Documentation for the first parameter starts here.
+			/// Since it's a really complicated parameter we need 2 lines
+			/// @param second Documentation for the second parameter
+			/// @return The result of the multiplication
+			/// and cookies with nutella
+			function mul(uint a, uint second) public returns(uint d) {
+				return a * 7 + second;
+			}
+		}
+	)";
+
+	char const *natspec = R"ABCDEF({
+	"methods" : {
+		"mul(uint256,uint256)" : {
+			"details" : "Multiplies a number by 7 and adds second parameter",
+			"params" : {
+				"a" : "Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines",
+				"second" : "Documentation for the second parameter"
+			},
+			"return" : "The result of the multiplication and cookies with nutella"
+		},
+		"constructor" : {
+			"author" : "Alex",
+			"params" : {
+				"a" : "the parameter a is really nice and very useful",
+				"second" : "the second parameter is not very useful, it just provides additional confusion"
+			}
+		}
+	}
+	})ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, false);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }