Skip to content

Commit

Permalink
fix: gas-named-return-values
Browse files Browse the repository at this point in the history
  • Loading branch information
dbale-altoros committed Feb 29, 2024
1 parent 507f9e9 commit b781694
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 19 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## [4.2.0] - 2024-03-15

### Updated
- Rule: named-return-values rule was renamed to gas-named-return-values and now it is part of Gas Consumption ruleset





## [4.1.1] - 2024-01-08

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ module.exports = Object.freeze({
'gas-increment-by-one': 'warn',
'gas-indexed-events': 'warn',
'gas-multitoken1155': 'warn',
'gas-named-return-values': 'warn',
'gas-small-strings': 'warn',
'gas-struct-packing': 'warn',
'comprehensive-interface': 'warn',
Expand All @@ -48,7 +49,6 @@ module.exports = Object.freeze({
],
'modifier-name-mixedcase': 'warn',
'named-parameters-mapping': 'off',
'named-return-values': 'warn',
'private-vars-leading-underscore': [
'warn',
{
Expand Down
2 changes: 1 addition & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ title: "Rule Index of Solhint"
| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md) | Suggest incrementation by one like this ++i instead of other type | | |
| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | |
| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | |
| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named | | |
| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md) | Keep strings smaller than 32 bytes | | |
| [gas-struct-packing](./rules/gas-consumption/gas-struct-packing.md) | Suggest to re-arrange struct packing order when it is inefficient | | |
Expand All @@ -59,7 +60,6 @@ title: "Rule Index of Solhint"
| [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. | | |
| [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | |
| [named-return-values](./rules/naming/named-return-values.md) | Enforce the return values of a function to be named | | |
| [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | |
| [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. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | |
Expand Down
50 changes: 50 additions & 0 deletions docs/rules/gas-consumption/gas-named-return-values.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
warning: "This is a dynamically generated file. Do not edit manually."
layout: "default"
title: "gas-named-return-values | Solhint"
---

# gas-named-return-values
![Category Badge](https://img.shields.io/badge/-Gas%20Consumption%20Rules-informational)
![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)

## Description
Enforce the return values of a function to be named

## Options
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.

### Example Config
```json
{
"rules": {
"gas-named-return-values": "warn"
}
}
```


## Examples
### 👍 Examples of **correct** code for this rule

#### Function definition with named return values

```solidity
function checkBalance(address wallet) external view returns(uint256 retBalance) {}
```

### 👎 Examples of **incorrect** code for this rule

#### Function definition with UNNAMED return values

```solidity
function checkBalance(address wallet) external view returns(uint256) {}
```

## Version
This rule is introduced in the latest version.

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-named-return-values.js)
- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-named-return-values.md)
- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-named-return-values.js)
50 changes: 50 additions & 0 deletions docs/rules/gas-consumption/named-return-values.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
warning: "This is a dynamically generated file. Do not edit manually."
layout: "default"
title: "named-return-values | Solhint"
---

# named-return-values
![Category Badge](https://img.shields.io/badge/-Gas%20Consumption%20Rules-informational)
![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)

## Description
Enforce the return values of a function to be named

## Options
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.

### Example Config
```json
{
"rules": {
"named-return-values": "warn"
}
}
```


## Examples
### 👍 Examples of **correct** code for this rule

#### Function definition with named return values

```solidity
function checkBalance(address wallet) external view returns(uint256 retBalance) {}
```

### 👎 Examples of **incorrect** code for this rule

#### Function definition with UNNAMED return values

```solidity
function checkBalance(address wallet) external view returns(uint256) {}
```

## Version
This rule is introduced in the latest version.

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-named-return-values.js)
- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-named-return-values.md)
- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-named-return-values.js)
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ const { severityDescription } = require('../../doc/utils')

const DEFAULT_SEVERITY = 'warn'

const ruleId = 'named-return-values'
const ruleId = 'gas-named-return-values'
const meta = {
type: 'naming',
type: 'gas-consumption',

docs: {
description: `Enforce the return values of a function to be named`,
category: 'Style Guide Rules',
category: 'Gas Consumption Rules',
options: [
{
description: severityDescription,
Expand Down Expand Up @@ -39,7 +39,7 @@ const meta = {
schema: null,
}

class NamedReturnValuesChecker extends BaseChecker {
class GasNamedReturnValuesChecker extends BaseChecker {
constructor(reporter) {
super(reporter, ruleId, meta)
}
Expand All @@ -49,12 +49,12 @@ class NamedReturnValuesChecker extends BaseChecker {
let index = 0
for (const returnValue of node.returnParameters) {
if (!returnValue.name) {
this.error(node, `Named return value is missing - Index ${index}`)
this.error(node, `GC: Named return value is missing - Index ${index}`)
}
index++
}
}
}
}

module.exports = NamedReturnValuesChecker
module.exports = GasNamedReturnValuesChecker
2 changes: 2 additions & 0 deletions lib/rules/gas-consumption/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const GasIndexedEvents = require('./gas-indexed-events')
const GasCalldataParameters = require('./gas-calldata-parameters')
const GasIncrementByOne = require('./gas-increment-by-one')
const GasStructPacking = require('./gas-struct-packing')
const GasNamedReturnValuesChecker = require('./gas-named-return-values')

module.exports = function checkers(reporter, config) {
return [
Expand All @@ -13,5 +14,6 @@ module.exports = function checkers(reporter, config) {
new GasCalldataParameters(reporter, config),
new GasIncrementByOne(reporter, config),
new GasStructPacking(reporter, config),
new GasNamedReturnValuesChecker(reporter),
]
}
2 changes: 0 additions & 2 deletions lib/rules/naming/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const NamedParametersMappingChecker = require('./named-parameters-mapping')
const ImmutableVarsNamingChecker = require('./immutable-vars-naming')
const FunctionNamedParametersChecker = require('./func-named-parameters')
const FoundryTestFunctionsChecker = require('./foundry-test-functions')
const NamedReturnValuesChecker = require('./named-return-values')

module.exports = function checkers(reporter, config) {
return [
Expand All @@ -28,6 +27,5 @@ module.exports = function checkers(reporter, config) {
new ImmutableVarsNamingChecker(reporter, config),
new FunctionNamedParametersChecker(reporter, config),
new FoundryTestFunctionsChecker(reporter, config),
new NamedReturnValuesChecker(reporter),
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ const linter = require('../../../lib/index')
const contractWith = require('../../common/contract-builder').contractWith
const { assertErrorCount, assertNoErrors, assertWarnsCount } = require('../../common/asserts')

describe('Linter - named-return-values', () => {
describe('Linter - gas-named-return-values', () => {
it('should NOT raise error for named return values', () => {
const code = contractWith(
`function getBalanceFromTokens(address wallet) public returns(address token1, address token2, uint256 balance1, uint256 balance2) { balance = 1; }`
)
const report = linter.processStr(code, {
rules: { 'named-return-values': 'error' },
rules: { 'gas-named-return-values': 'error' },
})
assertNoErrors(report)
})
Expand All @@ -19,19 +19,22 @@ describe('Linter - named-return-values', () => {
`function getBalanceFromTokens(address wallet) public returns(address, address, uint256, uint256) { balance = 1; }`
)
const report = linter.processStr(code, {
rules: { 'named-return-values': 'error' },
rules: { 'gas-named-return-values': 'error' },
})

assertErrorCount(report, 4)
for (let index = 0; index < report.reports.length; index++) {
assert.equal(report.reports[index].message, `Named return value is missing - Index ${index}`)
assert.equal(
report.reports[index].message,
`GC: Named return value is missing - Index ${index}`
)
}
})

it('should NOT raise error for functions without return values', () => {
const code = contractWith(`function writeOnStorage(address wallet) public { balance = 1; }`)
const report = linter.processStr(code, {
rules: { 'named-return-values': 'error' },
rules: { 'gas-named-return-values': 'error' },
})
assertNoErrors(report)
})
Expand All @@ -41,12 +44,12 @@ describe('Linter - named-return-values', () => {
`function getBalanceFromTokens(address wallet) public returns(address user, address, uint256 amount, uint256) { balance = 1; }`
)
const report = linter.processStr(code, {
rules: { 'named-return-values': 'error' },
rules: { 'gas-named-return-values': 'error' },
})

assertErrorCount(report, 2)
assert.equal(report.reports[0].message, `Named return value is missing - Index 1`)
assert.equal(report.reports[1].message, `Named return value is missing - Index 3`)
assert.equal(report.reports[0].message, `GC: Named return value is missing - Index 1`)
assert.equal(report.reports[1].message, `GC: Named return value is missing - Index 3`)
})

it('should NOT raise error for solhint:recommended setup', () => {
Expand Down Expand Up @@ -86,7 +89,10 @@ describe('Linter - named-return-values', () => {

assertWarnsCount(report, 2)
for (let index = 0; index < report.reports.length; index++) {
assert.equal(report.reports[index].message, `Named return value is missing - Index ${index}`)
assert.equal(
report.reports[index].message,
`GC: Named return value is missing - Index ${index}`
)
}
})
})

0 comments on commit b781694

Please sign in to comment.