Disable coloring if output is not a terminal, fix Windows tests (#1244)

* Disable coloring if output is not a terminal

Color escape codes are most likely not expected if piping the output
of slither into a file or other tool. Check if stdout is a tty before
enabling colored output.

.github/workflows/ci.yml

@@ -3,8 +3,7 @@ name: CI
 
 defaults:
   run:
-    # To load bashrc
-    shell: bash -ieo pipefail {0}
+    shell: bash
 
 on:
   push:
@@ -52,16 +51,10 @@ jobs:
     steps:
       - uses: actions/checkout@v1
 
-      - name: Set up shell
-        if: runner.os == 'Windows'
-        run: |
-          echo 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH"
-          echo 'C:\msys64\usr\bin' >> "$GITHUB_PATH"
-
-      - name: Set up Python 3.6
+      - name: Set up Python 3.8
         uses: actions/setup-python@v3
         with:
-          python-version: 3.6
+          python-version: 3.8
       - name: Install dependencies
         run: |
 
@@ -87,6 +80,7 @@ jobs:
 
       - name: Run Tests
         env:
+          PYTHONUTF8: 1
           TEST_TYPE: ${{ matrix.type }}
           GITHUB_ETHERSCAN: ${{ secrets.GITHUB_ETHERSCAN }}
         run: |

scripts/ci_test_kspec.sh

@@ -7,7 +7,7 @@ slither-check-kspec "$DIR_TESTS/safeAdd/safeAdd.sol" "$DIR_TESTS/safeAdd/spec.md
 DIFF=$(diff test_1.txt "$DIR_TESTS/test_1.txt")
 if [  "$DIFF" != "" ]
 then
-    echo "slither-check-upgradeability 1 failed"
+    echo "slither-check-kspec 1 failed"
     cat test_1.txt
     echo ""
     cat "$DIR_TESTS/test_1.txt"

scripts/ci_test_simil.sh

@@ -2,8 +2,8 @@
 
 ### Install requisites
 
-pip3.6 install pybind11
-pip3.6 install https://github.com/facebookresearch/fastText/archive/0.2.0.zip
+pip3 install pybind11
+pip3 install https://github.com/facebookresearch/fastText/archive/0.2.0.zip
 
 ### Test slither-simil
 

slither/__main__.py

@@ -659,7 +659,7 @@ def main_impl(all_detector_classes, all_printer_classes):
         cp.enable()
 
     # Set colorization option
-    set_colorization_enabled(not args.disable_color)
+    set_colorization_enabled(False if args.disable_color else sys.stdout.isatty())
 
     # Define some variables for potential JSON output
     json_results = {}

slither/utils/colors.py

@@ -1,5 +1,6 @@
 from functools import partial
 import platform
+import sys
 
 
 class Colors:  # pylint: disable=too-few-public-methods
@@ -73,7 +74,7 @@ def set_colorization_enabled(enabled: bool):
     if enabled and platform.system() == "Windows":
         Colors.COLORIZATION_ENABLED = enable_windows_virtual_terminal_sequences()
     else:
-        # This is not windows so we can enable color immediately.
+        # This is not windows, or colorization is being disabled, so we can adjust the state immediately.
         Colors.COLORIZATION_ENABLED = enabled
 
 
@@ -83,6 +84,5 @@ red = partial(colorize, Colors.RED)
 blue = partial(colorize, Colors.BLUE)
 magenta = partial(colorize, Colors.MAGENTA)
 
-# We enable colorization by default (this call is important as it will enable color mode on Windows by default),
-# regardless of whether Slither is interacted with from CLI or another script.
-set_colorization_enabled(True)
+# We enable colorization by default if the output is a tty
+set_colorization_enabled(sys.stdout.isatty())

tests/check-kspec/test_1.txt

@@ -1,7 +1,7 @@
 ## Check for functions coverage
-[✓] SafeAdd.add(uint256,uint256)
-
-[ ] (Missing function) SafeAdd.add_v2(uint256,uint256)
-
-[ ] (Unresolved) SafeAdd.addv2(uint256,uint256)
-
+[✓] SafeAdd.add(uint256,uint256)
+
+[ ] (Missing function) SafeAdd.add_v2(uint256,uint256)
+
+[ ] (Unresolved) SafeAdd.addv2(uint256,uint256)
+

tests/check-upgradeability/test_1.txt

@@ -1,4 +1,4 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
 INFO:Slither:1 findings, 12 detectors run

tests/check-upgradeability/test_10.txt

@@ -1,13 +1,13 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+INFO:Slither:
 ContractV1.destination (tests/check-upgradeability/contractV1.sol#2) was not constant but ContractV2.destination (tests/check-upgradeability/contract_v2_constant.sol#2) is.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-that-should-not-be-constant
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-that-should-not-be-constant
+INFO:Slither:
 Variable missing in ContractV2 (tests/check-upgradeability/contract_v2_constant.sol#1-3): ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#missing-variables
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#missing-variables
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
 INFO:Slither:4 findings, 18 detectors run

tests/check-upgradeability/test_11.txt

@@ -1,7 +1,7 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+INFO:Slither:
 ContractV1.destination (tests/check-upgradeability/contract_v1_var_init.sol#2) is a state variable with an initial value.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#state-variable-initialized
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#state-variable-initialized
 INFO:Slither:2 findings, 8 detectors run

tests/check-upgradeability/test_2.txt

@@ -1,7 +1,7 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
 INFO:Slither:2 findings, 22 detectors run

tests/check-upgradeability/test_3.txt

@@ -1,23 +1,23 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+INFO:Slither:
 Different variables between ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5) and Proxy (tests/check-upgradeability/proxy.sol#7-27)
 	 ContractV2.destination (tests/check-upgradeability/contractV2_bug.sol#2)
 	 Proxy.destination (tests/check-upgradeability/proxy.sol#9)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
+INFO:Slither:
 Function shadowing found: ContractV2.myFunc (tests/check-upgradeability/contractV2_bug.sol#4) Proxy.myFunc() (tests/check-upgradeability/proxy.sol#11)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-shadowing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-shadowing
+INFO:Slither:
 Different variables between ContractV1 (tests/check-upgradeability/contractV1.sol#1-3) and ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5)
 	 ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
 	 ContractV2.destination (tests/check-upgradeability/contractV2_bug.sol#2)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
+INFO:Slither:
 Extra variables in ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5): ContractV2.myFunc (tests/check-upgradeability/contractV2_bug.sol#4)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
 INFO:Slither:6 findings, 22 detectors run

tests/check-upgradeability/test_4.txt

@@ -1,20 +1,20 @@
-INFO:Slither:
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+INFO:Slither:
 Different variables between ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6) and Proxy (tests/check-upgradeability/proxy.sol#7-27)
 	 Base.val (tests/check-upgradeability/contractV2_bug2.sol#2)
 	 Proxy.destination (tests/check-upgradeability/proxy.sol#9)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
+INFO:Slither:
 Different variables between ContractV1 (tests/check-upgradeability/contractV1.sol#1-3) and ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6)
 	 ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
 	 Base.val (tests/check-upgradeability/contractV2_bug2.sol#2)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
+INFO:Slither:
 Extra variables in ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6): ContractV2.destination (tests/check-upgradeability/contractV2_bug2.sol#5)
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
+INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
 INFO:Slither:5 findings, 22 detectors run

tests/check-upgradeability/test_5.txt

@@ -1,4 +1,4 @@
-INFO:Slither:
+INFO:Slither:
 Contract_no_bug (tests/check-upgradeability/contract_initialization.sol#11-17) needs to be initialized by Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:1 findings, 12 detectors run

tests/check-upgradeability/test_6.txt

@@ -1,7 +1,7 @@
-INFO:Slither:
+INFO:Slither:
 Contract_lack_to_call_modifier (tests/check-upgradeability/contract_initialization.sol#19-24) needs to be initialized by Contract_lack_to_call_modifier.initialize() (tests/check-upgradeability/contract_initialization.sol#21-23).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:
 Contract_lack_to_call_modifier.initialize() (tests/check-upgradeability/contract_initialization.sol#21-23) does not call the initializer modifier.
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-not-called
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-not-called
 INFO:Slither:2 findings, 12 detectors run

tests/check-upgradeability/test_7.txt

@@ -1,7 +1,7 @@
-INFO:Slither:
+INFO:Slither:
 Contract_not_called_super_init (tests/check-upgradeability/contract_initialization.sol#26-32) needs to be initialized by Contract_not_called_super_init.initialize() (tests/check-upgradeability/contract_initialization.sol#28-30).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:
 Missing call to Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15) in Contract_not_called_super_init.initialize() (tests/check-upgradeability/contract_initialization.sol#28-30).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-not-called
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-not-called
 INFO:Slither:2 findings, 12 detectors run

tests/check-upgradeability/test_8.txt

@@ -1,4 +1,4 @@
-INFO:Slither:
+INFO:Slither:
 Contract_no_bug_inherits (tests/check-upgradeability/contract_initialization.sol#34-40) needs to be initialized by Contract_no_bug_inherits.initialize() (tests/check-upgradeability/contract_initialization.sol#36-38).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:1 findings, 12 detectors run

tests/check-upgradeability/test_9.txt

@@ -1,7 +1,7 @@
-INFO:Slither:
+INFO:Slither:
 Contract_double_call (tests/check-upgradeability/contract_initialization.sol#42-49) needs to be initialized by Contract_double_call.initialize() (tests/check-upgradeability/contract_initialization.sol#44-47).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
-INFO:Slither:
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:
 Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15) is called multiple times in Contract_double_call.initialize() (tests/check-upgradeability/contract_initialization.sol#44-47).
-Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-called-multiple-times
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-called-multiple-times
 INFO:Slither:2 findings, 12 detectors run