[tor-commits] [tor/master] practracker: add a --regen-overbroad option to remove overbroad exceptions.

[nickm at torproject.org]

commit 2542a24b63f9d5c7dfd05932fc3b2f05cd720024
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 3 13:02:12 2020 -0500

    practracker: add a --regen-overbroad option to remove overbroad exceptions.
    
    Closes ticket 32372.
---
 changes/ticket32372                      |  4 ++++
 scripts/maint/practracker/practracker.py | 27 ++++++++++++++++++++++-----
 scripts/maint/practracker/problem.py     | 21 +++++++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/changes/ticket32372 b/changes/ticket32372
new file mode 100644
index 000000000..ed2d38b40
--- /dev/null
+++ b/changes/ticket32372
@@ -0,0 +1,4 @@
+  o Minor features (best practices tracker):
+    - Practracker now supports a --regen-overbroad option to regenerate
+      the exceptions file, but only to revise exceptions to be _less_
+      tolerant of best-practices violations.  Closes ticket 32372.
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index e806aef3b..3f40f63c4 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -185,6 +185,9 @@ def main(argv):
                         help="Regenerate the exceptions file")
     parser.add_argument("--list-overbroad", action="store_true",
                         help="List over-broad exceptions")
+    parser.add_argument("--regen-overbroad", action="store_true",
+                        help="Regenerate the exceptions file, "
+                             "removing over-broad exceptions.")
     parser.add_argument("--exceptions",
                         help="Override the location for the exceptions file")
     parser.add_argument("--strict", action="store_true",
@@ -227,8 +230,9 @@ def main(argv):
     filt.addThreshold(problem.DependencyViolationItem("*.c", int(args.max_dependency_violations)))
     filt.addThreshold(problem.DependencyViolationItem("*.h", int(args.max_dependency_violations)))
 
-    if args.list_overbroad and args.regen:
-        print("Cannot use --regen with --list-overbroad",
+    if args.list_overbroad + args.regen + args.regen_overbroad > 1:
+        print("Cannot use more than one of --regen, --list-overbroad, and "
+              "--regen-overbroad.",
               file=sys.stderr)
         sys.exit(1)
 
@@ -247,13 +251,15 @@ def main(argv):
         ProblemVault = problem.ProblemVault(exceptions_file)
         problem_file = sys.stdout
 
-    if args.list_overbroad:
-        # If we're listing overbroad exceptions, don't list problems.
+    if args.list_overbroad or args.regen_overbroad:
+        # If we're looking for overbroad exceptions, don't list problems
+        # immediately to the problem file.
         problem_file = util.NullFile()
 
     # 2.1) Adjust the exceptions so that we warn only about small problems,
     # and produce errors on big ones.
-    if not (args.regen or args.list_overbroad or args.strict):
+    if not (args.regen or args.list_overbroad or args.regen_overbroad or
+            args.strict):
         ProblemVault.set_tolerances(TOLERANCE_FNS)
 
     # 3) Go through all the files and report problems if they are not exceptions
@@ -272,6 +278,16 @@ def main(argv):
         os.rename(tmpname, exceptions_file)
         sys.exit(0)
 
+    if args.regen_overbroad:
+        tmpname = exceptions_file + ".tmp"
+        tmpfile = open(tmpname, "w")
+        tmpfile.write(HEADER)
+        for item in ProblemVault.list_exceptions_without_overbroad():
+            print(item, file=tmpfile)
+        tmpfile.close()
+        os.rename(tmpname, exceptions_file)
+        sys.exit(0)
+
     # If new issues were found, try to give out some advice to the developer on how to resolve it.
     if found_new_issues and not args.regen and not args.terse:
         new_issues_str = """\
@@ -296,6 +312,7 @@ variable.
             else:
                 print(ex, "->", p.metric_value)
 
+
     sys.exit(found_new_issues)
 
 if __name__ == '__main__':
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index bee5eeb90..a3255dcc8 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -29,6 +29,8 @@ class ProblemVault(object):
     def __init__(self, exception_fname=None):
         # Exception dictionary: { problem.key() : Problem object }
         self.exceptions = {}
+        # Exception list: list of Problem objects, in the order added.
+        self.exception_list = []
         # Exception dictionary: maps key to the problem it was used to
         # suppress.
         self.used_exception_for = {}
@@ -63,6 +65,7 @@ class ProblemVault(object):
                 sys.exit(1)
 
             self.exceptions[problem.key()] = problem
+            self.exception_list.append(problem)
             #print "Registering exception: %s" % problem
 
     def register_problem(self, problem):
@@ -98,6 +101,24 @@ class ProblemVault(object):
             if p is None or e.is_worse_than(p):
                 yield (e, p)
 
+    def list_exceptions_without_overbroad(self):
+        """Return an iterator of new problems, such that overbroad
+           exceptions are replaced with minimally broad versions, or removed.
+        """
+        for e in self.exception_list:
+            p = self.used_exception_for.get(e.key())
+            if p is None:
+                # This exception wasn't needed at all.
+                continue
+            if e.is_worse_than(p):
+                # The exception is worse than the problem we found.
+                # Yield the problem as the new exception value.
+                yield p
+            else:
+                # The problem is as bad as the exception, or worse.
+                # Yield the exception.
+                yield e
+
     def set_tolerances(self, fns):
         """Adjust the tolerances for the exceptions in this vault.  Takes
            a map of problem type to a function that adjusts the permitted