Merge pull request #421 from protofire/fix/named-parameters-mapping-no-enforce-nested

Rule Modification: no enforcing parameters on nested mappings

Author: dbale-altoros

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)

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,32 +82,23 @@ 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
     }
 
     if (!mainKeyName) {
       this.report(variable, 'Main key')
     }
 
-    if (!nestedKeyName && isNested) {
-      this.report(variable, 'Nested key')
-    }
-
-    if (!valueName) {
+    if (!valueName && !isNested) {
       this.report(variable, 'Value')
     }
   }

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',
   },

test/rules/naming/named-parameters-mapping.js

@@ -16,17 +16,13 @@ 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) => {
   const errorArray = []
   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
 }