diff --git a/Changelog.md b/Changelog.md index 3c9d79694..f00d555e9 100644 --- a/Changelog.md +++ b/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: diff --git a/liblangutil/ErrorReporter.cpp b/liblangutil/ErrorReporter.cpp index 312f159c0..c6c059bc7 100644 --- a/liblangutil/ErrorReporter.cpp +++ b/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 + ); +} diff --git a/liblangutil/ErrorReporter.h b/liblangutil/ErrorReporter.h index 3acf5afee..5aca4beb9 100644 --- a/liblangutil/ErrorReporter.h +++ b/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; diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index 9b9ad8847..9d748c570 100644 --- a/liblangutil/Scanner.cpp +++ b/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 diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h index 1e08e0b6e..8f6cdab87 100644 --- a/liblangutil/Scanner.h +++ b/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(); diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index cadd12fc3..e802c4204 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/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 const validTags = set{"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 const validTags = set{"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); } diff --git a/libsolidity/analysis/DocStringAnalyser.h b/libsolidity/analysis/DocStringAnalyser.h index 2b9e23195..b9f816854 100644 --- a/libsolidity/analysis/DocStringAnalyser.h +++ b/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; diff --git a/test/cmdlineTests/structured_documentation_source_location/err b/test/cmdlineTests/structured_documentation_source_location/err new file mode 100644 index 000000000..1081fdaa8 --- /dev/null +++ b/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 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test/cmdlineTests/structured_documentation_source_location/exit b/test/cmdlineTests/structured_documentation_source_location/exit new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/test/cmdlineTests/structured_documentation_source_location/exit @@ -0,0 +1 @@ +1 diff --git a/test/libsolidity/syntaxTests/natspec/invalid/dosctring_return_size_mismatch.sol b/test/cmdlineTests/structured_documentation_source_location/input.sol similarity index 53% rename from test/libsolidity/syntaxTests/natspec/invalid/dosctring_return_size_mismatch.sol rename to test/cmdlineTests/structured_documentation_source_location/input.sol index 23f4828e4..ceec13547 100644 --- a/test/libsolidity/syntaxTests/natspec/invalid/dosctring_return_size_mismatch.sol +++ b/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); +} \ No newline at end of file diff --git a/test/libsolidity/syntaxTests/natspec/dosctring_named_return_parameter.sol b/test/libsolidity/syntaxTests/natspec/docstring_named_return_parameter.sol similarity index 100% rename from test/libsolidity/syntaxTests/natspec/dosctring_named_return_parameter.sol rename to test/libsolidity/syntaxTests/natspec/docstring_named_return_parameter.sol diff --git a/test/libsolidity/syntaxTests/natspec/invalid/dosctring_named_return_param_mismatch.sol b/test/libsolidity/syntaxTests/natspec/invalid/docstring_named_return_param_mismatch.sol similarity index 55% rename from test/libsolidity/syntaxTests/natspec/invalid/dosctring_named_return_param_mismatch.sol rename to test/libsolidity/syntaxTests/natspec/invalid/docstring_named_return_param_mismatch.sol index 23f4828e4..c2f29c038 100644 --- a/test/libsolidity/syntaxTests/natspec/invalid/dosctring_named_return_param_mismatch.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/natspec/invalid/docstring_parameter.sol b/test/libsolidity/syntaxTests/natspec/invalid/docstring_parameter.sol index d0a39af65..3e6dc0885 100644 --- a/test/libsolidity/syntaxTests/natspec/invalid/docstring_parameter.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/natspec/invalid/docstring_return_size_mismatch.sol b/test/libsolidity/syntaxTests/natspec/invalid/docstring_return_size_mismatch.sol new file mode 100644 index 000000000..6e95d788d --- /dev/null +++ b/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.