Repository: protofire/solhint
Pull request #468 (2023-08-03 21:14:53)

New Rule: func-named-parameters
requested in 
https://github.com/protofire/solhint/issues/360



Patch:
diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
index 0f6669a..6ceced1 100644
--- a/conf/rulesets/solhint-all.js
+++ b/conf/rulesets/solhint-all.js
@@ -6,6 +6,7 @@
 module.exports = Object.freeze({
   rules: {
     'code-complexity': ['warn', 7],
+    'explicit-types': ['warn', 'explicit'],
     'function-max-lines': ['warn', 50],
     'max-line-length': ['error', 120],
     'max-states-count': ['warn', 15],
@@ -28,7 +29,14 @@ module.exports = Object.freeze({
     'contract-name-camelcase': 'warn',
     'event-name-camelcase': 'warn',
     'func-name-mixedcase': 'warn',
+    'func-named-parameters': ['warn', 2],
     'func-param-name-mixedcase': 'warn',
+    'immutable-vars-naming': [
+      'warn',
+      {
+        immutablesAsConstants: true,
+      },
+    ],
     'modifier-name-mixedcase': 'warn',
     'named-parameters-mapping': 'off',
     'private-vars-leading-underscore': [
diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js
index 489007f..dbd05e7 100644
--- a/conf/rulesets/solhint-recommended.js
+++ b/conf/rulesets/solhint-recommended.js
@@ -5,6 +5,7 @@
 
 module.exports = Object.freeze({
   rules: {
+    'explicit-types': ['warn', 'explicit'],
     'max-states-count': ['warn', 15],
     'no-console': 'error',
     'no-empty-blocks': 'warn',
@@ -23,6 +24,13 @@ module.exports = Object.freeze({
     'contract-name-camelcase': 'warn',
     'event-name-camelcase': 'warn',
     'func-name-mixedcase': 'warn',
+    'func-named-parameters': ['warn', 2],
+    'immutable-vars-naming': [
+      'warn',
+      {
+        immutablesAsConstants: true,
+      },
+    ],
     'use-forbidden-name': 'warn',
     'var-name-mixedcase': 'warn',
     'imports-on-top': 'warn',
diff --git a/docs/rules.md b/docs/rules.md
index 5c1ce26..1a7d2c2 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -6,20 +6,21 @@ title:       "Rule Index of Solhint"
 
 ## Best Practise Rules
 
-| Rule Id                                                            | Error                                                                                                                 | Recommended |
-| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ----------- |
-| [code-complexity](./rules/best-practises/code-complexity.md)       | Function has cyclomatic complexity "current" but allowed no more than maxcompl.                                       |             |
-| [function-max-lines](./rules/best-practises/function-max-lines.md) | Function body contains "count" lines but allowed no more than maxlines.                                               |             |
-| [max-line-length](./rules/best-practises/max-line-length.md)       | Line length must be no more than maxlen.                                                                              |             |
-| [max-states-count](./rules/best-practises/max-states-count.md)     | Contract has "some count" states declarations but allowed no more than maxstates.                                     | ✔️          |
-| [no-console](./rules/best-practises/no-console.md)                 | No console.log/logInt/logBytesX/logString/etc & No hardhat and forge-std console.sol import statements                | ✔️          |
-| [no-empty-blocks](./rules/best-practises/no-empty-blocks.md)       | Code contains empty block.                                                                                            | ✔️          |
-| [no-global-import](./rules/best-practises/no-global-import.md)     | Import statement includes an entire file instead of selected symbols                                                  | ✔️          |
-| [no-unused-import](./rules/best-practises/no-unused-import.md)     | Imported name is not used                                                                                             | ✔️          |
-| [no-unused-vars](./rules/best-practises/no-unused-vars.md)         | Variable "name" is unused.                                                                                            | ✔️          |
-| [payable-fallback](./rules/best-practises/payable-fallback.md)     | When fallback is not payable you will not be able to receive ethers.                                                  | ✔️          |
-| [reason-string](./rules/best-practises/reason-string.md)           | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | ✔️          |
-| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword.                                                                  |             |
+| Rule Id                                                            | Error                                                                                                                 | Recommended  | Deprecated |
+| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
+| [code-complexity](./rules/best-practises/code-complexity.md)       | Function has cyclomatic complexity "current" but allowed no more than maxcompl.                                       |              |            |
+| [explicit-types](./rules/best-practises/explicit-types.md)         | Forbid or enforce explicit types (like uint256) that have an alias (like uint).                                       | $~~~~~~~~$✔️ |            |
+| [function-max-lines](./rules/best-practises/function-max-lines.md) | Function body contains "count" lines but allowed no more than maxlines.                                               |              |            |
+| [max-line-length](./rules/best-practises/max-line-length.md)       | Line length must be no more than maxlen.                                                                              |              |            |
+| [max-states-count](./rules/best-practises/max-states-count.md)     | Contract has "some count" states declarations but allowed no more than maxstates.                                     | $~~~~~~~~$✔️ |            |
+| [no-console](./rules/best-practises/no-console.md)                 | No console.log/logInt/logBytesX/logString/etc & No hardhat and forge-std console.sol import statements.               | $~~~~~~~~$✔️ |            |
+| [no-empty-blocks](./rules/best-practises/no-empty-blocks.md)       | Code block has zero statements inside. Exceptions apply.                                                              | $~~~~~~~~$✔️ |            |
+| [no-global-import](./rules/best-practises/no-global-import.md)     | Import statement includes an entire file instead of selected symbols.                                                 | $~~~~~~~~$✔️ |            |
+| [no-unused-import](./rules/best-practises/no-unused-import.md)     | Imported name is not used.                                                                                            | $~~~~~~~~$✔️ |            |
+| [no-unused-vars](./rules/best-practises/no-unused-vars.md)         | Variable "name" is unused.                                                                                            | $~~~~~~~~$✔️ |            |
+| [payable-fallback](./rules/best-practises/payable-fallback.md)     | When fallback is not payable you will not be able to receive ethers.                                                  | $~~~~~~~~$✔️ |            |
+| [reason-string](./rules/best-practises/reason-string.md)           | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | $~~~~~~~~$✔️ |            |
+| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword.                                                                  |              |            |
         
 
 ## Miscellaneous
@@ -38,6 +39,7 @@ title:       "Rule Index of Solhint"
 | [contract-name-camelcase](./rules/naming/contract-name-camelcase.md)                 | Contract name must be in CamelCase.                                                                     | $~~~~~~~~$✔️ |             |
 | [event-name-camelcase](./rules/naming/event-name-camelcase.md)                       | Event name must be in CamelCase.                                                                        | $~~~~~~~~$✔️ |             |
 | [func-name-mixedcase](./rules/naming/func-name-mixedcase.md)                         | Function name must be in mixedCase.                                                                     | $~~~~~~~~$✔️ |             |
+| [func-named-parameters](./rules/naming/func-named-parameters.md)                     | Enforce function calls with Named Parameters when containing more than the configured qty               | $~~~~~~~~$✔️ |             |
 | [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md)             | Function param name must be in mixedCase.                                                               |              |             |
 | [immutable-vars-naming](./rules/naming/immutable-vars-naming.md)                     | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration.              | $~~~~~~~~$✔️ |             |
 | [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md)                 | Modifier name must be in mixedCase.                                                                     |              |             |
diff --git a/docs/rules/best-practises/no-unused-import.md b/docs/rules/best-practises/no-unused-import.md
index f24eae3..5643b9f 100644
--- a/docs/rules/best-practises/no-unused-import.md
+++ b/docs/rules/best-practises/no-unused-import.md
@@ -14,7 +14,6 @@ title:       "no-unused-import | Solhint"
 ## Description
 Imported name is not used.
 
-
 ## Options
 This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
 
diff --git a/docs/rules/naming/func-named-parameters.md b/docs/rules/naming/func-named-parameters.md
new file mode 100644
index 0000000..3c6710c
--- /dev/null
+++ b/docs/rules/naming/func-named-parameters.md
@@ -0,0 +1,71 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "func-named-parameters | Solhint"
+---
+
+# func-named-parameters
+![Recommended Badge](https://img.shields.io/badge/-Recommended-brightgreen)
+![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
+> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.
+
+
+## Description
+Enforce function calls with Named Parameters when containing more than the configured qty
+
+## Options
+This rule accepts an array of options:
+
+| Index | Description                                            | Default Value |
+| ----- | ------------------------------------------------------ | ------------- |
+| 0     | Rule severity. Must be one of "error", "warn", "off".  | warn          |
+| 1     | Maximum qty of unnamed parameters for a function call. | 2             |
+
+
+### Example Config
+```json
+{
+  "rules": {
+    "func-named-parameters": ["warn",2]
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Function call with two UNNAMED parameters (default is max 2)
+
+```solidity
+functionName('0xA81705c8C247C413a19A244938ae7f4A0393944e', 1e18)
+```
+
+#### Function call with two NAMED parameters
+
+```solidity
+functionName({ sender: '0xA81705c8C247C413a19A244938ae7f4A0393944e', amount: 1e18})
+```
+
+#### Function call with four NAMED parameters
+
+```solidity
+functionName({ sender: _senderAddress, amount: 1e18, token: _tokenAddress, receiver: _receiverAddress })
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Function call with four UNNAMED parameters (default is max 2)
+
+```solidity
+functionName(_senderAddress, 1e18, _tokenAddress, _receiverAddress )
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/func-named-parameters.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/func-named-parameters.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/func-named-parameters.js)
diff --git a/lib/config.js b/lib/config.js
index 697ac89..44b4c42 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -7,7 +7,7 @@ module.exports = {
 
   getNumberByPath(path, defaultValue) {
     const configVal = _.get(this, path)
-    return _.isNumber(configVal) && configVal > 0 ? configVal : defaultValue
+    return _.isNumber(configVal) ? configVal : defaultValue
   },
 
   getBooleanByPath(path, defaultValue) {
diff --git a/lib/rules/best-practises/max-states-count.js b/lib/rules/best-practises/max-states-count.js
index 34f7066..b2ff5cb 100644
--- a/lib/rules/best-practises/max-states-count.js
+++ b/lib/rules/best-practises/max-states-count.js
@@ -49,7 +49,8 @@ class MaxStatesCountChecker extends BaseChecker {
   constructor(reporter, config) {
     super(reporter, ruleId, meta)
 
-    this.maxStatesCount = (config && config.getNumber('max-states-count', 15)) || 15
+    this.maxStatesCount =
+      (config && config.getNumber(ruleId, DEFAULT_MAX_STATES_COUNT)) || DEFAULT_MAX_STATES_COUNT
   }
 
   ContractDefinition(node) {
diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js
new file mode 100644
index 0000000..24d71d1
--- /dev/null
+++ b/lib/rules/naming/func-named-parameters.js
@@ -0,0 +1,78 @@
+const BaseChecker = require('../base-checker')
+const { severityDescription } = require('../../doc/utils')
+
+const DEFAULT_SEVERITY = 'warn'
+const DEFAULT_MAX_UNNAMED_ARGUMENTS = 2
+
+const ruleId = 'func-named-parameters'
+const meta = {
+  type: 'naming',
+
+  docs: {
+    description:
+      'Enforce function calls with Named Parameters when containing more than the configured qty',
+    category: 'Style Guide Rules',
+    options: [
+      {
+        description: severityDescription,
+        default: DEFAULT_SEVERITY,
+      },
+      {
+        description: 'Maximum qty of unnamed parameters for a function call.',
+        default: JSON.stringify(DEFAULT_MAX_UNNAMED_ARGUMENTS),
+      },
+    ],
+    examples: {
+      good: [
+        {
+          description: 'Function call with two UNNAMED parameters (default is max 2)',
+          code: "functionName('0xA81705c8C247C413a19A244938ae7f4A0393944e', 1e18)",
+        },
+        {
+          description: 'Function call with two NAMED parameters',
+          code: "functionName({ sender: '0xA81705c8C247C413a19A244938ae7f4A0393944e', amount: 1e18})",
+        },
+        {
+          description: 'Function call with four NAMED parameters',
+          code: 'functionName({ sender: _senderAddress, amount: 1e18, token: _tokenAddress, receiver: _receiverAddress })',
+        },
+      ],
+      bad: [
+        {
+          description: 'Function call with four UNNAMED parameters (default is max 2)',
+          code: 'functionName(_senderAddress, 1e18, _tokenAddress, _receiverAddress )',
+        },
+      ],
+    },
+  },
+
+  isDefault: false,
+  recommended: true,
+  defaultSetup: [DEFAULT_SEVERITY, DEFAULT_MAX_UNNAMED_ARGUMENTS],
+
+  schema: { type: 'integer' },
+}
+
+class FunctionNamedParametersChecker extends BaseChecker {
+  constructor(reporter, config) {
+    super(reporter, ruleId, meta)
+    this.maxUnnamedArguments = config && config.getNumber(ruleId, DEFAULT_MAX_UNNAMED_ARGUMENTS)
+    if (!(this.maxUnnamedArguments >= 0)) this.maxUnnamedArguments = DEFAULT_MAX_UNNAMED_ARGUMENTS
+  }
+
+  FunctionCall(node) {
+    const qtyNamed = node.names.length
+    const qtyArgs = node.arguments.length
+
+    if (qtyArgs !== 0) {
+      if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) {
+        this.error(
+          node,
+          `Missing named parameters. Max unnamed parameters value is ${this.maxUnnamedArguments}`
+        )
+      }
+    }
+  }
+}
+
+module.exports = FunctionNamedParametersChecker
diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js
index 60db92c..1dffa3f 100644
--- a/lib/rules/naming/index.js
+++ b/lib/rules/naming/index.js
@@ -9,6 +9,7 @@ const UseForbiddenNameChecker = require('./use-forbidden-name')
 const VarNameMixedcaseChecker = require('./var-name-mixedcase')
 const NamedParametersMappingChecker = require('./named-parameters-mapping')
 const ImmutableVarsNamingChecker = require('./immutable-vars-naming')
+const FunctionNamedParametersChecker = require('./func-named-parameters')
 
 module.exports = function checkers(reporter, config) {
   return [
@@ -23,5 +24,6 @@ module.exports = function checkers(reporter, config) {
     new VarNameMixedcaseChecker(reporter),
     new NamedParametersMappingChecker(reporter),
     new ImmutableVarsNamingChecker(reporter, config),
+    new FunctionNamedParametersChecker(reporter, config),
   ]
 }


Test patch:
diff --git a/test/fixtures/naming/func-named-parameters.js b/test/fixtures/naming/func-named-parameters.js
new file mode 100644
index 0000000..d4ceb7a
--- /dev/null
+++ b/test/fixtures/naming/func-named-parameters.js
@@ -0,0 +1,54 @@
+const FUNCTION_CALLS_ERRORS = {
+  maxUnnamed_0_1u: {
+    code: 'funcName(_sender);',
+    maxUnnamed: 0,
+  },
+
+  maxUnnamed_1_3u: {
+    code: 'funcName(_sender, amount, receiver);',
+    maxUnnamed: 1,
+  },
+
+  maxUnnamed_2_3u: {
+    code: 'funcName(_sender, amount, receiver);',
+    maxUnnamed: 2,
+  },
+
+  maxUnnamed_3_4u: {
+    code: 'funcName(_sender, amount, receiver, token);',
+    maxUnnamed: 3,
+  },
+}
+
+const FUNCTION_CALLS_OK = {
+  maxUnnamed_0_0u: {
+    code: 'funcName();',
+    maxUnnamed: 0,
+  },
+
+  maxUnnamed_0_0uB: {
+    code: 'funcName();',
+    maxUnnamed: 10,
+  },
+
+  maxUnnamed_1_0u: {
+    code: 'funcName({ sender: _sender, amount: _amount, receiver: _receiver });',
+    maxUnnamed: 1,
+  },
+
+  maxUnnamed_1_1u: {
+    code: 'funcName(sender);',
+    maxUnnamed: 1,
+  },
+  maxUnnamed_2_2u: {
+    code: 'funcName(sender, amount);',
+    maxUnnamed: 2,
+  },
+
+  maxUnnamed3_0u: {
+    code: 'funcName({ sender: _sender, amount: _amount, receiver: _receiver });',
+    maxUnnamed: 3,
+  },
+}
+
+module.exports = { FUNCTION_CALLS_ERRORS, FUNCTION_CALLS_OK }
diff --git a/test/rules/naming/func-named-parameters.js b/test/rules/naming/func-named-parameters.js
new file mode 100644
index 0000000..9fe9056
--- /dev/null
+++ b/test/rules/naming/func-named-parameters.js
@@ -0,0 +1,85 @@
+const linter = require('../../../lib/index')
+const contractWith = require('../../common/contract-builder').contractWith
+const {
+  assertErrorCount,
+  assertNoErrors,
+  assertErrorMessage,
+  assertWarnsCount,
+} = require('../../common/asserts')
+const {
+  FUNCTION_CALLS_ERRORS,
+  FUNCTION_CALLS_OK,
+} = require('../../fixtures/naming/func-named-parameters')
+
+describe('Linter - func-named-parameters', () => {
+  for (const key in FUNCTION_CALLS_ERRORS) {
+    it(`should raise error for FUNCTION_CALLS_ERRORS [${key}]`, () => {
+      const { code, maxUnnamed } = FUNCTION_CALLS_ERRORS[key]
+
+      const sourceCode = contractWith('function callerFunction() public { ' + code + ' }')
+
+      const report = linter.processStr(sourceCode, {
+        rules: { 'func-named-parameters': ['error', maxUnnamed] },
+      })
+
+      assertErrorCount(report, 1)
+      assertErrorMessage(
+        report,
+        `Missing named parameters. Max unnamed parameters value is ${maxUnnamed}`
+      )
+    })
+  }
+
+  for (const key in FUNCTION_CALLS_OK) {
+    it(`should NOT raise error for FUNCTION_CALLS_OK [${key}]`, () => {
+      const { code, maxUnnamed } = FUNCTION_CALLS_OK[key]
+
+      const sourceCode = contractWith('function callerFunction() public { ' + code + ' }')
+
+      const report = linter.processStr(sourceCode, {
+        rules: { 'func-named-parameters': ['error', maxUnnamed] },
+      })
+
+      assertNoErrors(report)
+    })
+  }
+
+  it('should NOT raise error when default rules are configured', () => {
+    const code = contractWith(
+      `function callerFunction() public { funcName(sender, amount, receiver); }`
+    )
+    const report = linter.processStr(code, {
+      extends: 'solhint:default',
+      rules: {},
+    })
+
+    assertNoErrors(report)
+  })
+
+  it('should raise error when recommended rules are configured', () => {
+    const code = contractWith(
+      `function callerFunction() public { funcName(sender, amount, receiver); }`
+    )
+    const report = linter.processStr(code, {
+      extends: 'solhint:recommended',
+      rules: { 'compiler-version': 'off' },
+    })
+
+    assertWarnsCount(report, 1)
+    assertErrorMessage(report, `Missing named parameters. Max unnamed parameters value is 2`)
+  })
+
+  it('should raise error when rule has no maxUnnamed arguments is set and default value takes over', () => {
+    const code = contractWith(
+      `function callerFunction() public { funcName(sender, amount, receiver); }`
+    )
+    const report = linter.processStr(code, {
+      rules: { 'func-named-parameters': 'error' },
+    })
+
+    assertErrorCount(report, 1)
+
+    // default value is 2 for this rule
+    assertErrorMessage(report, `Missing named parameters. Max unnamed parameters value is 2`)
+  })
+})