Skip to content

Commit

Permalink
Merge pull request #483 from protofire/i230-fix-compiler-version-defa…
Browse files Browse the repository at this point in the history
…ult-rule

Fix and Bump compiler-version default
  • Loading branch information
dbale-altoros authored Aug 11, 2023
2 parents 038e10d + 9228379 commit b67d523
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 40 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ You should add this rule manually:
```
If not explicitly added, this rule will not be executed.

### SPECIAL ATTENTION
- RULE: `compiler-version` default was updated from ^0.5.2 to ^0.8.0

### Added
- New Rule: Enforces the use of Custom Errors over Require and Revert statements [#475](https://github.com/protofire/solhint/pull/475)
- New Rule: Enforces the test_ prefix on a file for Foundry users [#476](https://github.com/protofire/solhint/pull/476)
Expand All @@ -26,6 +29,12 @@ If not explicitly added, this rule will not be executed.
- `func-named-parameters` - false positives on builtin functions [#472](https://github.com/protofire/solhint/pull/472)
- `ordering` - treat initializer weight same as constructor [#474](https://github.com/protofire/solhint/pull/474)
- `check-send-result` - false positive on `erc777.send()`` function [#477](https://github.com/protofire/solhint/pull/477)
- `explicit-types` - default value is now taking into account when no value is specified in config [#481](https://github.com/protofire/solhint/pull/481)
- `compiler-version` - default value is now taking into account when no value is specified in config [#483](https://github.com/protofire/solhint/pull/483)

### Updates
- Rule: `check-send-result` added config clarification in the new `Notes` section [#482](https://github.com/protofire/solhint/pull/482)
- Rule: `compiler-version` default was updated from ^0.5.2 to ^0.8.0 [#483](https://github.com/protofire/solhint/pull/483)



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 @@ -60,7 +60,7 @@ module.exports = Object.freeze({
'avoid-throw': 'warn',
'avoid-tx-origin': 'warn',
'check-send-result': 'warn',
'compiler-version': ['error', '^0.5.8'],
'compiler-version': ['error', '^0.8.0'],
'func-visibility': [
'warn',
{
Expand Down
2 changes: 1 addition & 1 deletion conf/rulesets/solhint-recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module.exports = Object.freeze({
'avoid-throw': 'warn',
'avoid-tx-origin': 'warn',
'check-send-result': 'warn',
'compiler-version': ['error', '^0.5.8'],
'compiler-version': ['error', '^0.8.0'],
'func-visibility': [
'warn',
{
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/security/compiler-version.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ This rule accepts an array of options:
| Index | Description | Default Value |
| ----- | ----------------------------------------------------- | ------------- |
| 0 | Rule severity. Must be one of "error", "warn", "off". | error |
| 1 | Semver requirement | ^0.5.8 |
| 1 | Semver requirement | ^0.8.0 |


### Example Config
```json
{
"rules": {
"compiler-version": ["error","^0.5.8"]
"compiler-version": ["error","^0.8.0"]
}
}
```
Expand Down
2 changes: 1 addition & 1 deletion e2e/05-max-warnings/contracts/Foo.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.5.8;
pragma solidity >=0.8.0;

contract Foo {
uint256 public constant test1 = 1;
Expand Down
2 changes: 1 addition & 1 deletion e2e/06-formatters/contracts/Foo2.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.5.8;
pragma solidity >=0.8.0;

contract Foo {
uint256 public constant test1 = 1;
Expand Down
2 changes: 1 addition & 1 deletion e2e/06-formatters/contracts/Foo3.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.5.8;
pragma solidity >=0.8.0;

contract Foo {
uint256 public constant TEST1 = 1;
Expand Down
2 changes: 1 addition & 1 deletion e2e/06-formatters/helpers/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const foo1Output = [
line: 2,
column: 1,
severity: 'Error',
message: 'Compiler version >=0.6.0 does not satisfy the ^0.5.8 semver requirement',
message: 'Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement',
ruleId: 'compiler-version',
fix: null,
filePath: 'contracts/Foo.sol',
Expand Down
2 changes: 1 addition & 1 deletion e2e/07-foundry-test/contracts/Foo.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.6.0;
pragma solidity >=0.8.0;

contract Foo {
uint256 public constant TEST = 1;
Expand Down
2 changes: 1 addition & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module.exports = {
return this.getBooleanByPath(`rules["${ruleName}"][1][${ruleProperty}]`, defaultValue)
},

getString(ruleName, defaultValue) {
getStringFromArray(ruleName, defaultValue) {
if (this.rules && this.rules[ruleName]) {
const ruleValue = this.rules[ruleName]
return Array.isArray(ruleValue) ? ruleValue[1] : defaultValue
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/best-practises/explicit-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ const meta = {
class ExplicitTypesChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)
this.configOption = (config && config.getString(ruleId, DEFAULT_OPTION)) || DEFAULT_OPTION
this.configOption =
(config && config.getStringFromArray(ruleId, DEFAULT_OPTION)) || DEFAULT_OPTION
this.isExplicit = this.configOption === 'explicit'
}

Expand Down
5 changes: 3 additions & 2 deletions lib/rules/security/compiler-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { severityDescription } = require('../../doc/utils')

const ruleId = 'compiler-version'
const DEFAULT_SEVERITY = 'error'
const DEFAULT_SEMVER = '^0.5.8'
const DEFAULT_SEMVER = '^0.8.0'
const meta = {
type: 'security',

Expand Down Expand Up @@ -36,7 +36,8 @@ class CompilerVersionChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)

this.requirement = (config && config.getString(ruleId, DEFAULT_SEMVER)) || DEFAULT_SEMVER
this.requirement =
(config && config.getStringFromArray(ruleId, DEFAULT_SEMVER)) || DEFAULT_SEMVER
}

SourceUnit(node) {
Expand Down
2 changes: 1 addition & 1 deletion test/rules/best-practises/no-global-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Linter - no-global-import', () => {
assertErrorMessage(report, 'Specify names to import individually')
})
it('should raise warning when using solhint:recommended', () => {
const code = `pragma solidity ^0.5.8; import "./A.sol";`
const code = `pragma solidity ^0.8.0; import "./A.sol";`

const report = linter.processStr(code, {
extends: 'solhint:recommended',
Expand Down
2 changes: 1 addition & 1 deletion test/rules/best-practises/no-unused-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Linter - no-unused-import', () => {
})

it('should raise error when using solhint:recommended', () => {
const code = `pragma solidity ^0.5.8; import {A} from "./A.sol";`
const code = `pragma solidity ^0.8.0; import {A} from "./A.sol";`

const report = linter.processStr(code, {
extends: 'solhint:recommended',
Expand Down
110 changes: 85 additions & 25 deletions test/rules/security/compiler-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ const assert = require('assert')
const { assertNoErrors, assertErrorCount, assertErrorMessage } = require('../../common/asserts')
const linter = require('../../../lib/index')

const DEFAULT_SEMVER = '^0.8.0'

describe('Linter - compiler-version', () => {
it('should disable only one compiler error on next line', () => {
const report = linter.processStr(
Expand All @@ -11,7 +13,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

Expand All @@ -26,7 +28,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

Expand All @@ -41,7 +43,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

Expand All @@ -56,7 +58,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

Expand All @@ -72,12 +74,12 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

assertErrorCount(report, 1)
assertErrorMessage(report, '0.5.2')
assertErrorMessage(report, DEFAULT_SEMVER)
})

it('should disable all errors', () => {
Expand All @@ -88,7 +90,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
assertNoErrors(report)
Expand All @@ -103,85 +105,85 @@ describe('Linter - compiler-version', () => {
pragma solidity ^0.4.4;
`,
{
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)

assertErrorCount(report, 1)
assertErrorMessage(report, '0.5.2')
assertErrorMessage(report, DEFAULT_SEMVER)
})

it('should return compiler version error', () => {
const report = linter.processStr('pragma solidity 0.3.4;', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 1)
assert.ok(report.reports[0].message.includes('0.5.2'))
assert.ok(report.reports[0].message.includes(DEFAULT_SEMVER))
})

it('should not report compiler version error on exact match', () => {
const report = linter.processStr('pragma solidity 0.5.2;', {
rules: { 'compiler-version': ['error', '0.5.2'] },
const report = linter.processStr('pragma solidity 0.8.0;', {
rules: { 'compiler-version': ['error', '0.8.0'] },
})

assert.equal(report.errorCount, 0)
})

it('should not report compiler version error on range match', () => {
const report = linter.processStr('pragma solidity ^0.5.2;', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
const report = linter.processStr('pragma solidity ^0.8.0;', {
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 0)
})

it('should not report compiler version error on patch bump', () => {
const report = linter.processStr('pragma solidity 0.5.3;', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
const report = linter.processStr('pragma solidity 0.8.1;', {
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 0)
})

it('should not report compiler version error on range match', () => {
const report = linter.processStr('pragma solidity ^0.5.3;', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
const report = linter.processStr('pragma solidity ^0.8.2;', {
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 0)
})

it('should report compiler version error on range not matching', () => {
const report = linter.processStr('pragma solidity ^0.5.2;', {
rules: { 'compiler-version': ['error', '^0.5.3'] },
const report = linter.processStr('pragma solidity ^0.8.1;', {
rules: { 'compiler-version': ['error', '^0.8.3'] },
})

assert.equal(report.errorCount, 1)
})

it('should report compiler version error on minor bump', () => {
const report = linter.processStr('pragma solidity 0.6.0;', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 1)
})

it(`should report compiler version error if pragma doesn't exist`, () => {
const report = linter.processStr('contract Foo {}', {
rules: { 'compiler-version': ['error', '^0.5.2'] },
rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})

assert.equal(report.errorCount, 1)
})

it(`should not report compiler version error if pragma exist`, () => {
const report = linter.processStr(
`pragma solidity 0.6.0;
`pragma solidity 0.8.2;
contract Foo {}`,
{
rules: { 'compiler-version': ['error', '^0.6.0'] },
rules: { 'compiler-version': ['error', '^0.8.2'] },
}
)

Expand All @@ -202,4 +204,62 @@ describe('Linter - compiler-version', () => {

assert.equal(report.errorCount, 0)
})

it(`should not report compiler version error using default and correct pragma`, () => {
const report = linter.processStr(
`pragma solidity ^0.8.1;
pragma experimental ABIEncoderV2;
contract Main {
}`,
{
rules: { 'compiler-version': 'error' },
}
)

assert.equal(report.errorCount, 0)
})

it(`should report compiler version error using default and lower pragma`, () => {
const report = linter.processStr(
`pragma solidity ^0.7.4;
contract Main {
}`,
{
rules: { 'compiler-version': 'error' },
}
)

assert.equal(report.errorCount, 1)
assertErrorMessage(report, DEFAULT_SEMVER)
})

it(`should not report compiler version error using >= and default and correct pragma`, () => {
const report = linter.processStr(
`pragma solidity >=0.8.0;
contract Main {
}`,
{
rules: { 'compiler-version': 'error' },
}
)

assert.equal(report.errorCount, 0)
})

it(`should report compiler version error using >= and default and lower pragma`, () => {
const report = linter.processStr(
`pragma solidity >=0.7.4;
contract Main {
}`,
{
rules: { 'compiler-version': 'error' },
}
)
assert.equal(report.errorCount, 1)
assertErrorMessage(report, DEFAULT_SEMVER)
})
})

0 comments on commit b67d523

Please sign in to comment.