Merge pull request #3263 from duncankl/refactor-test-tidy

Refactor python/tidy.py (updated post-cargoify)

3 files changed, 79 insertions, 99 deletions

diff --git a/python/licenseck.py b/python/licenseck.py
index 0e862e35ef4..0b2f6700c82 100644
--- a/python/licenseck.py
+++ b/python/licenseck.py
@@ -7,25 +7,29 @@
 # option. This file may not be copied, modified, or distributed
 # except according to those terms.
 
-license0="""\
+
+# These licenses are valid for use in Servo
+licenses = [
+
+"""\
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-"""
+""",
 
-license1="""\
+"""\
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
-"""
+""",
 
-license2="""\
+"""\
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
-"""
+""",
 
-license3 = """\
+"""\
 // Copyright 2013 The Servo Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution.
 //
@@ -34,9 +38,9 @@ license3 = """\
 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
-"""
+""",
 
-license4 = """\
+"""\
 # Copyright 2013 The Servo Project Developers. See the COPYRIGHT
 # file at the top-level directory of this distribution.
 #
@@ -45,31 +49,6 @@ license4 = """\
 # <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 # option. This file may not be copied, modified, or distributed
 # except according to those terms.
-"""
+""",
 
-licenses = [license0, license1, license2, license3, license4]
-
-exceptions = [
-    "servo/dom/bindings/codegen/ply/ply/yacc.py", # BSD
-    "servo/dom/bindings/codegen/ply/ply/__init__.py", # BSD
-    "servo/dom/bindings/codegen/ply/ply/lex.py", # BSD
 ]
-
-def check_license(name, contents):
-    valid_license = False
-    for a_valid_license in licenses:
-        if contents.startswith(a_valid_license):
-            valid_license = True
-            break
-    if valid_license:
-        return True
-
-    for exception in exceptions:
-        if name.endswith(exception):
-            return True
-
-    firstlineish = contents[:100]
-    if firstlineish.find("xfail-license") != -1:
-        return True
-
-    return False
diff --git a/python/servo/testing_commands.py b/python/servo/testing_commands.py
index 19ecc4ef4cf..e9e46c30c69 100644
--- a/python/servo/testing_commands.py
+++ b/python/servo/testing_commands.py
@@ -105,11 +105,7 @@ class MachCommands(CommandBase):
              description='Run the source code tidiness check',
              category='testing')
     def test_tidy(self):
-        errors = 0
-        for p in ["src", "components"]:
-            ret = tidy.scan(path.join(self.context.topdir, p))
-            if ret != 0: errors = 1
-        return errors
+        return tidy.scan()
 
     @Command('test-wpt',
              description='Run the web platform tests',
diff --git a/python/tidy.py b/python/tidy.py
index 04743433dbf..0fc71ac3134 100644
--- a/python/tidy.py
+++ b/python/tidy.py
@@ -10,82 +10,87 @@
 #!/usr/bin/env python
 
 import os
-from licenseck import check_license
+import fnmatch
+from licenseck import licenses
 
-# FIXME(#3242): Don't use globals
-err = 0
+directories_to_check = ["src", "components"]
+filetypes_to_check = [".rs", ".rc", ".cpp", ".c", ".h", ".py"]
 
+ignored_files = [
+    # Upstream
+    "support/*",
+    "tests/wpt/web-platform-tests/*",
 
-def report_error_name_no(name, no, s):
-    global err
-    print("%s:%d: %s" % (name, no, s))
-    err = 1
+    # Generated and upstream code combined with our own. Could use cleanup
+    "components/script/dom/bindings/codegen/*",
+    "components/style/properties/mod.rs",
+]
 
 
-def do_license_check(name, contents):
-    if not check_license(name, contents):
-        report_error_name_no(name, 1, "incorrect license")
+def collect_file_names(top_directories):
+    for top_directory in top_directories:
+        for dirname, dirs, files in os.walk(top_directory):
+            for basename in files:
+                yield os.path.join(dirname, basename)
 
 
-def do_whitespace_check(name, contents):
-    for idx, line in enumerate(contents):
-        if line[-1] == "\n":
-            line = line[:-1]
-        else:
-            report_error_name_no(name, idx + 1, "No newline at EOF")
+def should_check(file_name):
+    if ".#" in file_name:
+        return False
+    if os.path.splitext(file_name)[1] not in filetypes_to_check:
+        return False
+    for pattern in ignored_files:
+        if fnmatch.fnmatch(file_name, pattern):
+            return False
+    return True
 
-        if line.endswith(' '):
-            report_error_name_no(name, idx + 1, "trailing whitespace")
 
-        if '\t' in line:
-            report_error_name_no(name, idx + 1, "tab on line")
+def check_license(contents):
+    valid_license = any(contents.startswith(license) for license in licenses)
+    acknowledged_bad_license = "xfail-license" in contents[:100]
+    if not (valid_license or acknowledged_bad_license):
+        yield (1, "incorrect license")
 
-        if '\r' in line:
-            report_error_name_no(name, idx + 1, "CR on line")
 
+def check_whitespace(contents):
+    lines = contents.splitlines(True)
+    for idx, line in enumerate(lines):
+        if line[-1] == "\n":
+            line = line[:-1]
+        else:
+            yield (idx + 1, "no newline at EOF")
 
-exceptions = [
-    # Upstream
-    "support",
-    "tests/wpt/web-platform-tests",
+        if line.endswith(" "):
+            yield (idx + 1, "trailing whitespace")
 
-    # Generated and upstream code combined with our own. Could use cleanup
-    "components/script/dom/bindings/codegen",
-    "components/style/properties/mod.rs",
-]
+        if "\t" in line:
+            yield (idx + 1, "tab on line")
 
+        if "\r" in line:
+            yield (idx + 1, "CR on line")
 
-def should_check(name):
-    if ".#" in name:
-        return False
-    if not (name.endswith(".rs")
-            or name.endswith(".rc")
-            or name.endswith(".cpp")
-            or name.endswith(".c")
-            or name.endswith(".h")
-            or name.endswith(".py")):
-        return False
-    for exception in exceptions:
-        if exception in name:
-            return False
-    return True
 
+def collect_errors_for_files(files_to_check, checking_functions):
+    for file_name in files_to_check:
+        with open(file_name, "r") as fp:
+            contents = fp.read()
+            for check in checking_functions:
+                for error in check(contents):
+                    # filename, line, message
+                    yield (file_name, error[0], error[1])
 
-def scan(start_path):
-    global err
-    err = 0
 
-    file_names = []
-    for root, dirs, files in os.walk(start_path):
-        for myfile in files:
-            file_name = root + "/" + myfile
-            if should_check(file_name):
-                file_names.append(file_name)
+def scan():
+    all_files = collect_file_names(directories_to_check)
+    files_to_check = filter(should_check, all_files)
 
-    for path in file_names:
-        with open(path, "r") as fp:
-            lines = fp.readlines()
-            do_license_check(path, "".join(lines))
-            do_whitespace_check(path, lines)
+    checking_functions = [check_license, check_whitespace]
+    errors = collect_errors_for_files(files_to_check, checking_functions)
+    errors = list(errors)
 
-    return err
+    if errors:
+        for error in errors:
+            print("{}:{}: {}".format(*error))
+        return 1
+    else:
+        return 0