From 8fe1cfb12ef49a74eaebed56d160e88cfd9a4de2 Mon Sep 17 00:00:00 2001 From: bitshift Date: Fri, 9 Mar 2018 17:38:17 +0100 Subject: [PATCH 1/3] Defaults to external visibility for interfaces. --- libsolidity/analysis/StaticAnalyzer.cpp | 8 ++++++++ libsolidity/analysis/SyntaxChecker.cpp | 15 +++------------ .../visibility/interface/function_default.sol | 6 ++++++ .../visibility/interface/function_default050.sol | 7 +++++++ 4 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_default.sol create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_default050.sol diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 6aee260e7..20464765b 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -50,6 +50,14 @@ void StaticAnalyzer::endVisit(ContractDefinition const&) bool StaticAnalyzer::visit(FunctionDefinition const& _function) { + const bool isInterface = m_currentContract->contractKind() == ContractDefinition::ContractKind::Interface; + if (_function.noVisibilitySpecified()) + m_errorReporter.warning( + _function.location(), + "No visibility specified. Defaulting to \"" + + (isInterface ? "external" : Declaration::visibilityToString(_function.visibility())) + + "\"." + ); if (_function.isImplemented()) m_currentFunction = &_function; else diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index ddac194b9..5de9a2705 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -214,18 +214,9 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) { bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (_function.noVisibilitySpecified()) - { - if (v050) - m_errorReporter.syntaxError(_function.location(), "No visibility specified."); - else - m_errorReporter.warning( - _function.location(), - "No visibility specified. Defaulting to \"" + - Declaration::visibilityToString(_function.visibility()) + - "\"." - ); - } + if (v050 && _function.noVisibilitySpecified()) + m_errorReporter.syntaxError(_function.location(), "No visibility specified."); + return true; } diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_default.sol b/test/libsolidity/syntaxTests/visibility/interface/function_default.sol new file mode 100644 index 000000000..9889d038a --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_default.sol @@ -0,0 +1,6 @@ +interface I { + function f(); +} +// ---- +// Warning: Functions in interfaces should be declared external. +// Warning: No visibility specified. Defaulting to "external". diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_default050.sol b/test/libsolidity/syntaxTests/visibility/interface/function_default050.sol new file mode 100644 index 000000000..127d4e92a --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_default050.sol @@ -0,0 +1,7 @@ +pragma experimental "v0.5.0"; +interface I { + function f(); +} +// ---- +// SyntaxError: No visibility specified. +// TypeError: Functions in interfaces must be declared external. From 601659c3844767686a324ea363498e85320c92b1 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Fri, 23 Mar 2018 15:09:45 +0100 Subject: [PATCH 2/3] Moves all interface function visibility related tests. --- .../SolidityNameAndTypeResolution.cpp | 48 ------------------- .../interface/function_external050.sol | 5 ++ .../interface/function_internal.sol | 5 ++ .../visibility/interface/function_private.sol | 5 ++ .../visibility/interface/function_public.sol | 5 ++ .../interface/function_public050.sol | 6 +++ 6 files changed, 26 insertions(+), 48 deletions(-) create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_external050.sol create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_internal.sol create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_private.sol create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_public.sol create mode 100644 test/libsolidity/syntaxTests/visibility/interface/function_public050.sol diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index e5269bc5f..1e98d50d1 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -6429,54 +6429,6 @@ BOOST_AUTO_TEST_CASE(interface_function_bodies) CHECK_ERROR(text, TypeError, "Functions in interfaces cannot have an implementation"); } -BOOST_AUTO_TEST_CASE(interface_function_external) -{ - char const* text = R"( - pragma experimental "v0.5.0"; - interface I { - function f() external; - } - )"; - CHECK_SUCCESS(text); -} - -BOOST_AUTO_TEST_CASE(interface_function_public) -{ - char const* text = R"( - interface I { - function f() public; - } - )"; - CHECK_WARNING(text, "Functions in interfaces should be declared external."); - text = R"( - pragma experimental "v0.5.0"; - interface I { - function f() public; - } - )"; - CHECK_ERROR(text, TypeError, "Functions in interfaces must be declared external."); -} - -BOOST_AUTO_TEST_CASE(interface_function_internal) -{ - char const* text = R"( - interface I { - function f() internal; - } - )"; - CHECK_ERROR(text, TypeError, "Functions in interfaces cannot be internal or private."); -} - -BOOST_AUTO_TEST_CASE(interface_function_private) -{ - char const* text = R"( - interface I { - function f() private; - } - )"; - CHECK_ERROR(text, TypeError, "Functions in interfaces cannot be internal or private."); -} - BOOST_AUTO_TEST_CASE(interface_events) { char const* text = R"( diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_external050.sol b/test/libsolidity/syntaxTests/visibility/interface/function_external050.sol new file mode 100644 index 000000000..3f0a9acab --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_external050.sol @@ -0,0 +1,5 @@ +pragma experimental "v0.5.0"; +interface I { + function f() external; +} +// ---- diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_internal.sol b/test/libsolidity/syntaxTests/visibility/interface/function_internal.sol new file mode 100644 index 000000000..a1cf52463 --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_internal.sol @@ -0,0 +1,5 @@ +interface I { + function f() internal; +} +// ---- +// TypeError: Functions in interfaces cannot be internal or private. diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_private.sol b/test/libsolidity/syntaxTests/visibility/interface/function_private.sol new file mode 100644 index 000000000..887ebd4bf --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_private.sol @@ -0,0 +1,5 @@ +interface I { + function f() private; +} +// ---- +// TypeError: Functions in interfaces cannot be internal or private. diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_public.sol b/test/libsolidity/syntaxTests/visibility/interface/function_public.sol new file mode 100644 index 000000000..146d4f5b5 --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_public.sol @@ -0,0 +1,5 @@ +interface I { + function f() public; +} +// ---- +// Warning: Functions in interfaces should be declared external. \ No newline at end of file diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_public050.sol b/test/libsolidity/syntaxTests/visibility/interface/function_public050.sol new file mode 100644 index 000000000..f957f0b4c --- /dev/null +++ b/test/libsolidity/syntaxTests/visibility/interface/function_public050.sol @@ -0,0 +1,6 @@ +pragma experimental "v0.5.0"; +interface I { + function f() public; +} +// ---- +// TypeError: Functions in interfaces must be declared external. \ No newline at end of file From f9efa417492916546d23115da7a55e86090d47dd Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 28 Mar 2018 18:10:32 +0200 Subject: [PATCH 3/3] Makes visibility warning more concise. --- libsolidity/analysis/StaticAnalyzer.cpp | 6 ++++-- .../syntaxTests/visibility/interface/function_default.sol | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 20464765b..d96f87484 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -51,12 +51,14 @@ void StaticAnalyzer::endVisit(ContractDefinition const&) bool StaticAnalyzer::visit(FunctionDefinition const& _function) { const bool isInterface = m_currentContract->contractKind() == ContractDefinition::ContractKind::Interface; + if (_function.noVisibilitySpecified()) m_errorReporter.warning( _function.location(), "No visibility specified. Defaulting to \"" + - (isInterface ? "external" : Declaration::visibilityToString(_function.visibility())) + - "\"." + Declaration::visibilityToString(_function.visibility()) + + "\". " + + (isInterface ? "In interfaces it defaults to external." : "") ); if (_function.isImplemented()) m_currentFunction = &_function; diff --git a/test/libsolidity/syntaxTests/visibility/interface/function_default.sol b/test/libsolidity/syntaxTests/visibility/interface/function_default.sol index 9889d038a..7b9044dd2 100644 --- a/test/libsolidity/syntaxTests/visibility/interface/function_default.sol +++ b/test/libsolidity/syntaxTests/visibility/interface/function_default.sol @@ -3,4 +3,4 @@ interface I { } // ---- // Warning: Functions in interfaces should be declared external. -// Warning: No visibility specified. Defaulting to "external". +// Warning: No visibility specified. Defaulting to "public". In interfaces it defaults to external.