fail-on: rework feature

This adds a new `fail_on` config option that can be changed with
a mutually exclusive group of argument flags. It also decouples
the exclude_* and fail_on flags, so you can do things like
`fail_on: pedantic` while disabling optimization findings.
Additionally, this adds some new code to detect the old-style
config options, migrate their settings, and alert the user.

Fixes #1458

slither/__main__.py

@@ -28,6 +28,8 @@
 from slither.utils.output_capture import StandardOutputCapture
 from slither.utils.colors import red, set_colorization_enabled
 from slither.utils.command_line import (
+    FailOnLevel,
+    migrate_config_options,
     output_detectors,
     output_results_to_markdown,
     output_detectors_json,
@@ -220,22 +222,22 @@ def choose_detectors(
         detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT)
         return detectors_to_run
 
-    if args.exclude_optimization and not args.fail_pedantic:
+    if args.exclude_optimization:
         detectors_to_run = [
             d for d in detectors_to_run if d.IMPACT != DetectorClassification.OPTIMIZATION
         ]
 
-    if args.exclude_informational and not args.fail_pedantic:
+    if args.exclude_informational:
         detectors_to_run = [
             d for d in detectors_to_run if d.IMPACT != DetectorClassification.INFORMATIONAL
         ]
-    if args.exclude_low and not args.fail_low:
+    if args.exclude_low:
         detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.LOW]
-    if args.exclude_medium and not args.fail_medium:
+    if args.exclude_medium:
         detectors_to_run = [
             d for d in detectors_to_run if d.IMPACT != DetectorClassification.MEDIUM
         ]
-    if args.exclude_high and not args.fail_high:
+    if args.exclude_high:
         detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.HIGH]
     if args.detectors_to_exclude:
         detectors_to_run = [
@@ -401,41 +403,44 @@ def parse_args(
         default=defaults_flag_in_config["exclude_high"],
     )
 
-    group_detector.add_argument(
+    fail_on_group = group_detector.add_mutually_exclusive_group()
+    fail_on_group.add_argument(
         "--fail-pedantic",
-        help="Return the number of findings in the exit code",
-        action="store_true",
-        default=defaults_flag_in_config["fail_pedantic"],
+        help="Fail if any findings are detected",
+        action="store_const",
+        dest="fail_on",
+        const=FailOnLevel.PEDANTIC,
     )
-
-    group_detector.add_argument(
-        "--no-fail-pedantic",
-        help="Do not return the number of findings in the exit code. Opposite of --fail-pedantic",
-        dest="fail_pedantic",
-        action="store_false",
-        required=False,
-    )
-
-    group_detector.add_argument(
+    fail_on_group.add_argument(
         "--fail-low",
-        help="Fail if low or greater impact finding is detected",
-        action="store_true",
-        default=defaults_flag_in_config["fail_low"],
+        help="Fail if any low or greater impact findings are detected",
+        action="store_const",
+        dest="fail_on",
+        const=FailOnLevel.LOW,
     )
-
-    group_detector.add_argument(
+    fail_on_group.add_argument(
         "--fail-medium",
-        help="Fail if medium or greater impact finding is detected",
-        action="store_true",
-        default=defaults_flag_in_config["fail_medium"],
+        help="Fail if any medium or greater impact findings are detected",
+        action="store_const",
+        dest="fail_on",
+        const=FailOnLevel.MEDIUM,
     )
-
-    group_detector.add_argument(
+    fail_on_group.add_argument(
         "--fail-high",
-        help="Fail if high impact finding is detected",
-        action="store_true",
-        default=defaults_flag_in_config["fail_high"],
+        help="Fail if any high impact findings are detected",
+        action="store_const",
+        dest="fail_on",
+        const=FailOnLevel.HIGH,
+    )
+    fail_on_group.add_argument(
+        "--fail-none",
+        "--no-fail-pedantic",
+        help="Do not return the number of findings in the exit code",
+        action="store_const",
+        dest="fail_on",
+        const=FailOnLevel.NONE,
     )
+    fail_on_group.set_defaults(fail_on=FailOnLevel.PEDANTIC)
 
     group_detector.add_argument(
         "--show-ignored-findings",
@@ -910,17 +915,18 @@ def main_impl(
         stats = pstats.Stats(cp).sort_stats("cumtime")
         stats.print_stats()
 
-    if args.fail_high:
+    fail_on = FailOnLevel(args.fail_on)
+    if fail_on == FailOnLevel.HIGH:
         fail_on_detection = any(result["impact"] == "High" for result in results_detectors)
-    elif args.fail_medium:
+    elif fail_on == FailOnLevel.MEDIUM:
         fail_on_detection = any(
             result["impact"] in ["Medium", "High"] for result in results_detectors
         )
-    elif args.fail_low:
+    elif fail_on == FailOnLevel.LOW:
         fail_on_detection = any(
             result["impact"] in ["Low", "Medium", "High"] for result in results_detectors
         )
-    elif args.fail_pedantic:
+    elif fail_on == FailOnLevel.PEDANTIC:
         fail_on_detection = bool(results_detectors)
     else:
         fail_on_detection = False

slither/utils/command_line.py

@@ -1,4 +1,5 @@
 import argparse
+import enum
 import json
 import os
 import re
@@ -27,6 +28,15 @@
     "list-printers",
 ]
 
+
+class FailOnLevel(enum.Enum):
+    PEDANTIC = "pedantic"
+    LOW = "low"
+    MEDIUM = "medium"
+    HIGH = "high"
+    NONE = "none"
+
+
 # Those are the flags shared by the command line and the config file
 defaults_flag_in_config = {
     "detectors_to_run": "all",
@@ -38,10 +48,7 @@
     "exclude_low": False,
     "exclude_medium": False,
     "exclude_high": False,
-    "fail_pedantic": True,
-    "fail_low": False,
-    "fail_medium": False,
-    "fail_high": False,
+    "fail_on": FailOnLevel.PEDANTIC,
     "json": None,
     "sarif": None,
     "json-types": ",".join(DEFAULT_JSON_OUTPUT_TYPES),
@@ -57,6 +64,13 @@
     **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE,
 }
 
+deprecated_flags = {
+    "fail_pedantic": True,
+    "fail_low": False,
+    "fail_medium": False,
+    "fail_high": False,
+}
+
 
 def read_config_file(args: argparse.Namespace) -> None:
     # No config file was provided as an argument
@@ -73,6 +87,12 @@ def read_config_file(args: argparse.Namespace) -> None:
             with open(args.config_file, encoding="utf8") as f:
                 config = json.load(f)
                 for key, elem in config.items():
+                    if key in deprecated_flags:
+                        logger.info(
+                            yellow(f"{args.config_file} has a deprecated key: {key} : {elem}")
+                        )
+                        migrate_config_options(args, key, elem)
+                        continue
                     if key not in defaults_flag_in_config:
                         logger.info(
                             yellow(f"{args.config_file} has an unknown key: {key} : {elem}")
@@ -87,6 +107,28 @@ def read_config_file(args: argparse.Namespace) -> None:
         logger.error(yellow("Falling back to the default settings..."))
 
 
+def migrate_config_options(args: argparse.Namespace, key: str, elem):
+    if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]:
+        if key == "fail_pedantic":
+            pedantic_setting = elem
+            fail_on = pedantic_setting and FailOnLevel.PEDANTIC or FailOnLevel.NONE
+            setattr(args, "fail_on", fail_on)
+            logger.info(
+                "Migrating fail_pedantic: {} as fail_on: {}".format(pedantic_setting, fail_on.value)
+            )
+        elif key == "fail_low" and elem == True:
+            logger.info("Migrating fail_low: true -> fail_on: low")
+            setattr(args, "fail_on", FailOnLevel.LOW)
+        elif key == "fail_medium" and elem == True:
+            logger.info("Migrating fail_medium: true -> fail_on: medium")
+            setattr(args, "fail_on", FailOnLevel.MEDIUM)
+        elif key == "fail_high" and elem == True:
+            logger.info("Migrating fail_high: true -> fail_on: high")
+            setattr(args, "fail_on", FailOnLevel.HIGH)
+        else:
+            logger.warn(yellow("Key {} was deprecated but no migration was provided".format(key)))
+
+
 def output_to_markdown(
     detector_classes: List[Type[AbstractDetector]],
     printer_classes: List[Type[AbstractPrinter]],