Merge pull request #403 from protofire/fix/named-parameters-in-mappings

[ newRule ] named-parameters-mapping

docs/rules.md

@@ -39,6 +39,7 @@ title:       "Rule Index of Solhint"
 | [func-name-mixedcase](./rules/naming/func-name-mixedcase.md)                         | Function name must be in mixedCase.                                                    | ✔️          |
 | [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md)             | Function param name must be in mixedCase                                               |             |
 | [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md)                 | Modifier name must be in mixedCase.                                                    |             |
+| [named-parameters-mapping](./rules/naming/named-parameters-mapping.md)               | Solidity v0.8.18 introduced named parameters on the mappings definition                |             |
 | [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Private and internal names must start with a single underscore.                        |             |
 | [use-forbidden-name](./rules/naming/use-forbidden-name.md)                           | Avoid to use letters 'I', 'l', 'O' as identifiers.                                     | ✔️          |
 | [var-name-mixedcase](./rules/naming/var-name-mixedcase.md)                           | Variable name must be in mixedCase.                                                    | ✔️          |

docs/rules/naming/named-parameters-mapping.md

@@ -0,0 +1,68 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "named-parameters-mapping | Solhint"
+---
+
+# named-parameters-mapping
+![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
+![Default Severity Badge off](https://img.shields.io/badge/Default%20Severity-off-undefined)
+
+## Description
+Solidity v0.8.18 introduced named parameters on the mappings definition
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to off.
+
+### Example Config
+```json
+{
+  "rules": {
+    "named-parameters-mapping": "off"
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### To enter "users" mapping the key called "name" is needed to get the "balance"
+
+```solidity
+mapping(string name => uint256 balance) public users;
+```
+
+#### To enter owner token balance, the main key "owner" enters another mapping which its key is "token" to get its "balance"
+
+```solidity
+mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances;
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### No naming in regular mapping 
+
+```solidity
+mapping(address => uint256)) public tokenBalances;
+```
+
+#### No naming in nested mapping 
+
+```solidity
+mapping(address => mapping(address => uint256)) public tokenBalances;
+```
+
+#### No complete naming in nested mapping. Missing main key and value 
+
+```solidity
+mapping(address => mapping(address token => uint256)) public tokenBalances;
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/named-parameters-mapping.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/named-parameters-mapping.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/named-parameters-mapping.js)

lib/rules/naming/index.js

@@ -7,6 +7,7 @@ const ModifierNameMixedcaseChecker = require('./modifier-name-mixedcase')
 const PrivateVarsLeadingUnderscore = require('./private-vars-leading-underscore')
 const UseForbiddenNameChecker = require('./use-forbidden-name')
 const VarNameMixedcaseChecker = require('./var-name-mixedcase')
+const NamedParametersMapping = require('./named-parameters-mapping')
 
 module.exports = function checkers(reporter, config) {
   return [
@@ -19,5 +20,6 @@ module.exports = function checkers(reporter, config) {
     new PrivateVarsLeadingUnderscore(reporter, config),
     new UseForbiddenNameChecker(reporter),
     new VarNameMixedcaseChecker(reporter),
+    new NamedParametersMapping(reporter),
   ]
 }

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

@@ -0,0 +1,102 @@
+const BaseChecker = require('../base-checker')
+
+const ruleId = 'named-parameters-mapping'
+const meta = {
+  type: 'naming',
+  docs: {
+    description: `Solidity v0.8.18 introduced named parameters on the mappings definition`,
+    category: 'Style Guide Rules',
+    examples: {
+      good: [
+        {
+          description:
+            'To enter "users" mapping the key called "name" is needed to get the "balance"',
+          code: 'mapping(string name => uint256 balance) public users;',
+        },
+        {
+          description:
+            '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;',
+        },
+      ],
+      bad: [
+        {
+          description: 'No naming in regular mapping ',
+          code: 'mapping(address => uint256)) public tokenBalances;',
+        },
+        {
+          description: 'No naming in nested mapping ',
+          code: 'mapping(address => mapping(address => uint256)) public tokenBalances;',
+        },
+        {
+          description: 'No complete naming in nested mapping. Missing main key and value ',
+          code: 'mapping(address => mapping(address token => uint256)) public tokenBalances;',
+        },
+      ],
+    },
+  },
+  isDefault: false,
+  recommended: false,
+  defaultSetup: 'off',
+  schema: null,
+}
+
+class NamedParametersMapping extends BaseChecker {
+  constructor(reporter) {
+    super(reporter, ruleId, meta)
+  }
+
+  StateVariableDeclaration(node) {
+    let isNested = false
+    const variables = node.variables
+    variables.forEach((variable) => {
+      // maybe the comparission to VariableDeclaration can be deleted
+      if (variable.type === 'VariableDeclaration' && variable.typeName.type === 'Mapping') {
+        if (variable.typeName.valueType.type === 'Mapping') {
+          // isNested mapping
+          isNested = true
+        }
+        this.checkNameOnMapping(variable, isNested)
+      }
+    })
+  }
+
+  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) {
+      this.report(variable, 'Value')
+    }
+  }
+
+  report(variable, type) {
+    const message = `${type} parameter in mapping ${variable.name} is not named`
+    this.reporter.error(variable, this.ruleId, message)
+  }
+}
+
+module.exports = NamedParametersMapping

package.json

@@ -34,7 +34,7 @@
   "author": "Ilya Drabenia <ilya.drobenya@gmail.com>",
   "license": "MIT",
   "dependencies": {
-    "@solidity-parser/parser": "^0.14.5",
+    "@solidity-parser/parser": "^0.15.0",
     "ajv": "^6.12.6",
     "antlr4": "^4.11.0",
     "ast-parents": "^0.0.1",