revert differentiation of empty allow/ignore list from undefined list

Restore earlier behavior that treated empty list `[]` as “include
everything” for compatibility with existing deployments.

Author: yasunariw

documentation/config_docs.md

@@ -80,18 +80,28 @@ A json object with fields of bools for each status type.
     "default_channel": "default",
     "rules": [
         {
-            "allow": ["backend"],
+            "allow": [
+                "backend"
+            ],
+            "ignore": [],
             "channel": "backend"
         },
         {
-            "allow": ["a1"],
+            "allow": [
+                "a1"
+            ],
+            "ignore": [],
             "channel": "a1-bot"
         },
         {
-            "allow": ["a3"],
+            "allow": [
+                "a3"
+            ],
+            "ignore": [],
             "channel": "a3"
         },
         {
+            "allow": [],
             "ignore": [
                 "backend",
                 "a1",
@@ -110,12 +120,12 @@ A json object with fields of bools for each status type.
 
 ### Label Rule
 
-A **label rule** specifies whether or not a Slack channel should be notified, based on the labels present in the given payload. For each rule, `ignore` is a blacklist of labels that should not notify the rule's channel, and `allow` is a whitelist of labels that should. The `ignore` list takes precedence over the `allow` list. Both are optional; if neither are provided, the rule will always generate a notification for its channel.
+A **label rule** specifies whether or not a Slack channel should be notified, based on the labels present in the given payload. For each rule, `ignore` is a blacklist of labels that should not notify the rule's channel, and `allow` is a whitelist of labels that should. If a label exists in both lists, the `ignore` list takes precedence. If an empty `ignore` list is provided, nothing is ignored. If an empty `allow` list is provided, everything is allowed. Both are optional; if neither are provided, the rule will always generate a notification for its channel.
 
 | value | description | optional | default |
 |-|-|-|-|
-| `allow` | whitelist of labels that should match the rule | Yes | all labels allowed if no list provided |
-| `ignore` | blacklist of labels that shouldn't match the rule | Yes | - |
+| `allow` | if notifications match any label in this list, they should be routed to the channel | Yes | all labels allowed if no list provided |
+| `ignore` | if notifications match any label in this list, they shouldn't be routed to the channel (even if they match any allow labels) | Yes | - |
 | `channel` | channel to use as webhook if the rule is matched | No | - |
 
 ## Prefix Options
@@ -128,17 +138,23 @@ A **label rule** specifies whether or not a Slack channel should be notified, ba
     "default_channel": "default",
     "rules": [
         {
-            "allow": ["backend/a1"],
+            "allow": [
+                "backend/a1"
+            ],
+            "ignore": [],
             "channel": "a1"
         },
         {
             "allow": [
                 "backend/a5",
                 "backend/a4"
             ],
+            "ignore": [],
             "channel": "backend"
         },
         {
+            "allow": [],
+            "ignore": [],
             "channel": "all-push-events"
         }
     ]
@@ -151,6 +167,6 @@ A **prefix rule** specifies whether or not a Slack channel should be notified, b
 
 | value | description | optional | default |
 |-|-|-|-|
-| `allow` | whitelist of file prefixes that should match the rule | Yes | all prefixes allowed if no list provided |
-| `ignore` | blacklist of file prefixes that shouldn't match the rule | Yes | - |
+| `allow` | if commit files match any prefix in this list, they should be routed to the channel | Yes | all prefixes allowed if no list provided |
+| `ignore` | if commit files match any prefix in this list, they shouldn't be routed to the channel (even if they match any allow prefixes) | Yes | - |
 | `channel` | channel to use as webhook if the rule is matched | No | - |

lib/rule.ml

@@ -42,8 +42,8 @@ module Prefix = struct
   (** `match_rules f rs` returns the channel name of a rule in `rs` that matches
       file name `f` with the longest prefix, if one exists. A rule `r` matches
       `f` with prefix length `l`, if `f` has no prefix in `r.ignore` and `l` is
-      the length of the longest prefix of `f` in `r.allow`. An undefined allow
-      list is considered a prefix match of length 0. The ignore list is
+      the length of the longest prefix of `f` in `r.allow`. An undefined or empty
+      allow list is considered a prefix match of length 0. The ignore list is
       evaluated before the allow list. *)
   let match_rules filename ~rules =
     let compare a b = Int.compare (snd a) (snd b) in
@@ -53,7 +53,7 @@ module Prefix = struct
       | Some ignore_list when List.exists ignore_list ~f:is_prefix -> None
       | _ ->
       match rule.allow with
-      | None -> Some (rule, 0)
+      | None | Some [] -> Some (rule, 0)
       | Some allow_list ->
         allow_list
         |> List.filter_map ~f:(fun p -> if is_prefix p then Some (rule, String.length p) else None)
@@ -94,7 +94,7 @@ module Label = struct
       | Some ignore_list when List.exists ignore_list ~f:label_name_equal -> None
       | _ ->
       match rule.allow with
-      | None -> Some rule.channel_name
+      | None | Some [] -> Some rule.channel_name
       | Some allow_list -> if List.exists allow_list ~f:label_name_equal then Some rule.channel_name else None
     in
     rules |> List.filter_map ~f:match_rule |> List.dedup_and_sort ~compare:String.compare

test/notabot.json

@@ -20,22 +20,27 @@
                 "allow": [
                     "backend/a1"
                 ],
+                "ignore": [],
                 "channel": "a1"
             },
             {
                 "allow": [
                     "backend/a1/longest"
                 ],
+                "ignore": [],
                 "channel": "longest-a1"
             },
             {
                 "allow": [
                     "backend/a5",
                     "backend/a4"
                 ],
+                "ignore": [],
                 "channel": "backend"
             },
             {
+                "allow": [],
+                "ignore": [],
                 "channel": "all-push-events"
             }
         ]
@@ -47,21 +52,25 @@
                 "allow": [
                     "backend"
                 ],
+                "ignore": [],
                 "channel": "backend"
             },
             {
                 "allow": [
                     "a1"
                 ],
+                "ignore": [],
                 "channel": "a1-bot"
             },
             {
                 "allow": [
                     "a3"
                 ],
+                "ignore": [],
                 "channel": "a3"
             },
             {
+                "allow": [],
                 "ignore": [
                     "backend",
                     "a1",