solhint - npm Package Compare versions

Comparing version 2.0.0 to 2.1.0

CHANGELOG.md

@@ -0,1 +1,15 @@
+## [2.1.0] - 2019-05-30
+
+### Added
+
+- New `compiler-version` rule (see PR #112)
+
+### Fixed
+
+- Several fixes for the `mark-callable-contracts` rule (PRs #115, #117 and #119)
+
+## [2.0.0] - 2019-02-15
+
+Stable release
+
 ## [2.0.0-beta.1] - 2019-01-31
@@ -59,2 +73,2 @@ ### Fixed
 ### Fixed
-- Unable to satisfy indentation rules for functions with multiple return values [#49](https://github.com/protofire/solhint/issues/49)
+- Unable to satisfy indentation rules for functions with multiple return values [#49](https://github.com/protofire/solhint/issues/49)

docs/rules.md

@@ -11,3 +11,3 @@ ---
  Default list of options are *false, "error", "warn"*. It supports by all rules.
- It provides in format 
+ It provides in format
  ```json
@@ -35,2 +35,3 @@ {
  | **compiler-gt-0_4**           | Use at least '0.4' compiler version             | *[default](#options)*     |
+ | **compiler-version**          | Use a compiler version satisfying a semver requirement. | [*\<[default](#options)\>*,&nbsp;\<*version*\>] Default *version* is **'^0.5.8'**. |
  | **no-complex-fallback**       | Fallback function must be simple                | *[default](#options)*     |
@@ -45,9 +46,9 @@ | **mark-callable-contracts**   | Explicitly mark all external contracts as trusted or untrusted | *[default](#options)* |
  | **avoid-low-level-calls**     | Avoid to use low level calls.                                     | *[default](#options)* |
- 
+
 \* \- All security rules implemented according [ConsenSys Guide for Smart Contracts](https://consensys.github.io/smart-contract-best-practices/recommendations/)
- 
+
 ### Style Guide Rules
- 
+
  | Rule ID                       |                      Error                         |     Options                    |
- |-------------------------------|----------------------------------------------------|--------------------------------| 
+ |-------------------------------|----------------------------------------------------|--------------------------------|
  | **func-name-mixedcase**       | Function name must be in camelCase                 | *[default](#options)*          |
@@ -75,11 +76,11 @@ | **func-param-name-mixedcase** | Function param name must be in mixedCase           | *[default](#options)*          |
  | **no-spaces-before-semicolon**| Semicolon must not have spaces before              | *[default](#options)*          |
- 
+
 \* \- All style guide rules implemented according [Solidity Style Guide](
 http://solidity.readthedocs.io/en/develop/style-guide.html)
- 
+
 ### Best Practise Rules
- 
+
  | Rule ID                       |                      Error                         |     Options                   |
- |-------------------------------|----------------------------------------------------|-------------------------------| 
- | **max-line-length**           | Line length must be no more than *maxlen*. | [*\<[default](#options)\>*,&nbsp;*\<maxlen\>*] Default *maxlen* is **120**. | 
+ |-------------------------------|----------------------------------------------------|-------------------------------|
+ | **max-line-length**           | Line length must be no more than *maxlen*. | [*\<[default](#options)\>*,&nbsp;*\<maxlen\>*] Default *maxlen* is **120**. |
  | **payable-fallback**          | When fallback is not payable you will not be able to receive ethers | *[default](#options)* |
@@ -86,0 +87,0 @@ | **no-empty-blocks**           | Code contains empty block                          | *[default](#options)*         |

lib/config.js

@@ -13,5 +13,14 @@ const _ = require('lodash')
 
+  getStringByPath(path, defaultValue) {
+    const configVal = _.get(this, path)
+    return _.isString(configVal) ? configVal : defaultValue
+  },
+
   getNumber(ruleName, defaultValue) {
     return this.getNumberByPath(`rules["${ruleName}"][1]`, defaultValue)
+  },
+
+  getString(ruleName, defaultValue) {
+    return this.getStringByPath(`rules["${ruleName}"][1]`, defaultValue)
   }
 }

lib/rules/order/func-order.js

@@ -75,2 +75,3 @@ const BaseChecker = require('./../base-checker')
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -89,2 +90,3 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -102,2 +104,3 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -115,2 +118,3 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -127,2 +131,3 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -138,2 +143,3 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
       { on: 'public', goTo: 'AFTER_PUBLIC' },
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
       { on: 'internal', goTo: 'AFTER_INTERNAL' },
@@ -144,2 +150,12 @@ { on: 'private', goTo: 'AFTER_PRIVATE' }
 
+  AFTER_PUBLIC_CONSTANT: new State({
+    name: 'AFTER_PUBLIC_CONSTANT',
+    after: 'public constant',
+    rules: [
+      { on: 'public_const', goTo: 'AFTER_PUBLIC_CONSTANT' },
+      { on: 'internal', goTo: 'AFTER_INTERNAL' },
+      { on: 'private', goTo: 'AFTER_PRIVATE' }
+    ]
+  }),
+
   AFTER_INTERNAL: new State({
@@ -222,3 +238,7 @@ name: 'AFTER_INTERNAL',
 
-    return 'public'
+    if (includesAnyOf(text, ['pure', 'view', 'constant'])) {
+      return 'public_const'
+    } else {
+      return 'public'
+    }
   }
@@ -225,0 +245,0 @@ }

lib/rules/security/index.js

@@ -9,2 +9,3 @@ const AvoidCallValueChecker = require('./avoid-call-value')
 const CompilerFixedChecker = require('./compiler-fixed')
+const CompilerVersionChecker = require('./compiler-version')
 const CompilerGT04Checker = require('./compiler-gt-0_4')
@@ -22,3 +23,3 @@ const FuncVisibilityChecker = require('./func-visibility')
 
-module.exports = function security(reporter) {
+module.exports = function security(reporter, config) {
   return [
@@ -33,2 +34,3 @@ new AvoidCallValueChecker(reporter),
     new CompilerFixedChecker(reporter),
+    new CompilerVersionChecker(reporter, config),
     new CompilerGT04Checker(reporter),
@@ -35,0 +37,0 @@ new FuncVisibilityChecker(reporter),

lib/rules/security/mark-callable-contracts.js

@@ -28,11 +28,37 @@ const TreeTraversing = require('./../../common/tree-traversing')
     this.meta = meta
+    this.nonContractNames = []
   }
 
+  enterStructDefinition(ctx) {
+    this.gatherNonContractNames(ctx)
+  }
+
+  enterEventDefinition(ctx) {
+    this.gatherNonContractNames(ctx)
+  }
+
+  exitStateVariableDeclaration(ctx) {
+    const hasConstModifier = ctx.children.some(i => i.getText() === 'constant')
+
+    if (hasConstModifier) {
+      this._forEachIdentifier(ctx, (curId, name) => {
+        if (name) {
+          this.nonContractNames.push(name)
+        }
+      })
+    }
+  }
+
   exitIdentifier(ctx) {
     const identifier = ctx.getText()
     const isFirstCharUpper = /[A-Z]/.test(identifier[0])
-    const isContainsTrustInfo = identifier.toLowerCase().includes('trust')
+    const containsTrustInfo = identifier.toLowerCase().includes('trust')
     const isStatement = traversing.findParentType(ctx, 'StatementContext')
 
-    if (isFirstCharUpper && !isContainsTrustInfo && isStatement) {
+    if (
+      isFirstCharUpper &&
+      !containsTrustInfo &&
+      isStatement &&
+      !this.nonContractNames.includes(identifier)
+    ) {
       this.reporter.addMessage(
@@ -46,4 +72,23 @@ ctx.getSourceInterval(),
   }
+
+  gatherNonContractNames(ctx) {
+    const identifier = ctx.children[1]
+    const name = identifier.getText()
+
+    if (name) {
+      this.nonContractNames.push(name)
+    }
+  }
+
+  _forEachIdentifier(ctx, callback) {
+    for (const curId of traversing.findIdentifier(ctx)) {
+      const text = curId.getText()
+
+      if (callback) {
+        callback(curId, text)
+      }
+    }
+  }
 }
 
 module.exports = MarkCallableContractsChecker

package.json

 {
   "name": "solhint",
-  "version": "2.0.0",
+  "version": "2.1.0",
   "description": "Solidity Code Linter",
@@ -40,3 +40,4 @@ "main": "solhint.js",
     "js-yaml": "^3.12.0",
-    "lodash": "^4.17.11"
+    "lodash": "^4.17.11",
+    "semver": "^6.0.0"
   },
@@ -43,0 +44,0 @@ "devDependencies": {

README.md

@@ -203,5 +203,5 @@ # Solhint Project
   - [Proof of Bank Account (PoBA)](https://github.com/poanetwork/poa-poba)
-- [0x](https://github.com/0xProject/0x-monorepo/tree/development/packages/contracts)
+- [0x](https://github.com/0xProject/0x-monorepo/tree/development/contracts)
 - Gnosis:
   - [Gnosis Prediction Market Contracts](https://github.com/gnosis/pm-contracts)
   - [The DutchX decentralized trading protocol](https://github.com/gnosis/dex-contracts)

test/rules/order/func-order.js

@@ -34,2 +34,16 @@ const assert = require('assert')
 
+  it('should raise incorrect function order error for public constant funcs', () => {
+    const code = contractWith(`
+              function b() public pure {}
+              function c() public {}
+          `)
+
+    const report = linter.processStr(code, {
+      rules: { 'func-order': 'error' }
+    })
+
+    assert.equal(report.errorCount, 1)
+    assert.ok(report.messages[0].message.includes('Function order is incorrect'))
+  })
+
   it('should raise incorrect function order error for internal function', () => {
@@ -36,0 +50,0 @@ const code = contractWith(`

test/rules/security/mark-callable-contracts.js

 const assert = require('assert')
 const linter = require('./../../../lib/index')
-const funcWith = require('./../../common/contract-builder').funcWith
+const { funcWith, contractWith } = require('./../../common/contract-builder')
 
@@ -16,2 +16,76 @@ describe('Linter - mark-callable-contracts', () => {
   })
+
+  it('should not return error for external contract that is marked as trusted', () => {
+    const code = funcWith('TrustedBank.withdraw(100);')
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+
+  it('should not return error for external contract that is marked as untrusted', () => {
+    const code = funcWith('UntrustedBank.withdraw(100);')
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+
+  it('should not return error for a struct', () => {
+    const code = contractWith(`
+  struct Token {
+    address tokenAddress;
+    string tokenSymbol;
+  }
+
+  mapping(address => Token) public acceptedTokens;
+
+  function b(address tokenAddress, string memory tokenSymbol) public {
+    Token memory token = Token(tokenAddress, tokenSymbol);
+    acceptedTokens[tokenAddress] = token;
+  }
+    `)
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+
+  it('should not return error for an event', () => {
+    const code = contractWith(`
+  event UpdatedToken();
+
+  function b() public {
+    emit UpdatedToken();
+  }
+    `)
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+
+  it('should not return error for constant', () => {
+    const code = contractWith(`
+  uint8 private constant TOKEN_DECIMALS = 15;
+
+  function b() public view returns(uint8) {
+    return TOKEN_DECIMALS;
+  }
+    `)
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
 })

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

745680

increased by1.43%

Number of package files

177

increased by0.57%

Lines of code

16866

increased by1.91%

Worsened metrics

Dependency count

12

increased by9.09%

Dependency changes

• + Addedsemver@^6.0.0

• + Addedsemver@6.3.1(transitive)