iii

ethereum · Mar 25, 2020 · 0302dd8 · 0302dd8
1 parent 84f27c4
commit 0302dd8
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 55 deletions.
diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp
@@ -60,7 +60,14 @@ bool DocStringAnalyser::visit(FunctionDefinition const& _function)
 bool DocStringAnalyser::visit(VariableDeclaration const& _variable)
 {
 	if (_variable.isStateVariable())
-		handleDeclaration(_variable, _variable, _variable.annotation());
+	{
+		static set<string> const validPublicTags = set<string>{"dev", "notice"};
+		static set<string> const validNonPublicTags= set<string>{"dev"};
+		if (_variable.isPublic())
+			parseDocStrings(_variable, _variable.annotation(), validPublicTags, "public state variables");
+		else
+			parseDocStrings(_variable, _variable.annotation(), validNonPublicTags, "non-public state variables");
+	}
 	return true;
 }
 
@@ -124,16 +131,6 @@ void DocStringAnalyser::handleCallable(
 	checkParameters(_callable, _node, _annotation);
 }
 
-void DocStringAnalyser::handleDeclaration(
-	Declaration const&,
-	StructurallyDocumented const& _node,
-	StructurallyDocumentedAnnotation& _annotation
-)
-{
-	static set<string> const validPublicTags = set<string>{"dev", "notice"};
-	parseDocStrings(_node, _annotation, validPublicTags, "state variables");
-}
-
 void DocStringAnalyser::parseDocStrings(
 	StructurallyDocumented const& _node,
 	StructurallyDocumentedAnnotation& _annotation,

diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp
@@ -64,18 +64,16 @@ 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()))
 			{
-				if (var->isStateVariable() && var->isPublic())
+				solAssert(var->isStateVariable() && var->isPublic(), "");
+				string value = extractDoc(var->annotation().docTags, "notice");
+				if (!value.empty())
 				{
-					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;
-					}
+					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;
 				}
 			}
 		}
@@ -88,7 +86,6 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 {
 	Json::Value doc;
 	Json::Value methods(Json::objectValue);
-	Json::Value stateVariables(Json::objectValue);
 
 	auto author = extractDoc(_contractDef.annotation().docTags, "author");
 	if (!author.empty())
@@ -129,16 +126,17 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 		}
 		if (auto var = dynamic_cast<VariableDeclaration const*>(&it.second->declaration()))
 		{
-			Json::Value method(devDocumentation(var->annotation().docTags));
-			if (!method.empty() && var->isStateVariable() && var->isPublic())
-				methods[it.second->externalSignature()] = method;
+			Json::Value declarationDoc(devDocumentation(var->annotation().docTags));
+			if (!declarationDoc.empty() && var->isStateVariable() && var->isPublic())
+				methods[it.second->externalSignature()] = declarationDoc;
 		}
 	}
 
+	Json::Value stateVariables(Json::objectValue);
 	for (auto const& it: _contractDef.stateVariables())
 	{
 		Json::Value documentation(devDocumentation(it->annotation().docTags));
-		if (!documentation.empty() && it->visibility() != Visibility::Public)
+		if (!documentation.empty())
 			stateVariables[it->name()] = documentation;
 	}
 

diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
@@ -688,8 +688,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)
+		fatalParserError("Only state variables can retrieve a docstring.");
 
 	if (dynamic_cast<FunctionTypeName*>(type.get()) && _options.isStateVariable && m_scanner->currentToken() == Token::LBrace)
 		fatalParserError(

diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp
@@ -214,34 +214,24 @@ BOOST_AUTO_TEST_CASE(dev_public_state_variable)
 		}
 	)";
 
-	char const* natspec = "{"
-						  "\"methods\":{"
-						  "    \"state()\":{ \n"
-						  "        \"details\": \"example of dev\"\n"
+	char const* natspec = "{\n"
+						  "    \"methods\" : \n"
+						  "    {\n"
+						  "        \"state()\" : \n"
+						  "        {\n"
+						  "            \"details\" : \"example of dev\"\n"
+						  "        }\n"
+						  "    },\n"
+						  "    \"stateVariables\" : \n"
+						  "    {\n"
+						  "        \"state\" : \n"
+						  "        {\n"
+						  "            \"details\" : \"example of dev\"\n"
+						  "        }\n"
 						  "    }\n"
-						  "}}";
-
-	checkNatspec(sourceCode, "test", natspec, false);
-}
-
-BOOST_AUTO_TEST_CASE(user_variable_declaration)
-{
-	char const* sourceCode = R"(
-		contract test {
-			function f() public pure returns (uint) {
-				/// @notice example of notice
-				/// @dev example of dev
-				uint state = 42;
-				return state;
-			}
-		}
-	)";
-
-	char const* natspec = "{"
-						  "\"methods\":{}"
 						  "}";
 
-	checkNatspec(sourceCode, "test", natspec, true);
+	checkNatspec(sourceCode, "test", natspec, false);
 }
 
 BOOST_AUTO_TEST_CASE(user_public_state_variable)
@@ -268,7 +258,6 @@ BOOST_AUTO_TEST_CASE(dev_private_state_variable)
 {
 	char const* sourceCode = R"(
 		contract test {
-			/// @notice example of notice
 			/// @dev example of dev
 			uint private state;
 		}
@@ -290,7 +279,6 @@ BOOST_AUTO_TEST_CASE(user_private_state_variable)
 {
 	char const* sourceCode = R"(
 		contract test {
-			/// @notice example of notice
 			/// @dev example of dev
 			uint private state;
 		}

diff --git a/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol 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;
+}
+// ----
+// DocstringParsingError: (17-74): Documentation tag @notice not valid for non-public state variables.
diff --git a/test/libsolidity/syntaxTests/natspec/docstring_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_variable.sol
@@ -11,3 +11,4 @@ contract C {
     }
 }
 // ----
+// ParserError: (290-295): Only state variables can retrieve a docstring.