Merge pull request #545 from LefterisJP/sol_com_in_ast

Solidity function AST nodes get documentation attribute

libsolidity/AST.h

@@ -173,14 +173,21 @@ private:
 class FunctionDefinition: public Declaration
 {
 public:
-	FunctionDefinition(Location const& _location, ASTPointer<ASTString> const& _name, bool _isPublic,
-					   ASTPointer<ParameterList> const& _parameters,
-					   bool _isDeclaredConst,
-					   ASTPointer<ParameterList> const& _returnParameters,
-					   ASTPointer<Block> const& _body):
-		Declaration(_location, _name), m_isPublic(_isPublic), m_parameters(_parameters),
-		m_isDeclaredConst(_isDeclaredConst), m_returnParameters(_returnParameters),
-		m_body(_body) {}
+	FunctionDefinition(Location const& _location, ASTPointer<ASTString> const& _name,
+					bool _isPublic,
+					ASTPointer<ASTString> const& _documentation,
+					ASTPointer<ParameterList> const& _parameters,
+					bool _isDeclaredConst,
+					ASTPointer<ParameterList> const& _returnParameters,
+					ASTPointer<Block> const& _body):
+	Declaration(_location, _name), m_isPublic(_isPublic),
+	m_parameters(_parameters),
+	m_isDeclaredConst(_isDeclaredConst),
+	m_returnParameters(_returnParameters),
+	m_body(_body),
+	m_documentation(_documentation)
+	{}
+
 	virtual void accept(ASTVisitor& _visitor) override;
 
 	bool isPublic() const { return m_isPublic; }
@@ -190,6 +197,9 @@ public:
 	std::vector<ASTPointer<VariableDeclaration>> const& getReturnParameters() const { return m_returnParameters->getParameters(); }
 	ASTPointer<ParameterList> const& getReturnParameterList() const { return m_returnParameters; }
 	Block& getBody() { return *m_body; }
+	/// @return A shared pointer of an ASTString.
+	/// Can contain a nullptr in which case indicates absence of documentation
+	ASTPointer<ASTString> const& getDocumentation() { return m_documentation; }
 
 	void addLocalVariable(VariableDeclaration const& _localVariable) { m_localVariables.push_back(&_localVariable); }
 	std::vector<VariableDeclaration const*> const& getLocalVariables() const { return m_localVariables; }
@@ -203,6 +213,7 @@ private:
 	bool m_isDeclaredConst;
 	ASTPointer<ParameterList> m_returnParameters;
 	ASTPointer<Block> m_body;
+	ASTPointer<ASTString> m_documentation;
 
 	std::vector<VariableDeclaration const*> m_localVariables;
 };

libsolidity/Parser.cpp

@@ -117,6 +117,10 @@ ASTPointer<ContractDefinition> Parser::parseContractDefinition()
 ASTPointer<FunctionDefinition> Parser::parseFunctionDefinition(bool _isPublic)
 {
 	ASTNodeFactory nodeFactory(*this);
+	ASTPointer<ASTString> docstring;
+	if (m_scanner->getCurrentCommentLiteral() != "")
+		docstring = std::make_shared<ASTString>(m_scanner->getCurrentCommentLiteral());
+
 	expectToken(Token::FUNCTION);
 	ASTPointer<ASTString> name(expectIdentifierToken());
 	ASTPointer<ParameterList> parameters(parseParameterList());
@@ -142,8 +146,9 @@ ASTPointer<FunctionDefinition> Parser::parseFunctionDefinition(bool _isPublic)
 	}
 	ASTPointer<Block> block = parseBlock();
 	nodeFactory.setEndPositionFromNode(block);
-	return nodeFactory.createNode<FunctionDefinition>(name, _isPublic, parameters,
-			isDeclaredConst, returnParameters, block);
+	return nodeFactory.createNode<FunctionDefinition>(name, _isPublic, docstring,
+													  parameters,
+													  isDeclaredConst, returnParameters, block);
 }
 
 ASTPointer<StructDefinition> Parser::parseStructDefinition()

libsolidity/Scanner.cpp

@@ -102,18 +102,55 @@ int hexValue(char c)
 }
 } // end anonymous namespace
 
+
+
+/// Scoped helper for literal recording. Automatically drops the literal
+/// if aborting the scanning before it's complete.
+enum LiteralType {
+	LITERAL_TYPE_STRING,
+	LITERAL_TYPE_NUMBER, // not really different from string type in behaviour
+	LITERAL_TYPE_COMMENT
+};
+
+class LiteralScope
+{
+public:
+	explicit LiteralScope(Scanner* _self, enum LiteralType _type): m_type(_type)
+	, m_scanner(_self)
+	, m_complete(false)
+	{
+		if (_type == LITERAL_TYPE_COMMENT)
+			m_scanner->m_nextSkippedComment.literal.clear();
+		else
+			m_scanner->m_nextToken.literal.clear();
+	}
+	~LiteralScope()
+	{
+		if (!m_complete)
+		{
+			if (m_type == LITERAL_TYPE_COMMENT)
+				m_scanner->m_nextSkippedComment.literal.clear();
+			else
+				m_scanner->m_nextToken.literal.clear();
+		}
+	}
+	void complete() { m_complete = true; }
+
+private:
+	enum LiteralType m_type;
+	Scanner* m_scanner;
+	bool m_complete;
+}; // end of LiteralScope class
+
+
 void Scanner::reset(CharStream const& _source)
 {
-	bool foundDocComment;
 	m_source = _source;
 	m_char = m_source.get();
 	skipWhitespace();
-	foundDocComment = scanToken();
+	scanToken();
 
-	// special version of Scanner:next() taking the previous scanToken() result into account
-	m_currentToken = m_nextToken;
-	if (scanToken() || foundDocComment)
-		m_skippedComment = m_nextSkippedComment;
+	next();
 }
 
 
@@ -142,8 +179,9 @@ BOOST_STATIC_ASSERT(Token::NUM_TOKENS <= 0x100);
 Token::Value Scanner::next()
 {
 	m_currentToken = m_nextToken;
-	if (scanToken())
-		m_skippedComment = m_nextSkippedComment;
+	m_skippedComment = m_nextSkippedComment;
+	scanToken();
+
 	return m_currentToken.token;
 }
 
@@ -180,10 +218,26 @@ Token::Value Scanner::skipSingleLineComment()
 /// For the moment this function simply consumes a single line triple slash doc comment
 Token::Value Scanner::scanDocumentationComment()
 {
-	LiteralScope literal(this);
+	LiteralScope literal(this, LITERAL_TYPE_COMMENT);
 	advance(); //consume the last '/'
-	while (!isSourcePastEndOfInput() && !isLineTerminator(m_char))
+	while (!isSourcePastEndOfInput())
 	{
+		if (isLineTerminator(m_char))
+		{
+			// check if next line is also a documentation comment
+			skipWhitespace();
+			if (!m_source.isPastEndOfInput(3) &&
+				m_source.get(0) == '/' &&
+				m_source.get(1) == '/' &&
+				m_source.get(2) == '/')
+			{
+				addCommentLiteralChar('\n');
+				m_char = m_source.advanceAndGet(3);
+			}
+			else
+				break; // next line is not a documentation comment, we are done
+
+		}
 		addCommentLiteralChar(m_char);
 		advance();
 	}
@@ -214,10 +268,10 @@ Token::Value Scanner::skipMultiLineComment()
 	return Token::ILLEGAL;
 }
 
-bool Scanner::scanToken()
+void Scanner::scanToken()
 {
-	bool foundDocComment = false;
 	m_nextToken.literal.clear();
+	m_nextSkippedComment.literal.clear();
 	Token::Value token;
 	do
 	{
@@ -329,7 +383,6 @@ bool Scanner::scanToken()
 					m_nextSkippedComment.location.end = getSourcePos();
 					m_nextSkippedComment.token = comment;
 					token = Token::WHITESPACE;
-					foundDocComment = true;
 				}
 				else
 					token = skipSingleLineComment();
@@ -425,8 +478,6 @@ bool Scanner::scanToken()
 	while (token == Token::WHITESPACE);
 	m_nextToken.location.end = getSourcePos();
 	m_nextToken.token = token;
-
-	return foundDocComment;
 }
 
 bool Scanner::scanEscape()
@@ -474,7 +525,7 @@ Token::Value Scanner::scanString()
 {
 	char const quote = m_char;
 	advance();  // consume quote
-	LiteralScope literal(this);
+	LiteralScope literal(this, LITERAL_TYPE_STRING);
 	while (m_char != quote && !isSourcePastEndOfInput() && !isLineTerminator(m_char))
 	{
 		char c = m_char;
@@ -505,7 +556,7 @@ void Scanner::scanDecimalDigits()
 Token::Value Scanner::scanNumber(char _charSeen)
 {
 	enum { DECIMAL, HEX, BINARY } kind = DECIMAL;
-	LiteralScope literal(this);
+	LiteralScope literal(this, LITERAL_TYPE_NUMBER);
 	if (_charSeen == '.')
 	{
 		// we have already seen a decimal point of the float
@@ -758,7 +809,7 @@ Token::Value Scanner::scanIdentifierOrKeyword()
 {
 	if (asserts(isIdentifierStart(m_char)))
 		BOOST_THROW_EXCEPTION(InternalCompilerError());
-	LiteralScope literal(this);
+	LiteralScope literal(this, LITERAL_TYPE_STRING);
 	addLiteralCharAndAdvance();
 	// Scan the rest of the identifier characters.
 	while (isIdentifierPart(m_char))
@@ -767,14 +818,14 @@ Token::Value Scanner::scanIdentifierOrKeyword()
 	return KeywordOrIdentifierToken(m_nextToken.literal);
 }
 
-char CharStream::advanceAndGet()
+char CharStream::advanceAndGet(size_t _chars)
 {
 	if (isPastEndOfInput())
 		return 0;
-	++m_pos;
+	m_pos += _chars;
 	if (isPastEndOfInput())
 		return 0;
-	return get();
+	return m_source[m_pos];
 }
 
 char CharStream::rollback(size_t _amount)

libsolidity/Scanner.h

@@ -74,9 +74,9 @@ public:
 	CharStream(): m_pos(0) {}
 	explicit CharStream(std::string const& _source): m_source(_source), m_pos(0) {}
 	int getPos() const { return m_pos; }
-	bool isPastEndOfInput() const { return m_pos >= m_source.size(); }
-	char get() const { return m_source[m_pos]; }
-	char advanceAndGet();
+	bool isPastEndOfInput(size_t _charsForward = 0) const { return (m_pos + _charsForward) >= m_source.size(); }
+	char get(size_t _charsForward = 0) const { return m_source[m_pos + _charsForward]; }
+	char advanceAndGet(size_t _chars=1);
 	char rollback(size_t _amount);
 
 	///@{
@@ -93,22 +93,11 @@ private:
 };
 
 
+
 class Scanner
 {
+	friend class LiteralScope;
 public:
-	/// Scoped helper for literal recording. Automatically drops the literal
-	/// if aborting the scanning before it's complete.
-	class LiteralScope
-	{
-	public:
-		explicit LiteralScope(Scanner* self): m_scanner(self), m_complete(false) { m_scanner->startNewLiteral(); }
-		~LiteralScope() { if (!m_complete) m_scanner->dropLiteral(); }
-		void complete() { m_complete = true; }
-
-	private:
-		Scanner* m_scanner;
-		bool m_complete;
-	};
 
 	Scanner() { reset(CharStream()); }
 	explicit Scanner(CharStream const& _source) { reset(_source); }
@@ -133,8 +122,12 @@ public:
 
 	///@{
 	///@name Information about the current comment token
+
 	Location getCurrentCommentLocation() const { return m_skippedComment.location; }
 	std::string const& getCurrentCommentLiteral() const { return m_skippedComment.literal; }
+	/// Called by the parser during FunctionDefinition parsing to clear the current comment
+	void clearCurrentCommentLiteral() { m_skippedComment.literal.clear(); }
+
 	///@}
 
 	///@{
@@ -165,10 +158,8 @@ private:
 
 	///@{
 	///@name Literal buffer support
-	inline void startNewLiteral() { m_nextToken.literal.clear(); }
 	inline void addLiteralChar(char c) { m_nextToken.literal.push_back(c); }
 	inline void addCommentLiteralChar(char c) { m_nextSkippedComment.literal.push_back(c); }
-	inline void dropLiteral() { m_nextToken.literal.clear(); }
 	inline void addLiteralCharAndAdvance() { addLiteralChar(m_char); advance(); }
 	///@}
 
@@ -181,9 +172,8 @@ private:
 
 	bool scanHexByte(char& o_scannedByte);
 
-	/// Scans a single Solidity token. Returns true if the scanned token was
-	/// a skipped documentation comment. False in all other cases.
-	bool scanToken();
+	/// Scans a single Solidity token.
+	void scanToken();
 
 	/// Skips all whitespace and @returns true if something was skipped.
 	bool skipWhitespace();

test/solidityParser.cpp

@@ -37,13 +37,14 @@ namespace test
 
 namespace
 {
-ASTPointer<ASTNode> parseText(std::string const& _source)
+ASTPointer<ContractDefinition> parseText(std::string const& _source)
 {
 	Parser parser;
 	return parser.parse(std::make_shared<Scanner>(CharStream(_source)));
 }
 }
 
+
 BOOST_AUTO_TEST_SUITE(SolidityParser)
 
 BOOST_AUTO_TEST_CASE(smoke_test)
@@ -91,6 +92,164 @@ BOOST_AUTO_TEST_CASE(single_function_param)
 	BOOST_CHECK_NO_THROW(parseText(text));
 }
 
+BOOST_AUTO_TEST_CASE(function_natspec_documentation)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  /// This is a test function\n"
+					   "  function functionName(hash hashin) returns (hash hashout) {}\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(), " This is a test function");
+}
+
+BOOST_AUTO_TEST_CASE(function_normal_comments)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  // We won't see this comment\n"
+					   "  function functionName(hash hashin) returns (hash hashout) {}\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_MESSAGE(function->getDocumentation() == nullptr,
+						"Should not have gotten a Natspect comment for this function");
+}
+
+BOOST_AUTO_TEST_CASE(multiple_functions_natspec_documentation)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  /// This is test function 1\n"
+					   "  function functionName1(hash hashin) returns (hash hashout) {}\n"
+					   "  /// This is test function 2\n"
+					   "  function functionName2(hash hashin) returns (hash hashout) {}\n"
+					   "  // nothing to see here\n"
+					   "  function functionName3(hash hashin) returns (hash hashout) {}\n"
+					   "  /// This is test function 4\n"
+					   "  function functionName4(hash hashin) returns (hash hashout) {}\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(), " This is test function 1");
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(1));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(), " This is test function 2");
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(2));
+	BOOST_CHECK_MESSAGE(function->getDocumentation() == nullptr,
+						"Should not have gotten natspec comment for functionName3()");
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(3));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(), " This is test function 4");
+}
+
+BOOST_AUTO_TEST_CASE(multiline_function_documentation)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  /// This is a test function\n"
+					   "  /// and it has 2 lines\n"
+					   "  function functionName1(hash hashin) returns (hash hashout) {}\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(),
+					  " This is a test function\n"
+					  " and it has 2 lines");
+}
+
+BOOST_AUTO_TEST_CASE(natspec_comment_in_function_body)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  /// fun1 description\n"
+					   "  function fun1(uint256 a) {\n"
+					   "    var b;\n"
+					   "    /// I should not interfere with actual natspec comments\n"
+					   "    uint256 c;\n"
+					   "    mapping(address=>hash) d;\n"
+					   "    string name = \"Solidity\";"
+					   "  }\n"
+					   "  uint256 stateVar;\n"
+					   "  /// This is a test function\n"
+					   "  /// and it has 2 lines\n"
+					   "  function fun(hash hashin) returns (hash hashout) {}\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(), " fun1 description");
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(1));
+	BOOST_CHECK_EQUAL(*function->getDocumentation(),
+					  " This is a test function\n"
+					  " and it has 2 lines");
+}
+
+BOOST_AUTO_TEST_CASE(natspec_docstring_between_keyword_and_signature)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  function ///I am in the wrong place \n"
+					   "  fun1(uint256 a) {\n"
+					   "    var b;\n"
+					   "    /// I should not interfere with actual natspec comments\n"
+					   "    uint256 c;\n"
+					   "    mapping(address=>hash) d;\n"
+					   "    string name = \"Solidity\";"
+					   "  }\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_MESSAGE(!function->getDocumentation(),
+						"Shouldn't get natspec docstring for this function");
+}
+
+BOOST_AUTO_TEST_CASE(natspec_docstring_after_signature)
+{
+	ASTPointer<ContractDefinition> contract;
+	ASTPointer<FunctionDefinition> function;
+	char const* text = "contract test {\n"
+					   "  uint256 stateVar;\n"
+					   "  function fun1(uint256 a) {\n"
+					   "  /// I should have been above the function signature\n"
+					   "    var b;\n"
+					   "    /// I should not interfere with actual natspec comments\n"
+					   "    uint256 c;\n"
+					   "    mapping(address=>hash) d;\n"
+					   "    string name = \"Solidity\";"
+					   "  }\n"
+					   "}\n";
+	BOOST_REQUIRE_NO_THROW(contract = parseText(text));
+	auto functions = contract->getDefinedFunctions();
+
+	BOOST_REQUIRE_NO_THROW(function = functions.at(0));
+	BOOST_CHECK_MESSAGE(!function->getDocumentation(),
+						"Shouldn't get natspec docstring for this function");
+}
+
 BOOST_AUTO_TEST_CASE(struct_definition)
 {
 	char const* text = "contract test {\n"