diff --git a/docs/rules/naming/named-parameters-mapping.md b/docs/rules/naming/named-parameters-mapping.md index 37c3b1a0..f23c7a8e 100644 --- a/docs/rules/naming/named-parameters-mapping.md +++ b/docs/rules/naming/named-parameters-mapping.md @@ -39,28 +39,52 @@ mapping(string name => uint256 balance) public users; mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances; ``` +#### Main key of mapping is enforced. On nested mappings other naming are not neccesary + +```solidity +mapping(address owner => mapping(address => uint256)) public tokenBalances; +``` + +#### Main key of the parent mapping is enforced. No naming in nested mapping uint256 + +```solidity +mapping(address owner => mapping(address token => uint256)) public tokenBalances; +``` + +#### Main key of the parent mapping is enforced. No naming in nested mapping address + +```solidity +mapping(address owner => mapping(address => uint256 balance)) public tokenBalances; +``` + ### 👎 Examples of **incorrect** code for this rule -#### No naming in regular mapping +#### No naming at all in regular mapping ```solidity mapping(address => uint256)) public tokenBalances; ``` -#### No naming in nested mapping +#### Missing any variable name in regular mapping uint256 + +```solidity +mapping(address token => uint256)) public tokenBalances; +``` + +#### Missing any variable name in regular mapping address ```solidity -mapping(address => mapping(address => uint256)) public tokenBalances; +mapping(address => uint256 balance)) public tokenBalances; ``` -#### No complete naming in nested mapping. Missing main key and value +#### No MAIN KEY naming in nested mapping. Other naming are not enforced ```solidity -mapping(address => mapping(address token => uint256)) public tokenBalances; +mapping(address => mapping(address token => uint256 balance)) public tokenBalances; ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 3.4.0](https://github.com/protofire/solhint/tree/v3.4.0) ## Resources - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/named-parameters-mapping.js) diff --git a/lib/rules/naming/named-parameters-mapping.js b/lib/rules/naming/named-parameters-mapping.js index 7d42ec9f..099e71c2 100644 --- a/lib/rules/naming/named-parameters-mapping.js +++ b/lib/rules/naming/named-parameters-mapping.js @@ -18,19 +18,38 @@ const meta = { 'To enter owner token balance, the main key "owner" enters another mapping which its key is "token" to get its "balance"', code: 'mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances;', }, + { + description: + 'Main key of mapping is enforced. On nested mappings other naming are not neccesary', + code: 'mapping(address owner => mapping(address => uint256)) public tokenBalances;', + }, + { + description: + 'Main key of the parent mapping is enforced. No naming in nested mapping uint256', + code: 'mapping(address owner => mapping(address token => uint256)) public tokenBalances;', + }, + { + description: + 'Main key of the parent mapping is enforced. No naming in nested mapping address', + code: 'mapping(address owner => mapping(address => uint256 balance)) public tokenBalances;', + }, ], bad: [ { - description: 'No naming in regular mapping ', + description: 'No naming at all in regular mapping ', code: 'mapping(address => uint256)) public tokenBalances;', }, { - description: 'No naming in nested mapping ', - code: 'mapping(address => mapping(address => uint256)) public tokenBalances;', + description: 'Missing any variable name in regular mapping uint256', + code: 'mapping(address token => uint256)) public tokenBalances;', + }, + { + description: 'Missing any variable name in regular mapping address', + code: 'mapping(address => uint256 balance)) public tokenBalances;', }, { - description: 'No complete naming in nested mapping. Missing main key and value ', - code: 'mapping(address => mapping(address token => uint256)) public tokenBalances;', + description: 'No MAIN KEY naming in nested mapping. Other naming are not enforced', + code: 'mapping(address => mapping(address token => uint256 balance)) public tokenBalances;', }, ], }, @@ -63,20 +82,15 @@ class NamedParametersMapping extends BaseChecker { checkNameOnMapping(variable, isNested) { let mainKeyName - let nestedKeyName let valueName if (isNested) { mainKeyName = variable.typeName.keyName ? variable.typeName.keyName.name : null - nestedKeyName = variable.typeName.valueType.keyName - ? variable.typeName.valueType.keyName.name - : null valueName = variable.typeName.valueType.valueName ? variable.typeName.valueType.valueName.name : null } else { mainKeyName = variable.typeName.keyName ? variable.typeName.keyName.name : null - nestedKeyName = null valueName = variable.typeName.valueName ? variable.typeName.valueName.name : null } @@ -84,11 +98,7 @@ class NamedParametersMapping extends BaseChecker { this.report(variable, 'Main key') } - if (!nestedKeyName && isNested) { - this.report(variable, 'Nested key') - } - - if (!valueName) { + if (!valueName && !isNested) { this.report(variable, 'Value') } } diff --git a/test/fixtures/naming/named-parameters-mapping.js b/test/fixtures/naming/named-parameters-mapping.js index f5cee366..b2921022 100644 --- a/test/fixtures/naming/named-parameters-mapping.js +++ b/test/fixtures/naming/named-parameters-mapping.js @@ -7,21 +7,18 @@ const NO_NAMED_MAPPING_REGULAR = [ { code: 'mapping(string => address owner) public ownerAddresses;', error_mainKey: true, - error_nestedKey: false, error_value: false, mapping_name: 'ownerAddresses', }, { code: 'mapping(string ownerName => address) public ownerAddresses;', error_mainKey: false, - error_nestedKey: false, error_value: true, mapping_name: 'ownerAddresses', }, { code: 'mapping(string => address) public ownerAddresses;', error_mainKey: true, - error_nestedKey: false, error_value: true, mapping_name: 'ownerAddresses', }, @@ -31,49 +28,24 @@ const NO_NAMED_MAPPING_NESTED = [ { code: 'mapping(address => mapping(address => uint256)) public tokenBalances;', error_mainKey: true, - error_nestedKey: true, - error_value: true, - mapping_name: 'tokenBalances', - }, - { - code: 'mapping(address owner => mapping(address => uint256)) public tokenBalances;', - error_mainKey: false, - error_nestedKey: true, - error_value: true, - mapping_name: 'tokenBalances', - }, - { - code: 'mapping(address owner => mapping(address token => uint256)) public tokenBalances;', - error_mainKey: false, - error_nestedKey: false, - error_value: true, + error_value: false, mapping_name: 'tokenBalances', }, { code: 'mapping(address => mapping(address token => uint256)) public tokenBalances;', error_mainKey: true, - error_nestedKey: false, - error_value: true, - mapping_name: 'tokenBalances', - }, - { - code: 'mapping(address => mapping(address token => uint256 balance)) public tokenBalances;', - error_mainKey: true, - error_nestedKey: false, error_value: false, mapping_name: 'tokenBalances', }, { code: 'mapping(address => mapping(address => uint256 balance)) public tokenBalances;', error_mainKey: true, - error_nestedKey: true, error_value: false, mapping_name: 'tokenBalances', }, { - code: 'mapping(address owner => mapping(address => uint256 balance)) public tokenBalances;', - error_mainKey: false, - error_nestedKey: true, + code: 'mapping(address => mapping(address token => uint256 balance)) public tokenBalances;', + error_mainKey: true, error_value: false, mapping_name: 'tokenBalances', }, @@ -88,7 +60,6 @@ const OTHER_WRONG_DECLARATIONS = [ } mapping(address => ThisIsStruct) public ownerStuff;`, error_mainKey: true, - error_nestedKey: false, error_value: true, mapping_name: 'ownerStuff', }, @@ -100,7 +71,6 @@ const OTHER_WRONG_DECLARATIONS = [ } mapping(address owner => ThisIsStruct) public ownerStuff;`, error_mainKey: false, - error_nestedKey: false, error_value: true, mapping_name: 'ownerStuff', }, @@ -110,10 +80,9 @@ const OTHER_WRONG_DECLARATIONS = [ uint256 A; uint256 B; } - mapping(address owner => mapping(address token => ThisIsStruct)) public ownerStuffPerToken;`, - error_mainKey: false, - error_nestedKey: false, - error_value: true, + mapping(address => mapping(address token => ThisIsStruct)) public ownerStuffPerToken;`, + error_mainKey: true, + error_value: false, mapping_name: 'ownerStuffPerToken', }, { @@ -125,8 +94,7 @@ const OTHER_WRONG_DECLARATIONS = [ } mapping(address => mapping(address => ThisIsStruct)) public ownerStuffPerToken;`, error_mainKey: true, - error_nestedKey: true, - error_value: true, + error_value: false, mapping_name: 'ownerStuffPerToken', }, ] @@ -141,20 +109,17 @@ const OTHER_OK_DECLARATIONS = [ mapping(address owner => ThisIsStruct structContent) public ownerStuff; uint256 public A;`, error_mainKey: false, - error_nestedKey: false, error_value: false, mapping_name: 'ownerStuff', }, { code: 'uint256 public A;', error_mainKey: false, - error_nestedKey: false, error_value: false, }, { code: 'uint256 constant public A = 100000;', error_mainKey: false, - error_nestedKey: false, error_value: false, }, { @@ -164,7 +129,6 @@ const OTHER_OK_DECLARATIONS = [ uint256 B; }`, error_mainKey: false, - error_nestedKey: false, error_value: false, }, { @@ -175,7 +139,28 @@ const OTHER_OK_DECLARATIONS = [ } mapping(address owner => mapping(address token => ThisIsStruct structContent)) public ownerStuffPerToken;`, error_mainKey: false, - error_nestedKey: false, + error_value: false, + mapping_name: 'ownerStuffPerToken', + }, + { + code: ` + struct ThisIsStruct { + uint256 A; + uint256 B; + } + mapping(address owner => mapping(address => ThisIsStruct structContent)) public ownerStuffPerToken;`, + error_mainKey: false, + error_value: false, + mapping_name: 'ownerStuffPerToken', + }, + { + code: ` + struct ThisIsStruct { + uint256 A; + uint256 B; + } + mapping(address owner => mapping(address => ThisIsStruct)) public ownerStuffPerToken;`, + error_mainKey: false, error_value: false, mapping_name: 'ownerStuffPerToken', }, diff --git a/test/rules/naming/named-parameters-mapping.js b/test/rules/naming/named-parameters-mapping.js index 4cee6b3a..656ea188 100644 --- a/test/rules/naming/named-parameters-mapping.js +++ b/test/rules/naming/named-parameters-mapping.js @@ -16,7 +16,6 @@ const WRONG_DECLARATIONS = NO_NAMED_MAPPING_REGULAR.concat( OTHER_WRONG_DECLARATIONS ) const MAIN_KEY_ERROR = 'Main key parameter in mapping XXXXX is not named' -const NESTED_KEY_ERROR = 'Nested key parameter in mapping XXXXX is not named' const VALUE_ERROR = 'Value parameter in mapping XXXXX is not named' const getPositionErrors = (objectCode) => { @@ -24,9 +23,6 @@ const getPositionErrors = (objectCode) => { if (objectCode.error_mainKey) errorArray.push(MAIN_KEY_ERROR.replace('XXXXX', objectCode.mapping_name)) - if (objectCode.error_nestedKey) - errorArray.push(NESTED_KEY_ERROR.replace('XXXXX', objectCode.mapping_name)) - if (objectCode.error_value) errorArray.push(VALUE_ERROR.replace('XXXXX', objectCode.mapping_name)) return errorArray }