Merge pull request #8221 from ethereum/structured-docs-error-locations

[parser] Source locations for structured documentation errors

Changelog.md

@@ -3,6 +3,8 @@
 Language Features:
  * Allow contract types and enums as keys for mappings.
  * Allow function selectors to be used as compile-time constants.
+ * Report source locations for structured documentation errors.
+
 
 
 Compiler Features:

liblangutil/ErrorReporter.cpp

@@ -244,3 +244,12 @@ void ErrorReporter::docstringParsingError(string const& _description)
 		_description
 	);
 }
+
+void ErrorReporter::docstringParsingError(SourceLocation const& _location, string const& _description)
+{
+	error(
+		Error::Type::DocstringParsingError,
+		_location,
+		_description
+	);
+}

liblangutil/ErrorReporter.h

@@ -107,6 +107,7 @@ public:
 	void fatalTypeError(SourceLocation const& _location, SecondarySourceLocation const& _secondLocation, std::string const& _description);
 
 	void docstringParsingError(std::string const& _description);
+	void docstringParsingError(SourceLocation const& _location, std::string const& _description);
 
 	ErrorList const& errors() const;
 

liblangutil/Scanner.cpp

@@ -306,19 +306,24 @@ bool Scanner::tryScanEndOfLine()
 	return false;
 }
 
-Token Scanner::scanSingleLineDocComment()
+int Scanner::scanSingleLineDocComment()
 {
 	LiteralScope literal(this, LITERAL_TYPE_COMMENT);
+	int endPosition = m_source->position();
 	advance(); //consume the last '/' at ///
 
 	skipWhitespaceExceptUnicodeLinebreak();
 
 	while (!isSourcePastEndOfInput())
 	{
+		endPosition = m_source->position();
 		if (tryScanEndOfLine())
 		{
-			// check if next line is also a documentation comment
-			skipWhitespace();
+			// Check if next line is also a single-line comment.
+			// If any whitespaces were skipped, use source position before.
+			if (!skipWhitespace())
+				endPosition = m_source->position();
+
 			if (!m_source->isPastEndOfInput(3) &&
 				m_source->get(0) == '/' &&
 				m_source->get(1) == '/' &&
@@ -338,7 +343,7 @@ Token Scanner::scanSingleLineDocComment()
 		advance();
 	}
 	literal.complete();
-	return Token::CommentLiteral;
+	return endPosition;
 }
 
 Token Scanner::skipMultiLineComment()
@@ -426,12 +431,10 @@ Token Scanner::scanSlash()
 		else if (m_char == '/')
 		{
 			// doxygen style /// comment
-			Token comment;
 			m_skippedComments[NextNext].location.start = firstSlashPosition;
 			m_skippedComments[NextNext].location.source = m_source;
-			comment = scanSingleLineDocComment();
-			m_skippedComments[NextNext].location.end = sourcePos();
-			m_skippedComments[NextNext].token = comment;
+			m_skippedComments[NextNext].token = Token::CommentLiteral;
+			m_skippedComments[NextNext].location.end = scanSingleLineDocComment();
 			return Token::Whitespace;
 		}
 		else

liblangutil/Scanner.h

@@ -229,7 +229,8 @@ private:
 
 	Token scanString();
 	Token scanHexString();
-	Token scanSingleLineDocComment();
+	/// Scans a single line comment and returns its corrected end position.
+	int scanSingleLineDocComment();
 	Token scanMultiLineDocComment();
 	/// Scans a slash '/' and depending on the characters returns the appropriate token
 	Token scanSlash();

libsolidity/analysis/DocStringAnalyser.cpp

@@ -73,6 +73,7 @@ bool DocStringAnalyser::visit(EventDefinition const& _event)
 
 void DocStringAnalyser::checkParameters(
 	CallableDeclaration const& _callable,
+	StructurallyDocumented const& _node,
 	StructurallyDocumentedAnnotation& _annotation
 )
 {
@@ -86,6 +87,7 @@ void DocStringAnalyser::checkParameters(
 	for (auto i = paramRange.first; i != paramRange.second; ++i)
 		if (!validParams.count(i->second.paramName))
 			appendError(
+				_node.documentation()->location(),
 				"Documented parameter \"" +
 				i->second.paramName +
 				"\" not found in the parameter list of the function."
@@ -101,7 +103,7 @@ void DocStringAnalyser::handleConstructor(
 {
 	static set<string> const validTags = set<string>{"author", "dev", "notice", "param"};
 	parseDocStrings(_node, _annotation, validTags, "constructor");
-	checkParameters(_callable, _annotation);
+	checkParameters(_callable, _node, _annotation);
 }
 
 void DocStringAnalyser::handleCallable(
@@ -112,7 +114,7 @@ void DocStringAnalyser::handleCallable(
 {
 	static set<string> const validTags = set<string>{"author", "dev", "notice", "return", "param"};
 	parseDocStrings(_node, _annotation, validTags, "functions");
-	checkParameters(_callable, _annotation);
+	checkParameters(_callable, _node, _annotation);
 }
 
 void DocStringAnalyser::parseDocStrings(
@@ -134,7 +136,10 @@ void DocStringAnalyser::parseDocStrings(
 	for (auto const& docTag: _annotation.docTags)
 	{
 		if (!_validTags.count(docTag.first))
-			appendError("Documentation tag @" + docTag.first + " not valid for " + _nodeName + ".");
+			appendError(
+				_node.documentation()->location(),
+				"Documentation tag @" + docTag.first + " not valid for " + _nodeName + "."
+			);
 		else
 			if (docTag.first == "return")
 			{
@@ -145,14 +150,18 @@ void DocStringAnalyser::parseDocStrings(
 					string firstWord = content.substr(0, content.find_first_of(" \t"));
 
 					if (returnTagsVisited > function->returnParameters().size())
-						appendError("Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
+						appendError(
+							_node.documentation()->location(),
+							"Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
 							" exceedes the number of return parameters."
 						);
 					else
 					{
 						auto parameter = function->returnParameters().at(returnTagsVisited - 1);
 						if (!parameter->name().empty() && parameter->name() != firstWord)
-							appendError("Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
+							appendError(
+								_node.documentation()->location(),
+								"Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
 								" does not contain the name of its return parameter."
 							);
 					}
@@ -161,8 +170,8 @@ void DocStringAnalyser::parseDocStrings(
 	}
 }
 
-void DocStringAnalyser::appendError(string const& _description)
+void DocStringAnalyser::appendError(SourceLocation const& _location, string const& _description)
 {
 	m_errorOccured = true;
-	m_errorReporter.docstringParsingError(_description);
+	m_errorReporter.docstringParsingError(_location, _description);
 }

libsolidity/analysis/DocStringAnalyser.h

@@ -51,6 +51,7 @@ private:
 
 	void checkParameters(
 		CallableDeclaration const& _callable,
+		StructurallyDocumented const& _node,
 		StructurallyDocumentedAnnotation& _annotation
 	);
 
@@ -73,7 +74,7 @@ private:
 		std::string const& _nodeName
 	);
 
-	void appendError(std::string const& _description);
+	void appendError(langutil::SourceLocation const& _location, std::string const& _description);
 
 	bool m_errorOccured = false;
 	langutil::ErrorReporter& m_errorReporter;

test/cmdlineTests/structured_documentation_source_location/err

@@ -0,0 +1,11 @@
+Error: Documentation tag "@return No value returned" does not contain the name of its return parameter.
+ --> structured_documentation_source_location/input.sol:3:5:
+  |
+3 |     /// @param id Some identifier
+  |     ^ (Relevant source part starts here and spans across multiple lines).
+
+Error: Documentation tag "@return No value returned" does not contain the name of its return parameter.
+ --> structured_documentation_source_location/input.sol:7:5:
+  |
+7 |     /// @return No value returned
+  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

test/cmdlineTests/structured_documentation_source_location/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/structured_documentation_source_location/input.sol

@@ -1,7 +1,9 @@
+pragma solidity >= 0.0;
 abstract contract C {
     /// @param id Some identifier
     /// @return No value returned
     function vote(uint id) public virtual returns (uint value);
-}
-// ----
-// DocstringParsingError: Documentation tag "@return No value returned" does not contain the name of its return parameter.
+
+    /// @return No value returned
+    function unvote(uint id) public virtual returns (uint value);
+}

test/libsolidity/syntaxTests/natspec/invalid/docstring_named_return_param_mismatch.sol

@@ -4,4 +4,4 @@ abstract contract C {
     function vote(uint id) public virtual returns (uint value);
 }
 // ----
-// DocstringParsingError: Documentation tag "@return No value returned" does not contain the name of its return parameter.
+// DocstringParsingError: (26-89): Documentation tag "@return No value returned" does not contain the name of its return parameter.

test/libsolidity/syntaxTests/natspec/invalid/docstring_parameter.sol

@@ -7,5 +7,5 @@ contract C {
     }
 }
 // ----
-// DocstringParsingError: Documented parameter "" not found in the parameter list of the function.
-// DocstringParsingError: Documented parameter "_" not found in the parameter list of the function.
+// DocstringParsingError: (17-101): Documented parameter "" not found in the parameter list of the function.
+// DocstringParsingError: (17-101): Documented parameter "_" not found in the parameter list of the function.

test/libsolidity/syntaxTests/natspec/invalid/docstring_return_size_mismatch.sol

@@ -0,0 +1,11 @@
+abstract contract C {
+    /// @param id Some identifier
+    /// @return No value returned
+    function vote(uint id) public virtual returns (uint value);
+
+    /// @return No value returned
+    function unvote(uint id) public virtual returns (uint value);
+}
+// ----
+// DocstringParsingError: (26-89): Documentation tag "@return No value returned" does not contain the name of its return parameter.
+// DocstringParsingError: (159-188): Documentation tag "@return No value returned" does not contain the name of its return parameter.