Merge pull request #8413 from mijovic/depratateValueCalls

Deprecated warning for .value() and .gas() on function and constructr…

Changelog.md

@@ -2,6 +2,7 @@
 
 Language Features:
  * Inline Assembly: Allow assigning to `_slot` of local storage variable pointers.
+ * General: Deprecated `value(...)` and `gas(...)` in favor of `{value: ...}` and `{gas: ...}`
 
 
 Compiler Features:

docs/types/value-types.rst

@@ -646,7 +646,7 @@ External (or public) functions have the following members:
 Example that shows how to use the members::
 
     pragma solidity >=0.4.16 <0.7.0;
-
+    // This will report a warning
 
     contract Example {
         function f() public payable returns (bytes4) {

libsolidity/analysis/TypeChecker.cpp

@@ -2312,7 +2312,11 @@ bool TypeChecker::visit(FunctionCallOptions const& _functionCallOptions)
 			else if (!expressionFunctionType->isPayable())
 				m_errorReporter.typeError(
 					_functionCallOptions.location(),
-					"Cannot set option \"value\" on a non-payable function type."
+					kind == FunctionType::Kind::Creation ?
+						"Cannot set option \"value\", since the constructor of " +
+						expressionFunctionType->returnParameterTypes().front()->toString() +
+						" is not payable." :
+						"Cannot set option \"value\" on a non-payable function type."
 				);
 			else
 			{
@@ -2522,12 +2526,24 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
 	annotation.type = possibleMembers.front().type;
 
 	if (auto funType = dynamic_cast<FunctionType const*>(annotation.type))
+	{
 		solAssert(
 			!funType->bound() || exprType->isImplicitlyConvertibleTo(*funType->selfType()),
 			"Function \"" + memberName + "\" cannot be called on an object of type " +
 			exprType->toString() + " (expected " + funType->selfType()->toString() + ")."
 		);
 
+		if (
+			dynamic_cast<FunctionType const*>(exprType) &&
+			!annotation.referencedDeclaration &&
+			(memberName == "value" || memberName == "gas")
+		)
+			m_errorReporter.warning(
+				_memberAccess.location(),
+				"Using \"." + memberName + "(...)\" is deprecated. Use \"{" + memberName + ": ...}\" instead."
+			);
+	}
+
 	if (auto const* structType = dynamic_cast<StructType const*>(exprType))
 		annotation.isLValue = !structType->dataStoredIn(DataLocation::CallData);
 	else if (exprType->category() == Type::Category::Array)

test/libsolidity/semanticTests/tryCatch/assert.sol

@@ -4,7 +4,7 @@ contract C {
     }
     function f(bool x) public returns (uint) {
         // Set the gas to make this work on pre-byzantium VMs
-        try this.g.gas(8000)(x) {
+        try this.g{gas: 8000}(x) {
             return 1;
         } catch {
             return 2;

test/libsolidity/semanticTests/tryCatch/trivial.sol

@@ -4,7 +4,7 @@ contract C {
     }
     function f(bool x) public returns (uint) {
         // Set the gas to make this work on pre-byzantium VMs
-        try this.g.gas(8000)(x) {
+        try this.g{gas: 8000}(x) {
             return 1;
         } catch {
             return 2;

test/libsolidity/smtCheckerTests/types/function_type_members.sol

@@ -3,9 +3,12 @@ contract C {
     function f(function(uint) external payable g) internal {
 		g.selector;
 		g.gas(2).value(3)(4);
+		g{gas: 2, value: 3}(4);
     }
 }
 // ----
+// Warning: (122-127): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (122-136): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
 // Warning: (108-118): Assertion checker does not yet support this expression.
 // Warning: (122-130): Assertion checker does not yet implement this type of function call.
 // Warning: (122-139): Assertion checker does not yet implement this type of function call.

test/libsolidity/syntaxTests/constructor/constructor_payable.sol

@@ -0,0 +1,10 @@
+contract C {
+    constructor() public payable { }
+}
+
+contract D {
+    function createC() public returns (C) {
+       C c = (new C){value: 1}();
+       return c;
+    }
+}

test/libsolidity/syntaxTests/functionCalls/calloptions_repeated.sol

@@ -15,6 +15,7 @@ contract C {
 // TypeError: (78-110): Option "gas" has already been set.
 // TypeError: (120-154): Option "gas" has already been set.
 // TypeError: (164-198): Option "value" has already been set.
+// Warning: (208-222): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
 // TypeError: (208-242): Option "value" has already been set.
 // TypeError: (252-293): Option "value" has already been set.
 // TypeError: (252-293): Option "gas" has already been set.

test/libsolidity/syntaxTests/functionCalls/new_with_invalid_calloptions.sol

@@ -14,14 +14,14 @@ contract C {
 // ====
 // EVMVersion: >=constantinople
 // ----
-// TypeError: (64-98): Cannot set option "value" on a non-payable function type.
+// TypeError: (64-98): Cannot set option "value", since the constructor of contract D is not payable.
 // TypeError: (64-98): Function call option "gas" cannot be used with "new".
 // TypeError: (102-123): Unknown call option "slt". Valid options are "salt", "value" and "gas".
-// TypeError: (102-123): Cannot set option "value" on a non-payable function type.
+// TypeError: (102-123): Cannot set option "value", since the constructor of contract D is not payable.
 // TypeError: (127-139): Unknown call option "val". Valid options are "salt", "value" and "gas".
 // TypeError: (143-172): Duplicate option "salt".
-// TypeError: (176-199): Cannot set option "value" on a non-payable function type.
-// TypeError: (176-199): Cannot set option "value" on a non-payable function type.
+// TypeError: (176-199): Cannot set option "value", since the constructor of contract D is not payable.
+// TypeError: (176-199): Cannot set option "value", since the constructor of contract D is not payable.
 // TypeError: (203-220): Unknown call option "random". Valid options are "salt", "value" and "gas".
 // TypeError: (224-242): Unknown call option "what". Valid options are "salt", "value" and "gas".
 // TypeError: (246-259): Function call option "gas" cannot be used with "new".

test/libsolidity/syntaxTests/functionTypes/call_gas_on_function.sol

@@ -0,0 +1,7 @@
+contract C {
+    function (uint) external returns (uint) x;
+    function f() public {
+        x{gas: 2}(1);
+    }
+}
+

test/libsolidity/syntaxTests/functionTypes/call_value_library_function.sol

@@ -0,0 +1,10 @@
+library L {
+    function value(function()internal a, uint256 b) internal {}
+}
+contract C {
+    using L for function()internal;
+    function f() public {
+        function()internal x;
+        x.value(42);
+    }
+}

test/libsolidity/syntaxTests/functionTypes/call_value_on_non_payable_function_type.sol

@@ -1,7 +1,7 @@
 contract C {
     function (uint) external returns (uint) x;
     function f() public {
-        x.value(2)();
+        x.value(2)(1);
     }
 }
 // ----

test/libsolidity/syntaxTests/functionTypes/call_value_on_payable_function_type.sol

@@ -1,6 +1,6 @@
 contract C {
     function (uint) external payable returns (uint) x;
     function f() public {
-        x.value(2)(1);
+        x{value: 2}(1);
     }
 }

test/libsolidity/syntaxTests/functionTypes/call_value_options_on_non_payable_function_type.sol

@@ -0,0 +1,8 @@
+contract C {
+    function (uint) external returns (uint) x;
+    function g() public {
+        x{value: 2}(1);
+    }
+}
+// ----
+// TypeError: (94-105): Cannot set option "value" on a non-payable function type.

test/libsolidity/syntaxTests/functionTypes/warn_deprecate_gas_function.sol

@@ -0,0 +1,8 @@
+contract C {
+    function (uint) external payable returns (uint) x;
+    function f() public {
+        x.gas(2)(1);
+    }
+}
+// ----
+// Warning: (102-107): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.

test/libsolidity/syntaxTests/functionTypes/warn_deprecate_value_constructor.sol

@@ -0,0 +1,11 @@
+contract C {
+    constructor() payable public {}
+}
+contract D {
+    function createC() public returns (C) {
+        C c = (new C).value(2)();
+        return c;
+    }
+}
+// ----
+// Warning: (122-135): Using ".value(...)" is deprecated. Use "{value: ...}" instead.

test/libsolidity/syntaxTests/functionTypes/warn_deprecate_value_function.sol

@@ -0,0 +1,8 @@
+contract C {
+    function (uint) external payable returns (uint) x;
+    function f() public {
+        x.value(2)(1);
+    }
+}
+// ----
+// Warning: (102-109): Using ".value(...)" is deprecated. Use "{value: ...}" instead.

test/libsolidity/syntaxTests/nameAndTypeResolution/348_unused_return_value_call_value.sol

@@ -1,7 +1,10 @@
 contract test {
     function f() public {
         address(0x12).call.value(2)("abc");
+        address(0x12).call{value: 2}("abc");
     }
 }
 // ----
+// Warning: (50-74): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
 // Warning: (50-84): Return value of low-level calls not used.
+// Warning: (94-129): Return value of low-level calls not used.

test/libsolidity/syntaxTests/nameAndTypeResolution/361_calling_payable.sol

@@ -1,6 +1,11 @@
 contract receiver { function pay() payable public {} }
 contract test {
-    function f() public { (new receiver()).pay.value(10)(); }
+    function f() public { (new receiver()).pay{value: 10}(); }
+    function g() public { (new receiver()).pay.value(10)(); }
     receiver r = new receiver();
-    function g() public { r.pay.value(10)(); }
+    function h() public { r.pay{value: 10}(); }
+    function i() public { r.pay.value(10)(); }
 }
+// ----
+// Warning: (160-186): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
+// Warning: (303-314): Using ".value(...)" is deprecated. Use "{value: ...}" instead.

test/libsolidity/syntaxTests/nameAndTypeResolution/362_calling_nonpayable.sol

@@ -1,6 +1,8 @@
 contract receiver { function nopay() public {} }
 contract test {
-    function f() public { (new receiver()).nopay.value(10)(); }
+    function f() public { (new receiver()).nopay{value: 10}(); }
+    function g() public { (new receiver()).nopay.value(10)(); }
 }
 // ----
-// TypeError: (91-119): Member "value" is only available for payable functions.
+// TypeError: (91-124): Cannot set option "value" on a non-payable function type.
+// TypeError: (156-184): Member "value" is only available for payable functions.

test/libsolidity/syntaxTests/nameAndTypeResolution/399_does_not_warn_msg_value_in_internal_function.sol

@@ -3,3 +3,4 @@ contract C {
         msg.value;
     }
 }
+

test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_internal_functions.sol

@@ -5,6 +5,7 @@ contract C {
     }
 }
 // ----
+// Warning: (105-115): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
 // TypeError: (91-100): This type cannot be encoded.
 // TypeError: (102-103): This type cannot be encoded.
 // TypeError: (105-115): This type cannot be encoded.

test/libsolidity/syntaxTests/viewPureChecker/gas_value_without_call.sol

@@ -2,12 +2,25 @@ contract C {
     function f() external payable {}
 	function g(address a) external pure {
 		a.call.value(42);
+		a.call{value: 42};
 		a.call.gas(42);
+		a.call{gas: 42};
 		a.staticcall.gas(42);
+		a.staticcall{gas: 42};
 		a.delegatecall.gas(42);
+		a.delegatecall{gas: 42};
 	}
 	function h() external view {
 		this.f.value(42);
+		this.f{value: 42};
 		this.f.gas(42);
+		this.f{gas: 42};
 	}
 }
+// ----
+// Warning: (91-103): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
+// Warning: (132-142): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (169-185): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (218-236): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (304-316): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
+// Warning: (345-355): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.

test/libsolidity/syntaxTests/viewPureChecker/gas_with_call_nonpayable.sol

@@ -1,16 +1,27 @@
 contract C {
 	function f(address a) external view returns (bool success) {
 		(success,) = a.call.gas(42)("");
+		(success,) = a.call{gas: 42}("");
 	}
 	function g(address a) external view returns (bool success) {
 		(success,) = a.call.gas(42)("");
+		(success,) = a.call{gas: 42}("");
 	}
 	function h() external payable {}
 	function i() external view {
 		this.h.gas(42)();
 	}
+	function j() external view {
+		this.h{gas: 42}();
+	}
 }
 // ----
+// Warning: (90-100): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (226-236): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (351-361): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
 // TypeError: (90-108): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
-// TypeError: (190-208): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
-// TypeError: (279-295): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (125-144): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (226-244): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (261-280): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (351-367): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (404-421): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.

test/libsolidity/syntaxTests/viewPureChecker/staticcall_gas_view.sol

@@ -3,9 +3,13 @@ contract C {
 	function test(address a) external view returns (bool status) {
 		// This used to incorrectly raise an error about violating the view mutability.
 		(status,) = a.staticcall.gas(42)("");
+		(status,) = a.staticcall{gas: 42}("");
 		this.f.gas(42)();
+		this.f{gas: 42}();
 	}
 }
 // ====
 // EVMVersion: >=byzantium
 // ----
+// Warning: (207-223): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.
+// Warning: (276-286): Using ".gas(...)" is deprecated. Use "{gas: ...}" instead.

test/libsolidity/syntaxTests/viewPureChecker/value_with_call_nonpayable.sol

@@ -1,16 +1,25 @@
 contract C {
 	function f(address a) external view returns (bool success) {
 		(success,) = a.call.value(42)("");
+		(success,) = a.call{value: 42}("");
 	}
 	function g(address a) external view returns (bool success) {
 		(success,) = a.call.value(42)("");
+		(success,) = a.call{value: 42}("");
 	}
 	function h() external payable {}
 	function i() external view {
 		this.h.value(42)();
+		this.h{value: 42}();
 	}
 }
 // ----
+// Warning: (90-102): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
+// Warning: (230-242): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
+// Warning: (359-371): Using ".value(...)" is deprecated. Use "{value: ...}" instead.
 // TypeError: (90-110): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
-// TypeError: (192-212): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
-// TypeError: (283-301): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (127-148): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (230-250): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (267-288): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (359-377): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
+// TypeError: (381-400): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.