Merge pull request #518 from protofire/develop

v4 20231027 develop to master
protofire · Oct 27, 2023 · ca7ba78 · ca7ba78
2 parents 229db23 + a11b89a
commit ca7ba78
Show file tree

Hide file tree

Showing 48 changed files with 2,024 additions and 102 deletions.
diff --git a/.gitignore b/.gitignore
@@ -9,4 +9,5 @@ convertLib.sol
 antlr4.jar
 /docs/.sass-cache/
 _temp/
+*solhintReport*.*
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,26 @@
+## [4.0] - 2023-10-01
+
+### Updated
+- Enhance explicit types sensitivity [#493](https://github.com/protofire/solhint/pull/493) (Thanks to [@vladyan18](https://github.com/vladyan18))
+- Docs on `private-vars-leading-underscore` rule to clarify its functionality
+- Changelog and docs for `no-empty-blocks` rule to clarify its functionality
+- Require package with full path [#515](https://github.com/protofire/solhint/pull/515) (Thanks to [@zouguangxian](@https://github.com/zouguangxian))
+
+### Added
+- Check for updates on Solhint version to keep users up to date. There's an option to disable this check (`--disc`) [#506](https://github.com/protofire/solhint/pull/506)
+- `fix` option now shows the report on screen [#509](https://github.com/protofire/solhint/pull/509)
+- `save` option to store report on disk with the standard or the specified format [#509](https://github.com/protofire/solhint/pull/509) 
+- Autofix for `explicit-types` rule [#504](https://github.com/protofire/solhint/pull/504)
+- Autofix for `no-console` rule [#513](https://github.com/protofire/solhint/pull/513)
+- Autofix for `private-vars-leading-underscore` rule [#511](https://github.com/protofire/solhint/pull/511)
+
+### Fixed
+- Generate docs script on Windows OS [#494](https://github.com/protofire/solhint/pull/494) (Thanks to [@vladyan18](https://github.com/vladyan18))
+- `one-contract-per-file` ignore interfaces [#514](https://github.com/protofire/solhint/pull/514) (Thanks to [@cruzdanilo](https://github.com/cruzdanilo))
+
+
+
+
 ## [3.6.2] - 2023-08-17
 ### Added
 - New Rule: `one-contract-per-file` - Enforces the use of ONE contract per file [#487](https://github.com/protofire/solhint/pull/487)
@@ -31,28 +54,30 @@ 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
 
+
+### Updated
+- 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)
+
 ### 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)
 - New Rule: Enforces the naming of function return values [#478](https://github.com/protofire/solhint/pull/478)
 - `Notes` option on docs to add more information of each rule. See `foundry-test-functions`. [#476](https://github.com/protofire/solhint/pull/476)
-  
+
 ### Fixed
 - `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)
 
 
 
 ## [3.5.1] - 2023-08-04
 ### Updated
-- Support `ignoreConstructors` option for `no-empty-blocks` [#418](https://github.com/protofire/solhint/pull/418)
+- Ignores empty constructors when inheriting a base contract [#418](https://github.com/protofire/solhint/pull/418)
 - Bump json5 from 2.1.3 to 2.2.3 [#376](https://github.com/protofire/solhint/pull/376)
 - Bump json-schema and jsprim [#370](https://github.com/protofire/solhint/pull/370)
 - Bump semver from 6.3.0 to 7.5.2 [#438](https://github.com/protofire/solhint/pull/438)
@@ -64,7 +89,6 @@ If not explicitly added, this rule will not be executed.
 - Removed runtime dependencies on load-rules [#462](https://github.com/protofire/solhint/pull/462)
 - Allowed $ symbol as part of naming [#465](https://github.com/protofire/solhint/issues/465)
 - Disabled `no-empty-blocks` rule for receive() function [#466](https://github.com/protofire/solhint/pull/466)
-
 
 ### Added
 - New Rule: No unused imports [#417](https://github.com/protofire/solhint/pull/417)

diff --git a/README.md b/README.md
@@ -5,6 +5,7 @@
   By <a href="https://protofire.io/">Protofire</a>
 </p>
 
+[![Join Discord](https://img.shields.io/badge/join-Discord-red)](https://discord.gg/4TYGq3zpjs)
 [![Donate with Ethereum](https://img.shields.io/badge/Donate-ETH-blue)](https://etherscan.io/address/0xA81705c8C247C413a19A244938ae7f4A0393944e)
 [![NPM version](https://badge.fury.io/js/solhint.svg)](https://npmjs.org/package/solhint)
 [![Coverage Status](https://coveralls.io/repos/github/protofire/solhint/badge.svg?branch=master)](
@@ -14,6 +15,7 @@ https://coveralls.io/github/protofire/solhint?branch=master)
 This is an open source project for linting [Solidity](http://solidity.readthedocs.io/en/develop/) code. This project
 provides both **Security** and **Style Guide** validations.
 
+[JOIN OUR DISCORD SERVER](https://discord.gg/4TYGq3zpjs)
 ## Installation
 
 You can install Solhint using **npm**:
@@ -60,17 +62,29 @@ Options:
   -c, --config [file_name]                file to use as your .solhint.json
   -q, --quiet                             report errors only - default: false
   --ignore-path [file_name]               file to use as your .solhintignore
-  --fix                                   automatically fix problems
+  --fix                                   automatically fix problems and show report
+  --noPrompt                              do not suggest to backup files when any `fix` option is selected
   --init                                  create configuration file for solhint
+  --disc                                  do not check for solhint updates
+  --save                                  save report to file on current folder
   -h, --help                              output usage information
 
 Commands:
 
   stdin [options]                         linting of source code data provided to STDIN
   list-rules                              display covered rules of current .solhint.json
 ```
-### Note
-The `--fix`  option currently works only on "avoid-throw" and "avoid-sha3" rules
+### Notes
+- Solhint checks if there are newer versions. The `--disc`  option avoids that check.
+- `--save` option will create a file named as `YYYYMMDDHHMMSS_solhintReport.txt` on current folder with default or specified format 
+
+### Fix
+This option currently works on:
+- avoid-throw
+- avoid-sha3
+- no-console
+- explicit-types
+- private-vars-underscore
 <br><br>
 ## Configuration
 

diff --git a/docs/rules.md b/docs/rules.md
@@ -48,7 +48,7 @@ title:       "Rule Index of Solhint"
 | [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) | Private and internal names must start with a single underscore.                                               |              |             |
+| [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)                    | $~~~~~~~~$✔️ |             |
 | [func-order](./rules/order/func-order.md)                                            | Function order is incorrect.                                                                                  |              | $~~~~~~~$✔️ |

diff --git a/docs/rules/best-practises/explicit-types.md b/docs/rules/best-practises/explicit-types.md
@@ -32,6 +32,8 @@ This rule accepts an array of options:
 }
 ```
 
+### Notes
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule
@@ -48,6 +50,12 @@ uint256 public variableName
 uint public variableName
 ```
 
+#### If explicit is selected
+
+```solidity
+uint256 public variableName = uint256(5)
+```
+
 ### 👎 Examples of **incorrect** code for this rule
 
 #### If explicit is selected
@@ -62,6 +70,12 @@ uint public variableName
 uint256 public variableName
 ```
 
+#### At any setting
+
+```solidity
+uint public variableName = uint256(5)
+```
+
 ## Version
 This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
 

diff --git a/docs/rules/best-practises/no-empty-blocks.md b/docs/rules/best-practises/no-empty-blocks.md
@@ -26,6 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- The rule ignores an empty constructor by default as long as base contracts are being inherited. See "Empty Constructor" example.
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule

diff --git a/docs/rules/best-practises/one-contract-per-file.md b/docs/rules/best-practises/one-contract-per-file.md
@@ -31,7 +31,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.6.2](https://github.com/protofire/solhint/tree/v3.6.2)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/one-contract-per-file.js)

diff --git a/docs/rules/naming/private-vars-leading-underscore.md b/docs/rules/naming/private-vars-leading-underscore.md
@@ -9,15 +9,15 @@ title:       "private-vars-leading-underscore | Solhint"
 ![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
 
 ## Description
-Private and internal names must start with a single underscore.
+Non-external functions and state variables should start with a single underscore. Others, shouldn't
 
 ## Options
 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 "strict" specifying if the rule should apply to non state variables. Default: { strict: false }. | {"strict":false} |
+| Index | Description                                                                                                                               | Default Value    |
+| ----- | ----------------------------------------------------------------------------------------------------------------------------------------- | ---------------- |
+| 0     | Rule severity. Must be one of "error", "warn", "off".                                                                                     | warn             |
+| 1     | A JSON object with a single property "strict" specifying if the rule should apply to ALL non state variables. Default: { strict: false }. | {"strict":false} |
 
 
 ### Example Config
@@ -29,9 +29,64 @@ This rule accepts an array of options:
 }
 ```
 
+### Notes
+- This rule considers functions and variables in Libraries as well
+- This rule skips external and public functions
+- This rule skips external and public state variables
+- See [here](https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables) for further information
 
 ## Examples
-This rule does not have examples.
+### 👍 Examples of **correct** code for this rule
+
+#### Internal function with correct naming
+
+```solidity
+function _thisIsInternal() internal {}
+```
+
+#### Private function with correct naming
+
+```solidity
+function _thisIsPrivate() private {}
+```
+
+#### Internal state variable with correct naming
+
+```solidity
+uint256 internal _thisIsInternalVariable;
+```
+
+#### Internal state variable with correct naming (no visibility is considered internal)
+
+```solidity
+uint256 _thisIsInternalVariable;
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Internal function with incorrect naming
+
+```solidity
+function thisIsInternal() internal {}
+```
+
+#### Private function with incorrect naming
+
+```solidity
+function thisIsPrivate() private {}
+```
+
+#### Internal state variable with incorrect naming
+
+```solidity
+uint256 internal thisIsInternalVariable;
+```
+
+#### Internal state variable with incorrect naming (no visibility is considered internal)
+
+```solidity
+uint256 thisIsInternalVariable;
+```
 
 ## Version
 This rule was introduced in [Solhint 3.0.0-rc.3](https://github.com/protofire/solhint/tree/v3.0.0-rc.3)

diff --git a/docs/rules/security/avoid-sha3.md b/docs/rules/security/avoid-sha3.md
@@ -26,6 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 This rule does not have examples.

diff --git a/docs/rules/security/avoid-throw.md b/docs/rules/security/avoid-throw.md
@@ -26,6 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 This rule does not have examples.

diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
@@ -24,6 +24,7 @@ class MyNewRule {
 
     ...
   }
+}
 ```
 
 This is enough for the rule to work but, of course, this will do nothing. Rules are implemented using a visitor pattern: you implement methods that are called when a node in the AST is entered or exited. For example, let's make a rule that forbids naming contracts `Foo`:

diff --git a/e2e/06-formatters/helpers/helpers.js b/e2e/06-formatters/helpers/helpers.js
@@ -32,7 +32,6 @@ const foo1Output = [
     severity: 'Warning',
     message: "'TEST2' should start with _",
     ruleId: 'private-vars-leading-underscore',
-    fix: null,
     filePath: 'contracts/Foo.sol',
   },
   {

diff --git a/e2e/08-autofix/commands/.solhint.json b/e2e/08-autofix/commands/.solhint.json
@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "explicit-types": ["error", "explicit"]
+  }
+}
diff --git a/e2e/08-autofix/commands/Foo1.sol b/e2e/08-autofix/commands/Foo1.sol
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.4;
+
+import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol';
+
+contract Foo1 is ERC20Burnable {
+    uint public hola;
+    uint public hola2;
+    int public constant hola3 = 2;
+    ufixed hola4;
+    fixed internal hola5;
+
+    constructor() ERC20('MyToken', 'MTK') {}
+
+    // solhint-disable no-empty-blocks
+    function payableTrue() public payable {}
+
+    // solhint-disable no-empty-blocks
+    function payableFalse() public {}
+
+    function zarasa() {}
+}
diff --git a/e2e/08-autofix/commands/Foo1AfterFix.sol b/e2e/08-autofix/commands/Foo1AfterFix.sol
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.4;
+
+import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol';
+
+contract Foo1 is ERC20Burnable {
+    uint256 public hola;
+    uint256 public hola2;
+    int256 public constant hola3 = 2;
+    ufixed128x18 hola4;
+    fixed128x18 internal hola5;
+
+    constructor() ERC20('MyToken', 'MTK') {}
+
+    // solhint-disable no-empty-blocks
+    function payableTrue() public payable {}
+
+    // solhint-disable no-empty-blocks
+    function payableFalse() public {}
+
+    function zarasa() {}
+}
diff --git a/e2e/08-autofix/commands/Foo1BeforeFix.sol b/e2e/08-autofix/commands/Foo1BeforeFix.sol
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.4;
+
+import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol';
+
+contract Foo1 is ERC20Burnable {
+    uint public hola;
+    uint public hola2;
+    int public constant hola3 = 2;
+    ufixed hola4;
+    fixed internal hola5;
+
+    constructor() ERC20('MyToken', 'MTK') {}
+
+    // solhint-disable no-empty-blocks
+    function payableTrue() public payable {}
+
+    // solhint-disable no-empty-blocks
+    function payableFalse() public {}
+
+    function zarasa() {}
+}
diff --git a/e2e/08-autofix/explicit-types/.solhint.json b/e2e/08-autofix/explicit-types/.solhint.json
@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "explicit-types": ["error", "explicit"]
+  }
+}