Skip to content

Deprecate the var keyword in SyntaxChecker #3425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chriseth merged 3 commits into from
Feb 13, 2018
Merged

Deprecate the var keyword in SyntaxChecker #3425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc5ad89
Deprecate the var keyword (and mark it an error as experimental 0.5.0…
jevogel Jan 22, 2018
3ddbf16
Do not use var where not neccessary in tests
axic Feb 12, 2018
573aa01
Adjust tests for multiple errors with the var keyword
axic Feb 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Features:
* Inline Assembly: Support some restricted tokens (return, byte, address) as identifiers in Julia mode.
* SMT Checker: If-else branch conditions are taken into account in the SMT encoding of the program
variables.
* Syntax Checker: Deprecate the ``var`` keyword (and mark it an error as experimental 0.5.0 feature).
* Type Checker: Issue warning for using ``public`` visibility for interface functions.

Bugfixes:
Expand Down
14 changes: 14 additions & 0 deletions libsolidity/analysis/SyntaxChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,17 @@ bool SyntaxChecker::visit(FunctionTypeName const& _node)

return true;
}

bool SyntaxChecker::visit(VariableDeclaration const& _declaration)
{
bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);

if (!_declaration.typeName())
{
if (v050)
m_errorReporter.syntaxError(_declaration.location(), "Use of the \"var\" keyword is deprecated.");
else
m_errorReporter.warning(_declaration.location(), "Use of the \"var\" keyword is deprecated.");
}
return true;
}
2 changes: 2 additions & 0 deletions libsolidity/analysis/SyntaxChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ class SyntaxChecker: private ASTConstVisitor
virtual bool visit(FunctionDefinition const& _function) override;
virtual bool visit(FunctionTypeName const& _node) override;

virtual bool visit(VariableDeclaration const& _declaration) override;

ErrorReporter& m_errorReporter;

/// Flag that indicates whether a function modifier actually contains '_'.
Expand Down
32 changes: 21 additions & 11 deletions test/libsolidity/SolidityNameAndTypeResolution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,10 @@ BOOST_AUTO_TEST_CASE(warn_var_from_zero)
}
}
)";
CHECK_WARNING(sourceCode, "uint8, which can hold values between 0 and 255");
Copy link
Contributor

@chriseth chriseth Jan 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be checked that this error message is present until we disallow var completely.

Copy link
Contributor Author

@jevogel jevogel Jan 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify? CHECK_WARNING_ALLOW_MULTI seems to only allow the first warning to be checked, so if var is used in this test it will cause the first warning and it will not be possible to check for the size warning. Is there a way to check both?

Copy link
Contributor

@chriseth chriseth Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear someone has to extend the macro...

Copy link
Contributor Author

@jevogel jevogel Jan 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could look into this within the next week or so. Do you want to file it as a separate issue?

CHECK_WARNING_ALLOW_MULTI(sourceCode, (std::vector<std::string>{
"uint8, which can hold values between 0 and 255",
"Use of the \"var\" keyword is deprecated."
}));
sourceCode = R"(
contract test {
function f() pure public {
Expand All @@ -1853,7 +1856,10 @@ BOOST_AUTO_TEST_CASE(warn_var_from_zero)
}
}
)";
CHECK_WARNING(sourceCode, "uint256, which can hold values between 0 and 115792089237316195423570985008687907853269984665640564039457584007913129639935");
CHECK_WARNING_ALLOW_MULTI(sourceCode, (std::vector<std::string>{
"uint256, which can hold values between 0 and 115792089237316195423570985008687907853269984665640564039457584007913129639935",
"Use of the \"var\" keyword is deprecated."
}));
sourceCode = R"(
contract test {
function f() pure public {
Expand All @@ -1862,15 +1868,21 @@ BOOST_AUTO_TEST_CASE(warn_var_from_zero)
}
}
)";
CHECK_WARNING(sourceCode, "int8, which can hold values between -128 and 127");
CHECK_WARNING_ALLOW_MULTI(sourceCode, (std::vector<std::string>{
"int8, which can hold values between -128 and 127",
"Use of the \"var\" keyword is deprecated."
}));
sourceCode = R"(
contract test {
function f() pure public {
for (var i = 0; i < msg.data.length; i++) { }
}
}
)";
CHECK_WARNING(sourceCode, "uint8, which can hold");
CHECK_WARNING_ALLOW_MULTI(sourceCode, (std::vector<std::string>{
"uint8, which can hold",
"Use of the \"var\" keyword is deprecated."
}));
}

BOOST_AUTO_TEST_CASE(enum_member_access)
Expand Down Expand Up @@ -4887,8 +4899,7 @@ BOOST_AUTO_TEST_CASE(warn_about_callcode)
char const* text = R"(
contract test {
function f() pure public {
var x = address(0x12).callcode;
x;
address(0x12).callcode;
}
}
)";
Expand All @@ -4897,8 +4908,7 @@ BOOST_AUTO_TEST_CASE(warn_about_callcode)
pragma experimental "v0.5.0";
contract test {
function f() pure public {
var x = address(0x12).callcode;
x;
address(0x12).callcode;
}
Copy link
Contributor

@chriseth chriseth Jan 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

}
)";
Expand Down Expand Up @@ -6918,7 +6928,7 @@ BOOST_AUTO_TEST_CASE(function_types_sig)
}
}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
CHECK_WARNING(text, "Use of the \"var\" keyword is deprecated.");
text = R"(
contract C {
function h() pure external {
Expand All @@ -6941,7 +6951,7 @@ BOOST_AUTO_TEST_CASE(function_types_sig)
}
Copy link
Contributor

@chriseth chriseth Jan 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't modify the test by adding a type for i.

}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
CHECK_WARNING(text, "Use of the \"var\" keyword is deprecated.");
}

BOOST_AUTO_TEST_CASE(using_this_in_constructor)
Expand Down Expand Up @@ -7349,7 +7359,7 @@ BOOST_AUTO_TEST_CASE(warn_about_sha3)
char const* text = R"(
contract test {
function f() pure public {
var x = sha3(uint8(1));
bytes32 x = sha3(uint8(1));
x;
}
}
Expand Down
14 changes: 8 additions & 6 deletions test/libsolidity/ViewPureChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,12 @@ BOOST_AUTO_TEST_CASE(environment_access)
}
for (string const& x: pure)
{
CHECK_WARNING(
CHECK_WARNING_ALLOW_MULTI(
"contract C { function f() view public { var x = " + x + "; x; } }",
"restricted to pure"
);
(std::vector<std::string>{
"Function state mutability can be restricted to pure",
"Use of the \"var\" keyword is deprecated."
}));
}
}

Expand Down Expand Up @@ -282,9 +284,9 @@ BOOST_AUTO_TEST_CASE(builtin_functions)
require(this.call());
}
function g() pure public {
var x = keccak256("abc");
var y = sha256("abc");
var z = ecrecover(1, 2, 3, 4);
bytes32 x = keccak256("abc");
bytes32 y = sha256("abc");
address z = ecrecover(1, 2, 3, 4);
require(true);
assert(true);
x; y; z;
Expand Down