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

Support no-visibility constructors in >=0.7.0 #241

Merged
merged 1 commit into from
Aug 12, 2020
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
11 changes: 9 additions & 2 deletions docs/rules/security/func-visibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ title: "func-visibility | Solhint"
Explicitly mark visibility in function.

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

| Index | Description | Default Value |
| ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------- |
| 0 | Rule severity. Must be one of "error", "warn", "off". | warn |
| 1 | A JSON object with a single property "ignoreConstructors" specifying if the rule should ignore constructors. (**Note: This is required to be true for Solidity >=0.7.0 and false for <0.7.0**) | {"ignoreConstructors":false} |


### Example Config
```json
{
"rules": {
"func-visibility": "warn"
"func-visibility": ["warn",{"ignoreConstructors":false}]
}
}
```
Expand All @@ -37,6 +43,7 @@ function b() internal { }
function b() external { }
function b() private { }
function b() public { }
constructor() public { }
```

### 👎 Examples of **incorrect** code for this rule
Expand Down
43 changes: 39 additions & 4 deletions lib/rules/security/func-visibility.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
const BaseChecker = require('./../base-checker')
const { severityDescription } = require('../../doc/utils')

const DEFAULT_SEVERITY = 'warn'
const DEFAULT_IGNORE_CONSTRUCTORS = false
const DEFAULT_OPTION = { ignoreConstructors: DEFAULT_IGNORE_CONSTRUCTORS }

const ruleId = 'func-visibility'
const meta = {
Expand All @@ -7,6 +12,17 @@ const meta = {
docs: {
description: `Explicitly mark visibility in function.`,
category: 'Security Rules',
options: [
{
description: severityDescription,
default: DEFAULT_SEVERITY
},
{
description:
'A JSON object with a single property "ignoreConstructors" specifying if the rule should ignore constructors. (**Note: This is required to be true for Solidity >=0.7.0 and false for <0.7.0**)',
default: JSON.stringify(DEFAULT_OPTION)
}
],
examples: {
good: [
{
Expand All @@ -25,19 +41,38 @@ const meta = {

isDefault: false,
recommended: true,
defaultSetup: 'warn',
defaultSetup: [DEFAULT_SEVERITY, DEFAULT_OPTION],

schema: null
schema: {
type: 'object',
properties: {
ignoreConstructors: {
type: 'boolean'
}
}
}
}

class FuncVisibilityChecker extends BaseChecker {
constructor(reporter) {
constructor(reporter, config) {
super(reporter, ruleId, meta)

this.ignoreConstructors =
config && config.getObjectPropertyBoolean(ruleId, 'ignoreConstructors', false)
}

FunctionDefinition(node) {
if (node.isConstructor && this.ignoreConstructors) {
return
}

if (node.visibility === 'default') {
this.warn(node, 'Explicitly mark visibility in function')
this.warn(
node,
node.isConstructor
? 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)'
: 'Explicitly mark visibility in function'
)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module.exports = function security(reporter, config, inputSrc) {
new AvoidTxOriginChecker(reporter),
new CheckSendResultChecker(reporter),
new CompilerVersionChecker(reporter, config),
new FuncVisibilityChecker(reporter),
new FuncVisibilityChecker(reporter, config),
new MarkCallableContractsChecker(reporter),
new MultipleSendsChecker(reporter),
new NoComplexFallbackChecker(reporter),
Expand Down
10 changes: 5 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"author": "Ilya Drabenia <ilya.drobenya@gmail.com>",
"license": "MIT",
"dependencies": {
"@solidity-parser/parser": "^0.6.0",
"@solidity-parser/parser": "^0.7.0",
"ajv": "^6.6.1",
"antlr4": "4.7.1",
"ast-parents": "0.0.1",
Expand Down
3 changes: 2 additions & 1 deletion test/fixtures/security/functions-with-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ module.exports = [
'function b() internal { }',
'function b() external { }',
'function b() private { }',
'function b() public { }'
'function b() public { }',
'constructor() public { }'
]
26 changes: 26 additions & 0 deletions test/rules/security/func-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,30 @@ describe('Linter - func-visibility', () => {
assert.equal(report.warningCount, 0)
})
})

describe("when 'ignoreConstructors' is enabled", () => {
it('should ignore constructors without visibility', () => {
const code = contractWith('constructor () {}')

const report = linter.processStr(code, {
rules: {
'func-visibility': ['warn', { ignoreConstructors: true }]
}
})

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

it('should still report functions without visibility', () => {
const code = contractWith('function foo() {}')

const report = linter.processStr(code, {
rules: {
'func-visibility': ['warn', { ignoreConstructors: true }]
}
})

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