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

fix: named-return-values #552

Merged
merged 2 commits into from
Mar 5, 2024
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: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@

## [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.2] - 2024-02-06

### Updated
Expand All @@ -9,7 +16,6 @@
But overall functionality will work as expected.



## [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}`
)
}
})
})
Loading