Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Rule: func-named-parameters #468

Merged
merged 1 commit into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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': [
Expand Down
8 changes: 8 additions & 0 deletions conf/rulesets/solhint-recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
30 changes: 16 additions & 14 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. | | |
Expand Down
1 change: 0 additions & 1 deletion docs/rules/best-practises/no-unused-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
71 changes: 71 additions & 0 deletions docs/rules/naming/func-named-parameters.md
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 1 addition & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/best-practises/max-states-count.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
78 changes: 78 additions & 0 deletions lib/rules/naming/func-named-parameters.js
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions lib/rules/naming/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand All @@ -23,5 +24,6 @@ module.exports = function checkers(reporter, config) {
new VarNameMixedcaseChecker(reporter),
new NamedParametersMappingChecker(reporter),
new ImmutableVarsNamingChecker(reporter, config),
new FunctionNamedParametersChecker(reporter, config),
]
}
54 changes: 54 additions & 0 deletions test/fixtures/naming/func-named-parameters.js
Original file line number Diff line number Diff line change
@@ -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 }
Loading