Browse Source

Teach practracker about .h files

I'm using 500 as a file size limit, and 15 as an include limit.
This affects comparatively few files, but I think they are the worst
ones.

Closes ticket 31175.
Nick Mathewson 4 years ago
parent
commit
9a1e9b1d6c

+ 3 - 0
changes/ticket31175

@@ -0,0 +1,3 @@
+  o Minor features (development tools):
+    - Our best-practices tracker now looks at headers as well as
+      C files. Closes ticket 31175.

+ 3 - 1
scripts/maint/practracker/metrics.py

@@ -27,7 +27,9 @@ def get_function_lines(f):
 
     # Skip lines that look like they are defining functions with these
     # names: they aren't real function definitions.
-    REGEXP_CONFUSE_TERMS = {"MOCK_IMPL", "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", "DUMMY_TYPECHECK_INSTANCE",
+    REGEXP_CONFUSE_TERMS = {"MOCK_IMPL", "MOCK_DECL", "HANDLE_DECL",
+                            "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING",
+                            "DUMMY_TYPECHECK_INSTANCE",
                             "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"}
 
     in_function = False

+ 14 - 4
scripts/maint/practracker/practracker.py

@@ -35,6 +35,10 @@ MAX_FILE_SIZE = 3000 # lines
 MAX_FUNCTION_SIZE = 100 # lines
 # Recommended number of #includes
 MAX_INCLUDE_COUNT = 50
+# Recommended file size for headers
+MAX_H_FILE_SIZE = 500
+# Recommended include count for headers
+MAX_H_INCLUDE_COUNT = 15
 
 # Map from problem type to functions that adjust for tolerance
 TOLERANCE_FNS = {
@@ -161,8 +165,12 @@ def main(argv):
                         help="Make all warnings into errors")
     parser.add_argument("--terse", action="store_true",
                         help="Do not emit helpful instructions.")
+    parser.add_argument("--max-h-file-size", default=MAX_H_FILE_SIZE,
+                        help="Maximum lines per .H file")
+    parser.add_argument("--max-h-include-count", default=MAX_H_INCLUDE_COUNT,
+                        help="Maximum includes per .H file")
     parser.add_argument("--max-file-size", default=MAX_FILE_SIZE,
-                        help="Maximum lines per C file size")
+                        help="Maximum lines per C file")
     parser.add_argument("--max-include-count", default=MAX_INCLUDE_COUNT,
                         help="Maximum includes per C file")
     parser.add_argument("--max-function-size", default=MAX_FUNCTION_SIZE,
@@ -180,9 +188,11 @@ def main(argv):
 
     # 0) Configure our thresholds of "what is a problem actually"
     filt = problem.ProblemFilter()
-    filt.addThreshold(problem.FileSizeItem("*", int(args.max_file_size)))
-    filt.addThreshold(problem.IncludeCountItem("*", int(args.max_include_count)))
-    filt.addThreshold(problem.FunctionSizeItem("*", int(args.max_function_size)))
+    filt.addThreshold(problem.FileSizeItem("*.c", int(args.max_file_size)))
+    filt.addThreshold(problem.IncludeCountItem("*.c", int(args.max_include_count)))
+    filt.addThreshold(problem.FileSizeItem("*.h", int(args.max_h_file_size)))
+    filt.addThreshold(problem.IncludeCountItem("*.h", int(args.max_h_include_count)))
+    filt.addThreshold(problem.FunctionSizeItem("*.c", int(args.max_function_size)))
 
     # 1) Get all the .c files we care about
     files_list = util.get_tor_c_files(TOR_TOPDIR)

+ 9 - 2
scripts/maint/practracker/problem.py

@@ -108,10 +108,11 @@ class ProblemFilter(object):
         self.thresholds = dict()
 
     def addThreshold(self, item):
-        self.thresholds[item.get_type()] = item
+        self.thresholds[(item.get_type(),item.get_file_type())] = item
 
     def matches(self, item):
-        filt = self.thresholds.get(item.get_type(), None)
+        key = (item.get_type(), item.get_file_type())
+        filt = self.thresholds.get(key, None)
         if filt is None:
             return False
         return item.is_worse_than(filt)
@@ -158,6 +159,12 @@ class Item(object):
     def get_type(self):
         return self.problem_type
 
+    def get_file_type(self):
+        if self.problem_location.endswith(".h"):
+            return "*.h"
+        else:
+            return "*.c"
+
 class FileSizeItem(Item):
     """
     Denotes a problem with the size of a .c file.

+ 7 - 3
scripts/maint/practracker/util.py

@@ -5,12 +5,14 @@ import os
 EXCLUDE_SOURCE_DIRS = {"src/test/", "src/trunnel/", "src/rust/",
                        "src/ext/", ".git/"}
 
+EXCLUDE_FILES = {"orconfig.h"}
+
 def _norm(p):
     return os.path.normcase(os.path.normpath(p))
 
 def get_tor_c_files(tor_topdir):
     """
-    Return a list with the .c filenames we want to get metrics of.
+    Return a list with the .c and .h filenames we want to get metrics of.
     """
     files_list = []
     exclude_dirs = { _norm(os.path.join(tor_topdir, p)) for p in EXCLUDE_SOURCE_DIRS }
@@ -23,8 +25,10 @@ def get_tor_c_files(tor_topdir):
         directories.sort()
         filenames.sort()
         for filename in filenames:
-            # We only care about .c files
-            if not filename.endswith(".c"):
+            # We only care about .c and .h files
+            if not (filename.endswith(".c") or filename.endswith(".h")):
+                continue
+            if filename in EXCLUDE_FILES:
                 continue
 
             full_path = os.path.join(root,filename)