Merge pull request #13384 from zemse/develop

Allow named parameters in mapping types

Changelog.md

@@ -1,6 +1,7 @@
 ### 0.8.18 (unreleased)
 
 Language Features:
+* Allow named parameters in mapping types.
 
 
 Compiler Features:

docs/grammar/SolidityParser.g4

@@ -504,7 +504,7 @@ variableDeclarationTuple:
 variableDeclarationStatement: ((variableDeclaration (Assign expression)?) | (variableDeclarationTuple Assign expression)) Semicolon;
 expressionStatement: expression Semicolon;
 
-mappingType: Mapping LParen key=mappingKeyType DoubleArrow value=typeName RParen;
+mappingType: Mapping LParen key=mappingKeyType name=identifier? DoubleArrow value=typeName name=identifier? RParen;
 /**
  * Only elementary types or user defined types are viable as mapping keys.
  */

docs/types/mapping-types.rst

@@ -4,12 +4,13 @@
 Mapping Types
 =============
 
-Mapping types use the syntax ``mapping(KeyType => ValueType)`` and variables
-of mapping type are declared using the syntax ``mapping(KeyType => ValueType) VariableName``.
-The ``KeyType`` can be any
-built-in value type, ``bytes``, ``string``, or any contract or enum type. Other user-defined
-or complex types, such as mappings, structs or array types are not allowed.
-``ValueType`` can be any type, including mappings, arrays and structs.
+Mapping types use the syntax ``mapping(KeyType KeyName? => ValueType ValueName?)`` and variables of
+mapping type are declared using the syntax ``mapping(KeyType KeyName? => ValueType ValueName?)
+VariableName``. The ``KeyType`` can be any built-in value type, ``bytes``, ``string``, or any
+contract or enum type. Other user-defined or complex types, such as mappings, structs or array types
+are not allowed. ``ValueType`` can be any type, including mappings, arrays and structs. ``KeyName``
+and ``ValueName`` are optional (so ``mapping(KeyType => ValueType)`` works as well) and can be any
+valid identifier that is not a type.
 
 You can think of mappings as `hash tables <https://en.wikipedia.org/wiki/Hash_table>`_, which are virtually initialised
 such that every possible key exists and is mapped to a value whose
@@ -29,8 +30,10 @@ of contract functions that are publicly visible.
 These restrictions are also true for arrays and structs that contain mappings.
 
 You can mark state variables of mapping type as ``public`` and Solidity creates a
-:ref:`getter <visibility-and-getters>` for you. The ``KeyType`` becomes a parameter for the getter.
-If ``ValueType`` is a value type or a struct, the getter returns ``ValueType``.
+:ref:`getter <visibility-and-getters>` for you. The ``KeyType`` becomes a parameter
+with name ``KeyName`` (if specified) for the getter.
+If ``ValueType`` is a value type or a struct, the getter returns ``ValueType`` with
+name ``ValueName`` (if specified).
 If ``ValueType`` is an array or a mapping, the getter has one parameter for
 each ``KeyType``, recursively.
 
@@ -64,6 +67,25 @@ contract that returns the value at the specified address.
 The example below is a simplified version of an
 `ERC20 token <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol>`_.
 ``_allowances`` is an example of a mapping type inside another mapping type.
+
+In the example below, the optional ``KeyName`` and ``ValueName`` are provided for the mapping.
+It does not affect any contract functionality or bytecode, it only sets the ``name`` field
+for the inputs and outputs in the ABI for the mapping's getter.
+
+.. code-block:: solidity
+
+    // SPDX-License-Identifier: GPL-3.0
+    pragma solidity ^0.8.18;
+
+    contract MappingExampleWithNames {
+        mapping(address user => uint balance) public balances;
+
+        function update(uint newBalance) public {
+            balances[msg.sender] = newBalance;
+        }
+    }
+
+
 The example below uses ``_allowances`` to record the amount someone else is allowed to withdraw from your account.
 
 .. code-block:: solidity

libsolidity/analysis/DeclarationTypeChecker.cpp

@@ -267,14 +267,59 @@ void DeclarationTypeChecker::endVisit(Mapping const& _mapping)
 		solAssert(dynamic_cast<ElementaryTypeName const*>(&_mapping.keyType()), "");
 
 	Type const* keyType = _mapping.keyType().annotation().type;
+	ASTString keyName = _mapping.keyName();
+
 	Type const* valueType = _mapping.valueType().annotation().type;
+	ASTString valueName = _mapping.valueName();
 
 	// Convert key type to memory.
 	keyType = TypeProvider::withLocationIfReference(DataLocation::Memory, keyType);
 
 	// Convert value type to storage reference.
 	valueType = TypeProvider::withLocationIfReference(DataLocation::Storage, valueType);
-	_mapping.annotation().type = TypeProvider::mapping(keyType, valueType);
+	_mapping.annotation().type = TypeProvider::mapping(keyType, keyName, valueType, valueName);
+
+	// Check if parameter names are conflicting.
+	if (!keyName.empty())
+	{
+		auto childMappingType = dynamic_cast<MappingType const*>(valueType);
+		ASTString currentValueName = valueName;
+		bool loop = true;
+		while (loop)
+		{
+			bool isError = false;
+			// Value type is a mapping.
+			if (childMappingType)
+			{
+				// Compare top mapping's key name with child mapping's key name.
+				ASTString childKeyName = childMappingType->keyName();
+				if (keyName == childKeyName)
+					isError = true;
+
+				auto valueType = childMappingType->valueType();
+				currentValueName = childMappingType->valueName();
+				childMappingType = dynamic_cast<MappingType const*>(valueType);
+			}
+			else
+			{
+				// Compare top mapping's key name with the value name.
+				if (keyName == currentValueName)
+					isError = true;
+
+				loop = false; // We arrived at the end of mapping recursion.
+			}
+
+			// Report error.
+			if (isError)
+			{
+				m_errorReporter.declarationError(
+					1809_error,
+					_mapping.location(),
+					"Conflicting parameter name \"" + keyName + "\" in mapping."
+				);
+			}
+		}
+	}
 }
 
 void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName)

libsolidity/ast/AST.h

@@ -1427,18 +1427,29 @@ class Mapping: public TypeName
 		int64_t _id,
 		SourceLocation const& _location,
 		ASTPointer<TypeName> _keyType,
-		ASTPointer<TypeName> _valueType
+		ASTPointer<ASTString> _keyName,
+		ASTPointer<TypeName> _valueType,
+		ASTPointer<ASTString> _valueName
 	):
-		TypeName(_id, _location), m_keyType(std::move(_keyType)), m_valueType(std::move(_valueType)) {}
+		TypeName(_id, _location),
+		m_keyType(std::move(_keyType)),
+		m_keyName(std::move(_keyName)),
+		m_valueType(std::move(_valueType)),
+		m_valueName(std::move(_valueName))
+	{}
 	void accept(ASTVisitor& _visitor) override;
 	void accept(ASTConstVisitor& _visitor) const override;
 
 	TypeName const& keyType() const { return *m_keyType; }
+	ASTString keyName() const { return *m_keyName; }
 	TypeName const& valueType() const { return *m_valueType; }
+	ASTString valueName() const { return *m_valueName; }
 
 private:
 	ASTPointer<TypeName> m_keyType;
+	ASTPointer<ASTString> m_keyName;
 	ASTPointer<TypeName> m_valueType;
+	ASTPointer<ASTString> m_valueName;
 };
 
 /**

libsolidity/ast/ASTJsonExporter.cpp

@@ -598,7 +598,9 @@ bool ASTJsonExporter::visit(Mapping const& _node)
 {
 	setJsonNode(_node, "Mapping", {
 		make_pair("keyType", toJson(_node.keyType())),
+		make_pair("keyName", _node.keyName()),
 		make_pair("valueType", toJson(_node.valueType())),
+		make_pair("valueName", _node.valueName()),
 		make_pair("typeDescriptions", typePointerToJson(_node.annotation().type, true))
 	});
 	return false;

libsolidity/ast/ASTJsonImporter.cpp

@@ -648,7 +648,9 @@ ASTPointer<Mapping> ASTJsonImporter::createMapping(Json::Value const&  _node)
 	return createASTNode<Mapping>(
 		_node,
 		convertJsonToASTNode<TypeName>(member(_node, "keyType")),
-		convertJsonToASTNode<TypeName>(member(_node, "valueType"))
+		memberAsASTString(_node, "keyName"),
+		convertJsonToASTNode<TypeName>(member(_node, "valueType")),
+		memberAsASTString(_node, "valueName")
 	);
 }
 

libsolidity/ast/TypeProvider.cpp

@@ -571,9 +571,9 @@ MagicType const* TypeProvider::meta(Type const* _type)
 	return createAndGet<MagicType>(_type);
 }
 
-MappingType const* TypeProvider::mapping(Type const* _keyType, Type const* _valueType)
+MappingType const* TypeProvider::mapping(Type const* _keyType, ASTString _keyName, Type const* _valueType, ASTString _valueName)
 {
-	return createAndGet<MappingType>(_keyType, _valueType);
+	return createAndGet<MappingType>(_keyType, _keyName, _valueType, _valueName);
 }
 
 UserDefinedValueType const* TypeProvider::userDefinedValueType(UserDefinedValueTypeDefinition const& _definition)

libsolidity/ast/TypeProvider.h

@@ -195,7 +195,7 @@ class TypeProvider
 
 	static MagicType const* meta(Type const* _type);
 
-	static MappingType const* mapping(Type const* _keyType, Type const* _valueType);
+	static MappingType const* mapping(Type const* _keyType, ASTString _keyName, Type const* _valueType, ASTString _valueName);
 
 	static UserDefinedValueType const* userDefinedValueType(UserDefinedValueTypeDefinition const& _definition);
 

libsolidity/ast/Types.cpp

@@ -2788,14 +2788,16 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 	m_declaration(&_varDecl)
 {
 	auto returnType = _varDecl.annotation().type;
+	ASTString returnName;
 
 	while (true)
 	{
 		if (auto mappingType = dynamic_cast<MappingType const*>(returnType))
 		{
 			m_parameterTypes.push_back(mappingType->keyType());
-			m_parameterNames.emplace_back("");
+			m_parameterNames.push_back(mappingType->keyName());
 			returnType = mappingType->valueType();
+			returnName = mappingType->valueName();
 		}
 		else if (auto arrayType = dynamic_cast<ArrayType const*>(returnType))
 		{
@@ -2834,7 +2836,7 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 			DataLocation::Memory,
 			returnType
 		));
-		m_returnParameterNames.emplace_back("");
+		m_returnParameterNames.emplace_back(returnName);
 	}
 
 	solAssert(

libsolidity/ast/Types.h

@@ -1510,8 +1510,8 @@ class FunctionType: public Type
 class MappingType: public CompositeType
 {
 public:
-	MappingType(Type const* _keyType, Type const* _valueType):
-		m_keyType(_keyType), m_valueType(_valueType) {}
+	MappingType(Type const* _keyType, ASTString _keyName, Type const* _valueType, ASTString _valueName):
+		m_keyType(_keyType), m_keyName(_keyName), m_valueType(_valueType), m_valueName(_valueName) {}
 
 	Category category() const override { return Category::Mapping; }
 
@@ -1531,14 +1531,18 @@ class MappingType: public CompositeType
 	std::vector<std::tuple<std::string, Type const*>> makeStackItems() const override;
 
 	Type const* keyType() const { return m_keyType; }
+	ASTString keyName() const { return m_keyName; }
 	Type const* valueType() const { return m_valueType; }
+	ASTString valueName() const { return m_valueName; }
 
 protected:
 	std::vector<Type const*> decomposition() const override { return {m_valueType}; }
 
 private:
 	Type const* m_keyType;
+	ASTString m_keyName;
 	Type const* m_valueType;
+	ASTString m_valueName;
 };
 
 /**

libsolidity/parsing/Parser.cpp

@@ -1187,11 +1187,21 @@ ASTPointer<Mapping> Parser::parseMapping()
 	}
 	else
 		fatalParserError(1005_error, "Expected elementary type name or identifier for mapping key type");
+	ASTPointer<ASTString> keyName;
+	if (m_scanner->currentToken() == Token::Identifier)
+		keyName = getLiteralAndAdvance();
+	else
+		keyName = make_shared<ASTString>("");
 	expectToken(Token::DoubleArrow);
 	ASTPointer<TypeName> valueType = parseTypeName();
+	ASTPointer<ASTString> valueName;
+	if (m_scanner->currentToken() == Token::Identifier)
+		valueName = getLiteralAndAdvance();
+	else
+		valueName = make_shared<ASTString>("");
 	nodeFactory.markEndPosition();
 	expectToken(Token::RParen);
-	return nodeFactory.createNode<Mapping>(keyType, valueType);
+	return nodeFactory.createNode<Mapping>(keyType, keyName, valueType, valueName);
 }
 
 ASTPointer<ParameterList> Parser::parseParameterList(

test/libsolidity/ABIJson/mapping.sol

@@ -0,0 +1,77 @@
+contract test {
+    mapping(address owner => mapping(address spender => uint value)) public allowance;
+    mapping(bytes32 => address sender) public commits;
+    mapping(bytes32 => bytes32) public something;
+}
+// ----
+//     :test
+// [
+//   {
+//     "inputs":
+//     [
+//       {
+//         "internalType": "address",
+//         "name": "owner",
+//         "type": "address"
+//       },
+//       {
+//         "internalType": "address",
+//         "name": "spender",
+//         "type": "address"
+//       }
+//     ],
+//     "name": "allowance",
+//     "outputs":
+//     [
+//       {
+//         "internalType": "uint256",
+//         "name": "value",
+//         "type": "uint256"
+//       }
+//     ],
+//     "stateMutability": "view",
+//     "type": "function"
+//   },
+//   {
+//     "inputs":
+//     [
+//       {
+//         "internalType": "bytes32",
+//         "name": "",
+//         "type": "bytes32"
+//       }
+//     ],
+//     "name": "commits",
+//     "outputs":
+//     [
+//       {
+//         "internalType": "address",
+//         "name": "sender",
+//         "type": "address"
+//       }
+//     ],
+//     "stateMutability": "view",
+//     "type": "function"
+//   },
+//   {
+//     "inputs":
+//     [
+//       {
+//         "internalType": "bytes32",
+//         "name": "",
+//         "type": "bytes32"
+//       }
+//     ],
+//     "name": "something",
+//     "outputs":
+//     [
+//       {
+//         "internalType": "bytes32",
+//         "name": "",
+//         "type": "bytes32"
+//       }
+//     ],
+//     "stateMutability": "view",
+//     "type": "function"
+//   }
+// ]

test/libsolidity/ASTJSON/address_payable.json

@@ -48,6 +48,7 @@
           "typeName":
           {
             "id": 3,
+            "keyName": "",
             "keyType":
             {
               "id": 1,
@@ -67,6 +68,7 @@
               "typeIdentifier": "t_mapping$_t_address_$_t_address_payable_$",
               "typeString": "mapping(address => address payable)"
             },
+            "valueName": "",
             "valueType":
             {
               "id": 2,