Repository: protofire/solhint
Pull request #403 (2023-02-16 15:34:34)

[ newRule ] named-parameters-mapping
This rules solves the issue
https://github.com/protofire/solhint/issues/397

"
Solidity v0.8.18 was [released](https://blog.soliditylang.org/2023/02/01/solidity-0.8.18-release-announcement/) earlier this week, and one of the newly introduced features are [named parameters in mappings](https://twitter.com/PaulRBerg/status/1621949932608360453).  "


Patch:
diff --git a/docs/rules.md b/docs/rules.md
index e59f2a0..65aacdf 100644
--- a/docs/rules.md
+++ b/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.                                                    | ✔️          |
diff --git a/docs/rules/naming/named-parameters-mapping.md b/docs/rules/naming/named-parameters-mapping.md
new file mode 100644
index 0000000..37c3b1a
--- /dev/null
+++ b/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)
diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js
index 5d7d2ab..ad0a9e5 100644
--- a/lib/rules/naming/index.js
+++ b/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),
   ]
 }
diff --git a/lib/rules/naming/named-parameters-mapping.js b/lib/rules/naming/named-parameters-mapping.js
new file mode 100644
index 0000000..7d42ec9
--- /dev/null
+++ b/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
diff --git a/package-lock.json b/package-lock.json
index 2a907fe..1e4df0b 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -9,7 +9,7 @@
       "version": "3.3.8",
       "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",
@@ -681,9 +681,9 @@
       }
     },
     "node_modules/@solidity-parser/parser": {
-      "version": "0.14.5",
-      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.14.5.tgz",
-      "integrity": "sha512-6dKnHZn7fg/iQATVEzqyUOyEidbn05q7YA2mQ9hC0MMXhhV3/JrsxmFSYZAcr7j1yUP700LLhTruvJ3MiQmjJg==",
+      "version": "0.15.0",
+      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.15.0.tgz",
+      "integrity": "sha512-5UFJJTzWi1hgFk6aGCZ5rxG2DJkCJOzJ74qg7UkWSNCDSigW+CJLoYUb5bLiKrtI34Nr9rpFSUNHfkqtlL+N/w==",
       "dependencies": {
         "antlr4ts": "^0.5.0-alpha.4"
       }
@@ -5596,9 +5596,9 @@
       }
     },
     "@solidity-parser/parser": {
-      "version": "0.14.5",
-      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.14.5.tgz",
-      "integrity": "sha512-6dKnHZn7fg/iQATVEzqyUOyEidbn05q7YA2mQ9hC0MMXhhV3/JrsxmFSYZAcr7j1yUP700LLhTruvJ3MiQmjJg==",
+      "version": "0.15.0",
+      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.15.0.tgz",
+      "integrity": "sha512-5UFJJTzWi1hgFk6aGCZ5rxG2DJkCJOzJ74qg7UkWSNCDSigW+CJLoYUb5bLiKrtI34Nr9rpFSUNHfkqtlL+N/w==",
       "requires": {
         "antlr4ts": "^0.5.0-alpha.4"
       }
diff --git a/package.json b/package.json
index e769870..cff6ac2 100644
--- a/package.json
+++ b/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",


Test patch:
diff --git a/test/fixtures/naming/named-parameters-mapping.js b/test/fixtures/naming/named-parameters-mapping.js
new file mode 100644
index 0000000..f5cee36
--- /dev/null
+++ b/test/fixtures/naming/named-parameters-mapping.js
@@ -0,0 +1,191 @@
+const NAMED_MAPPING_REGULAR = 'mapping(string ownerName => address owner) public ownerAddresses;'
+
+const NAMED_MAPPING_NESTED =
+  'mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances;'
+
+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',
+  },
+]
+
+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,
+    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,
+    error_value: false,
+    mapping_name: 'tokenBalances',
+  },
+]
+
+const OTHER_WRONG_DECLARATIONS = [
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    mapping(address => ThisIsStruct) public ownerStuff;`,
+    error_mainKey: true,
+    error_nestedKey: false,
+    error_value: true,
+    mapping_name: 'ownerStuff',
+  },
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    mapping(address owner => ThisIsStruct) public ownerStuff;`,
+    error_mainKey: false,
+    error_nestedKey: false,
+    error_value: true,
+    mapping_name: 'ownerStuff',
+  },
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    mapping(address owner => mapping(address token => ThisIsStruct)) public ownerStuffPerToken;`,
+    error_mainKey: false,
+    error_nestedKey: false,
+    error_value: true,
+    mapping_name: 'ownerStuffPerToken',
+  },
+  {
+    code: `
+    uint256 public A;
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    mapping(address => mapping(address => ThisIsStruct)) public ownerStuffPerToken;`,
+    error_mainKey: true,
+    error_nestedKey: true,
+    error_value: true,
+    mapping_name: 'ownerStuffPerToken',
+  },
+]
+
+const OTHER_OK_DECLARATIONS = [
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    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,
+  },
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }`,
+    error_mainKey: false,
+    error_nestedKey: false,
+    error_value: false,
+  },
+  {
+    code: `
+    struct ThisIsStruct {
+      uint256 A;
+      uint256 B;
+    }
+    mapping(address owner => mapping(address token => ThisIsStruct structContent)) public ownerStuffPerToken;`,
+    error_mainKey: false,
+    error_nestedKey: false,
+    error_value: false,
+    mapping_name: 'ownerStuffPerToken',
+  },
+]
+
+module.exports = {
+  NAMED_MAPPING_REGULAR,
+  NO_NAMED_MAPPING_REGULAR,
+  NAMED_MAPPING_NESTED,
+  NO_NAMED_MAPPING_NESTED,
+  OTHER_OK_DECLARATIONS,
+  OTHER_WRONG_DECLARATIONS,
+}
diff --git a/test/rules/naming/named-parameters-mapping.js b/test/rules/naming/named-parameters-mapping.js
new file mode 100644
index 0000000..4cee6b3
--- /dev/null
+++ b/test/rules/naming/named-parameters-mapping.js
@@ -0,0 +1,140 @@
+const assert = require('assert')
+const { assertWarnsCount } = require('../../common/asserts')
+const linter = require('../../../lib/index')
+const contractWith = require('../../common/contract-builder').contractWith
+const {
+  NAMED_MAPPING_REGULAR,
+  NAMED_MAPPING_NESTED,
+  OTHER_OK_DECLARATIONS,
+  NO_NAMED_MAPPING_REGULAR,
+  NO_NAMED_MAPPING_NESTED,
+  OTHER_WRONG_DECLARATIONS,
+} = require('../../fixtures/naming/named-parameters-mapping')
+
+const WRONG_DECLARATIONS = NO_NAMED_MAPPING_REGULAR.concat(
+  NO_NAMED_MAPPING_NESTED,
+  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
+}
+
+describe('Linter - Named parameters for mapping', () => {
+  it('should not raise an error if all parameters are named correctly for regular mapping', () => {
+    const code = contractWith(NAMED_MAPPING_REGULAR)
+    const report = linter.processStr(code, {
+      rules: { 'named-parameters-mapping': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('should not raise an error if all parameters are named correctly for nested mapping', () => {
+    const code = contractWith(NAMED_MAPPING_NESTED)
+    const report = linter.processStr(code, {
+      rules: { 'named-parameters-mapping': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  OTHER_OK_DECLARATIONS.forEach((objectCode) =>
+    it('should not raise an error if all parameters are named correctly for correct declarations', () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'error' },
+      })
+
+      assert.equal(report.errorCount, 0)
+    })
+  )
+
+  WRONG_DECLARATIONS.forEach((objectCode) => {
+    it(`should raise an error if all parameters are not named for nested mappings`, () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'error' },
+      })
+
+      const errorPositions = getPositionErrors(objectCode)
+      const qtyErrors = errorPositions.length
+
+      assert.equal(report.errorCount, qtyErrors)
+
+      for (let i = 0; i < errorPositions.length; i++) {
+        assert.equal(report.reports[i].message, errorPositions[i])
+      }
+    })
+  })
+
+  // WARNING FLAG
+  WRONG_DECLARATIONS.forEach((objectCode) =>
+    it('should raise a warning if all parameters are not named for nested mappings', () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'warn' },
+      })
+
+      const errorPositions = getPositionErrors(objectCode)
+      const qtyErrors = errorPositions.length
+
+      assert.equal(report.errorCount, 0)
+      assertWarnsCount(report, qtyErrors)
+
+      for (let i = 0; i < errorPositions.length; i++) {
+        assert.equal(report.reports[i].message, errorPositions[i])
+      }
+    })
+  )
+
+  // WARNING FLAG
+  OTHER_OK_DECLARATIONS.forEach((objectCode) =>
+    it('should not raise an error if all parameters are named correctly for correct declarations', () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'warn' },
+      })
+
+      assert.equal(report.errorCount, 0)
+      assertWarnsCount(report, 0)
+    })
+  )
+
+  // OFF FLAG
+  WRONG_DECLARATIONS.forEach((objectCode) =>
+    it('should raise a warning if all parameters are not named for nested mappings', () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'off' },
+      })
+
+      assert.equal(report.errorCount, 0)
+      assertWarnsCount(report, 0)
+    })
+  )
+
+  // OFF FLAG
+  OTHER_OK_DECLARATIONS.forEach((objectCode) =>
+    it('should not raise an error if all parameters are named correctly for correct declarations', () => {
+      const code = contractWith(objectCode.code)
+      const report = linter.processStr(code, {
+        rules: { 'named-parameters-mapping': 'off' },
+      })
+
+      assert.equal(report.errorCount, 0)
+      assertWarnsCount(report, 0)
+    })
+  )
+})