From cb5bfc7436ca11bdfa2f0543a3611079e3f4cbee Mon Sep 17 00:00:00 2001 From: Alexander Arlt Date: Wed, 20 May 2020 11:25:23 -0500 Subject: [PATCH] Update natspec comments on state variables. - changing some warnings to errors --- libsolidity/analysis/DocStringAnalyser.cpp | 19 +++---------------- libsolidity/parsing/Parser.cpp | 2 +- test/compilationTests/corion/premium.sol | 12 ++++++------ test/compilationTests/corion/token.sol | 13 +++++++------ test/libsolidity/SolidityParser.cpp | 8 ++++---- .../docstring_author_title_state_variable.sol | 3 ++- ..._non_public_state_variable_with_return.sol | 2 +- .../docstring_private_state_variable.sol | 2 +- .../natspec/docstring_variable.sol | 2 +- 9 files changed, 26 insertions(+), 37 deletions(-) diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index 7d8a8a43f..9031b7574 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -60,25 +60,12 @@ bool DocStringAnalyser::visit(VariableDeclaration const& _variable) { if (_variable.isStateVariable()) { - static set const validPublicTags = set{"dev", "notice", "return", "title", "author"}; + static set const validPublicTags = set{"dev", "notice", "return"}; + static set const validNonPublicTags = set{"dev"}; if (_variable.isPublic()) parseDocStrings(_variable, _variable.annotation(), validPublicTags, "public state variables"); else - { - parseDocStrings(_variable, _variable.annotation(), validPublicTags, "non-public state variables"); - if (_variable.annotation().docTags.count("notice") > 0) - m_errorReporter.warning( - 7816_error, _variable.documentation()->location(), - "Documentation tag on non-public state variables will be disallowed in 0.7.0. " - "You will need to use the @dev tag explicitly." - ); - } - if (_variable.annotation().docTags.count("title") > 0 || _variable.annotation().docTags.count("author") > 0) - m_errorReporter.warning( - 8532_error, _variable.documentation()->location(), - "Documentation tag @title and @author is only allowed on contract definitions. " - "It will be disallowed in 0.7.0." - ); + parseDocStrings(_variable, _variable.annotation(), validNonPublicTags, "non-public state variables"); } return false; } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index c429ba786..ab8180dc7 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -697,7 +697,7 @@ ASTPointer Parser::parseVariableDeclaration( } if (!_options.isStateVariable && documentation != nullptr) - parserWarning(2837_error, "Only state variables can have a docstring. This will be disallowed in 0.7.0."); + parserError(2837_error, "Only state variables can have a docstring."); if (dynamic_cast(type.get()) && _options.isStateVariable && m_scanner->currentToken() == Token::LBrace) fatalParserError( diff --git a/test/compilationTests/corion/premium.sol b/test/compilationTests/corion/premium.sol index d3b842608..a703bec02 100644 --- a/test/compilationTests/corion/premium.sol +++ b/test/compilationTests/corion/premium.sol @@ -11,6 +11,12 @@ contract thirdPartyPContractAbstract { contract ptokenDB is tokenDB {} +/** + * + * @title Corion Platform Premium Token + * @author iFA @ Corion Platform + * + */ contract premium is module, safeMath { function replaceModule(address payable addr) external override returns (bool success) { require( super.isModuleHandler(msg.sender) ); @@ -23,12 +29,6 @@ contract premium is module, safeMath { require( _success && _active ); _; } - /** - * - * @title Corion Platform Premium Token - * @author iFA @ Corion Platform - * - */ string public name = "Corion Premium"; string public symbol = "CORP"; diff --git a/test/compilationTests/corion/token.sol b/test/compilationTests/corion/token.sol index 0d67feaa9..0888744db 100644 --- a/test/compilationTests/corion/token.sol +++ b/test/compilationTests/corion/token.sol @@ -11,6 +11,12 @@ contract thirdPartyContractAbstract { function approvedCorionToken(address, uint256, bytes calldata) external returns (bool) {} } +/** + * + * @title Corion Platform Token + * @author iFA @ Corion Platform + * + */ contract token is safeMath, module, announcementTypes { /* module callbacks @@ -26,12 +32,7 @@ contract token is safeMath, module, announcementTypes { require( _success && _active ); _; } - /** - * - * @title Corion Platform Token - * @author iFA @ Corion Platform - * - */ + string public name = "Corion"; string public symbol = "COR"; uint8 public decimals = 6; diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp index 12fbc3fce..ac1a56afd 100644 --- a/test/libsolidity/SolidityParser.cpp +++ b/test/libsolidity/SolidityParser.cpp @@ -254,7 +254,7 @@ BOOST_AUTO_TEST_CASE(natspec_comment_in_function_body) /// fun1 description function fun1(uint256 a) { var b; - /// I should not interfere with actual natspec comments + // I should not interfere with actual natspec comments (natspec comments on local variables not allowed anymore) uint256 c; mapping(address=>bytes32) d; bytes7 name = "Solidity"; @@ -286,7 +286,7 @@ BOOST_AUTO_TEST_CASE(natspec_docstring_between_keyword_and_signature) function ///I am in the wrong place fun1(uint256 a) { var b; - /// I should not interfere with actual natspec comments + // I should not interfere with actual natspec comments (natspec comments on local variables not allowed anymore) uint256 c; mapping(address=>bytes32) d; bytes7 name = "Solidity"; @@ -310,9 +310,9 @@ BOOST_AUTO_TEST_CASE(natspec_docstring_after_signature) contract test { uint256 stateVar; function fun1(uint256 a) { - /// I should have been above the function signature + // I should have been above the function signature (natspec comments on local variables not allowed anymore) var b; - /// I should not interfere with actual natspec comments + // I should not interfere with actual natspec comments (natspec comments on local variables not allowed anymore) uint256 c; mapping(address=>bytes32) d; bytes7 name = "Solidity"; diff --git a/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol index 78aaf920c..d7e52608c 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_author_title_state_variable.sol @@ -4,4 +4,5 @@ contract C { uint private state; } // ---- -// Warning: (17-56): Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0. +// DocstringParsingError: (17-56): Documentation tag @author not valid for non-public state variables. +// DocstringParsingError: (17-56): Documentation tag @title not valid for non-public state variables. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol b/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol index 2b079ee7f..d8f70ebd8 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_non_public_state_variable_with_return.sol @@ -3,4 +3,4 @@ contract test { uint private state; } // ---- -// DocstringParsingError: (18-47): Documentation tag "@return" is only allowed on public state-variables. +// DocstringParsingError: (18-47): Documentation tag @return not valid for non-public state variables. diff --git a/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol b/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol index 780501230..dbc5c9a0b 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_private_state_variable.sol @@ -4,4 +4,4 @@ contract C { uint private state; } // ---- -// Warning: (17-74): Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly. +// 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 index 726eba852..f1becfb5e 100644 --- a/test/libsolidity/syntaxTests/natspec/docstring_variable.sol +++ b/test/libsolidity/syntaxTests/natspec/docstring_variable.sol @@ -11,4 +11,4 @@ contract C { } } // ---- -// Warning: (290-295): Only state variables can have a docstring. This will be disallowed in 0.7.0. +// ParserError: (290-295): Only state variables can have a docstring.