Skip to content

Commit

Permalink
Merge branch 'develop' into interface-starts-with-i
Browse files Browse the repository at this point in the history
  • Loading branch information
dbale-altoros authored Mar 11, 2024
2 parents 0bdcb9c + c0c6696 commit 06754b5
Show file tree
Hide file tree
Showing 15 changed files with 471 additions and 36 deletions.
2 changes: 2 additions & 0 deletions conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ module.exports = Object.freeze({
'gas-custom-errors': 'warn',
'gas-increment-by-one': 'warn',
'gas-indexed-events': 'warn',
'gas-length-in-loops': 'warn',
'gas-multitoken1155': 'warn',
'gas-named-return-values': 'warn',
'gas-small-strings': 'warn',
'gas-strict-inequalities': 'warn',
'gas-struct-packing': 'warn',
'comprehensive-interface': 'warn',
quotes: ['error', 'double'],
Expand Down
2 changes: 2 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ title: "Rule Index of Solhint"
| [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-strict-inequalities](./rules/gas-consumption/gas-strict-inequalities.md) | Suggest Strict Inequalities over non Strict ones | | |
| [gas-struct-packing](./rules/gas-consumption/gas-struct-packing.md) | Suggest to re-arrange struct packing order when it is inefficient | | |
| [gas-length-in-loops](./rules/gas-consumption/gas-length-in-loops.md) | Suggest replacing object.length in a loop condition to avoid calculation on each lap | | |

## Miscellaneous
Expand Down
38 changes: 38 additions & 0 deletions docs/rules/gas-consumption/gas-length-in-loops.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
warning: "This is a dynamically generated file. Do not edit manually."
layout: "default"
title: "gas-length-in-loops | Solhint"
---

# gas-length-in-loops
![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
Suggest replacing object.length in a loop condition to avoid calculation on each lap

## 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-length-in-loops": "warn"
}
}
```

### Notes
- [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Array Length Caching)

## Examples
This rule does not have examples.

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

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

# gas-strict-inequalities
![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
Suggest Strict Inequalities over non Strict ones

## 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-strict-inequalities": "warn"
}
}
```

### Notes
- Strict inequality does not always saves gas. It is dependent on the context of the surrounding opcodes
- [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Less/Greater Than vs Less/Greater Than or Equal To)
- [source 2](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-7b77t) of the rule initiative

## Examples
This rule does not have examples.

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

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-strict-inequalities.js)
- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-strict-inequalities.md)
- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-strict-inequalities.js)
24 changes: 12 additions & 12 deletions e2e/autofix-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('e2e', function () {
}
)

expect(solhintProcess.status).to.equal(1)
expect(solhintProcess.status).to.equal(0)
expect(solhintProcess.stdout.toString().includes('5 problems (5 errors, 0 warnings)'))
})
})
Expand All @@ -129,7 +129,7 @@ describe('e2e', function () {
`${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
)

expect(code).to.equal(1)
expect(code).to.equal(0)

const reportLines = stdout.split('\n')
const finalLine = '5 problems (5 errors, 0 warnings)'
Expand Down Expand Up @@ -175,8 +175,8 @@ describe('e2e', function () {
expect(result).to.be.true
})

it('should execute and exit with code 1 (2)', () => {
expect(code).to.equal(1)
it('should execute and exit with code 0 (2)', () => {
expect(code).to.equal(0)
})

it('should get the right report (2)', () => {
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (3)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (3)', () => {
Expand Down Expand Up @@ -268,7 +268,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (4)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (4)', () => {
Expand Down Expand Up @@ -314,7 +314,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (5)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (5)', () => {
Expand Down Expand Up @@ -361,7 +361,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (6)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (6)', () => {
Expand Down Expand Up @@ -401,7 +401,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (6)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (6)', () => {
Expand Down Expand Up @@ -448,7 +448,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (7)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (7)', () => {
Expand Down Expand Up @@ -494,7 +494,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (8)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (8)', () => {
Expand Down Expand Up @@ -540,7 +540,7 @@ describe('e2e', function () {
})

it('should execute and exit with code 1 (8)', () => {
expect(code).to.equal(1)
expect(code).to.equal(0)
})

it('should get the right report (8)', () => {
Expand Down
12 changes: 6 additions & 6 deletions e2e/formatters-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('e2e', function () {
expect(reportLines[i]).to.equal(expectedLine)
}
// because there's an error
expect(code).to.equal(1)
expect(code).to.equal(0)

const finalLine = '10 problem/s (1 error/s, 9 warning/s)'
expect(reportLines[reportLines.length - 2]).to.contain(finalLine)
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('e2e', function () {
const strExpected = JSON.stringify(expectedFinalOutput)
expect(strExpected).to.equal(strOutput)
// There's an error, that is why exit code is 1
expect(code).to.equal(1)
expect(code).to.equal(0)
})
})

Expand Down Expand Up @@ -175,7 +175,7 @@ describe('e2e', function () {
expect(reportLines[i]).to.equal(expectedLine)
}
// because there's an error
expect(code).to.equal(1)
expect(code).to.equal(0)

const finalLine = '10 problem/s (1 error/s, 9 warning/s)'
expect(reportLines[reportLines.length - 2]).to.contain(finalLine)
Expand Down Expand Up @@ -240,7 +240,7 @@ describe('e2e', function () {

expect(reportLines[i].replace(/\s/g, '')).to.equal(expectedLine.replace(/\s/g, ''))
}
expect(code).to.equal(1)
expect(code).to.equal(0)

const finalLine = '\u2716 10 problems (1 error, 9 warnings)'
expect(reportLines[reportLines.length - 3]).to.equal(finalLine)
Expand Down Expand Up @@ -342,7 +342,7 @@ describe('e2e', function () {
expect(reportLines[3]).to.eq(tableHeader1)
expect(reportLines[4]).to.eq(tableHeader2)

expect(code).to.equal(1)
expect(code).to.equal(0)
})
})

Expand Down Expand Up @@ -670,7 +670,7 @@ describe('e2e', function () {
]

// There's an error, that is why exit code is 1
expect(code).to.equal(1)
expect(code).to.equal(0)
expect(sarifOutput.runs[0].artifacts[0].location.uri).to.eq(expectedUriPathFoo)
expect(sarifOutput.runs[0].artifacts[1].location.uri).to.eq(expectedUriPathFoo2)
expect(sarifOutput.runs[0].artifacts[2].location.uri).to.eq(expectedUriPathFoo3)
Expand Down
16 changes: 8 additions & 8 deletions e2e/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ describe('e2e', function () {
describe('no-empty-blocks', function () {
useFixture('03-no-empty-blocks')

it('should exit with 1', function () {
it('should end correctly (exit w/0), found 1 error', function () {
const { code, stdout } = shell.exec('solhint Foo.sol')

expect(code).to.equal(1)
expect(code).to.equal(0)
expect(stdout.trim()).to.contain('Code contains empty blocks')
})

it('should work with stdin', async function () {
it('should work with stdin, exit 0, found 1 error', async function () {
const child = cp.exec('solhint stdin')

const stdoutPromise = getStream(child.stdout)
Expand All @@ -96,7 +96,7 @@ describe('e2e', function () {

const code = await codePromise

expect(code).to.equal(1)
expect(code).to.equal(0)

const stdout = await stdoutPromise

Expand Down Expand Up @@ -127,7 +127,7 @@ describe('e2e', function () {
expect(stdout.trim()).to.not.contain(warningExceededMsg)
})

it('should display [warnings exceeded] for max 3 warnings and exit error 1', function () {
it('should display [warnings exceeded] for max 3 warnings and exit with 0', function () {
const { code, stdout } = shell.exec('solhint contracts/Foo.sol --max-warnings 3')

expect(code).to.equal(1)
Expand All @@ -137,13 +137,13 @@ describe('e2e', function () {
it('should return error for Compiler version rule, ignoring 3 --max-warnings', function () {
const { code, stdout } = shell.exec('solhint contracts/Foo2.sol --max-warnings 3')

expect(code).to.equal(1)
expect(code).to.equal(0)
expect(stdout.trim()).to.contain(errorFound)
})

it('should return error for Compiler version rule. No message for max-warnings', function () {
const { code, stdout } = shell.exec('solhint contracts/Foo2.sol --max-warnings 27')
expect(code).to.equal(1)
expect(code).to.equal(0)
expect(stdout.trim()).to.not.contain(errorFound)
})
})
Expand All @@ -163,7 +163,7 @@ describe('e2e', function () {
it(`should raise error for wrongFunctionDefinitionName() only`, () => {
const { code, stdout } = shell.exec('solhint -c test/.solhint.json test/FooTest.sol')

expect(code).to.equal(1)
expect(code).to.equal(0)
expect(stdout.trim()).to.contain(
'Function wrongFunctionDefinitionName() must match Foundry test naming convention'
)
Expand Down
75 changes: 75 additions & 0 deletions lib/rules/gas-consumption/gas-length-in-loops.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
const BaseChecker = require('../base-checker')

let found
const ruleId = 'gas-length-in-loops'
const meta = {
type: 'gas-consumption',

docs: {
description:
'Suggest replacing object.length in a loop condition to avoid calculation on each lap',
category: 'Gas Consumption Rules',
notes: [
{
note: '[source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Array Length Caching)',
},
],
},

isDefault: false,
recommended: false,
defaultSetup: 'warn',

schema: null,
}

class GasLengthInLoops extends BaseChecker {
constructor(reporter) {
super(reporter, ruleId, meta)
}

checkConditionForMemberAccessLength(node) {
if (found) return // Return early if the condition has already been found
if (typeof node === 'object' && node !== null) {
if (node.type === 'MemberAccess' && node.memberName === 'length') {
found = true // Update the flag if the condition is met
return
}
// Recursively search through all object properties
Object.values(node).forEach((value) => this.checkConditionForMemberAccessLength(value))
}
}

DoWhileStatement(node) {
found = false
this.checkConditionForMemberAccessLength(node.condition)
if (found) {
this.reportError(node)
}
}

WhileStatement(node) {
found = false
this.checkConditionForMemberAccessLength(node.condition)
if (found) {
this.reportError(node)
}
}

ForStatement(node) {
found = false
this.checkConditionForMemberAccessLength(node.conditionExpression)
if (found) {
this.reportError(node)
}
}

reportError(node) {
this.error(
node,
`GC: Found [ .length ] property in Loop condition. Suggestion: assign it to a variable`
)
}
}

module.exports = GasLengthInLoops
Loading

0 comments on commit 06754b5

Please sign in to comment.