Convert tidy to a non-egg Python package

It seems that servo-tidy is only used by webrender in my GitHub
searches. WebRender could simply use `rustfmt` and the tidy on pypi
hasn't been updated since 2018. Converting tidy to a normal Python
package removes the maintenance burden of continually fixing the easy
install configuration.

Fixes #29094.
Fixes #29334.

python/mach_bootstrap.py

@@ -14,7 +14,6 @@ from subprocess import Popen
 from tempfile import TemporaryFile
 
 SEARCH_PATHS = [
-    os.path.join("python", "tidy"),
     os.path.join("python", "mach"),
 ]
 

python/requirements.txt

@@ -39,4 +39,6 @@ notify-py == 0.3.42
 # For formatting C++ files.
 clang-format ~= 16.0.0
 
--e python/tidy
+# A few more requirements for tidy.
+voluptuous == 0.12.1
+PyYAML == 5.4

python/servo/lints/wpt_lint.py

@@ -1,45 +0,0 @@
-# Copyright 2013 The Servo Project Developers. See the COPYRIGHT
-# file at the top-level directory of this distribution.
-#
-# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-# <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.
-
-from __future__ import print_function
-
-import os
-import sys
-
-from servo_tidy.tidy import LintRunner, filter_file
-
-WPT_PATH = os.path.join(".", "tests", "wpt")
-SUITES = ["web-platform-tests", os.path.join("mozilla", "tests")]
-
-
-class Lint(LintRunner):
-    def _get_wpt_files(self, suite):
-        working_dir = os.path.join(WPT_PATH, suite, '')
-        file_iter = self.get_files(working_dir, exclude_dirs=[])
-        print('\nRunning the WPT lint on %s...' % working_dir)
-        for f in file_iter:
-            if filter_file(f):
-                yield f[len(working_dir):]
-
-    def run(self):
-        if self.stylo or self.no_wpt:
-            return
-
-        wpt_working_dir = os.path.abspath(os.path.join(WPT_PATH, "web-platform-tests"))
-        for suite in SUITES:
-            files = list(self._get_wpt_files(suite))
-            if not files:
-                continue
-            sys.path.insert(0, wpt_working_dir)
-            from tools.lint import lint
-            file_dir = os.path.abspath(os.path.join(WPT_PATH, suite))
-            returncode = lint.lint(file_dir, files, output_format="json")
-            sys.path.remove(wpt_working_dir)
-            if returncode:
-                yield ("WPT Lint Tool", "", "lint error(s) in Web Platform Tests: exit status %s" % returncode)

python/servo/testing_commands.py

@@ -31,14 +31,12 @@ from mach.decorators import (
     CommandProvider,
     Command,
 )
+import tidy
 
-from servo_tidy import tidy
 from servo.command_base import (
     CommandBase,
     call, check_call, check_output,
 )
-from servo_tidy_tests import test_tidy
-
 from servo.util import delete
 from distutils.dir_util import copy_tree
 
@@ -298,7 +296,7 @@ class MachCommands(CommandBase):
                      help="Only handle files in the stylo tree")
     def test_tidy(self, all_files, no_progress, self_test, stylo, force_cpp=False, no_wpt=False):
         if self_test:
-            return test_tidy.do_tests()
+            return tidy.do_tests()
         else:
             if no_wpt:
                 manifest_dirty = False

python/tidy/HISTORY.rst

@@ -1,31 +0,0 @@
-Release History
----------------
-
-0.3.0 (2017-02-24)
-++++++++++++++++++
-
-- Added checking for blocked packages
-- Added checking for alphabetical order for JSON keys and ``#![feature(...)]``
-- Improved duplicated packages checking
-- Improved ordering ``use`` statements
-- Allow tidy to run custom project-specific lints
-- And many other improvements
-
-0.2.0 (2016-08-09)
-++++++++++++++++++
-
-- Improve license checking to disregard comments and line breaks
-- License checking verifies that COPYRIGHT is specified when apache2 is used
-
-0.0.3 (2016-04-19)
-++++++++++++++++++
-
-- Add alternate wording of apache2 license
-
-0.0.2 (2016-04-17)
-++++++++++++++++++
-- Cleanup Tidy to work on external deps
-
-0.0.1 (2016-04-12)
-++++++++++++++++++
-- Package Tidy

python/tidy/Makefile

@@ -1,25 +0,0 @@
-PYTHON := python
-VENV := env-$(PYTHON)
-
-# for travis
-
-$(VENV)/bin/python:
-	[ -d $(VENV) ] || $(PYTHON) -m virtualenv $(VENV) || virtualenv $(VENV)
-	$(VENV)/bin/pip install --upgrade setuptools
-	$(VENV)/bin/python setup.py develop
-
-
-.PHONY: dev-env
-dev-env: $(VENV)/bin/python
-
-
-# for testing
-.PHONY: test
-test: dev-env
-	$(VENV)/bin/python -m unittest discover -s servo_tidy_tests -v
-
-
-.PHONY: clean
-clean:
-	find . -name "*.pyc" -type f -delete
-	rm -rf $(VENV)

python/tidy/README.rst

@@ -1,26 +0,0 @@
-servo_tidy
-==========
-
-Servo's code and license checker.
-
-Installation
-++++++++++++
-
-Install from PyPI:
-
-::
-
-  pip install servo-tidy
-
-or from git:
-
-::
-
-  pip install -e git+https://github.com/servo/servo.git#egg=servo_tidy&subdirectory=python/tidy
-
-To run the tests
-++++++++++++++++
-
-::
-
-  ./mach test-tidy --self-test

python/tidy/__init__.py

@@ -6,3 +6,6 @@
 # <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.
+
+from .tidy import scan  # noqa
+from .test import do_tests  # noqa

python/tidy/setup.py

@@ -1,60 +0,0 @@
-# Copyright 2013 The Servo Project Developers. See the COPYRIGHT
-# file at the top-level directory of this distribution.
-#
-# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-# <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.
-
-import os
-from setuptools import setup, find_packages
-
-
-VERSION = '0.3.0'
-
-install_requires = [
-    "flake8==3.8.3",
-    "toml==0.9.2",
-    "colorama==0.3.7",
-    "voluptuous==0.12.1",
-    "PyYAML==5.4",
-]
-
-here = os.path.dirname(os.path.abspath(__file__))
-# get documentation from the README and HISTORY
-try:
-    with open(os.path.join(here, 'README.rst')) as doc:
-        readme = doc.read()
-except Exception:
-    readme = ''
-
-try:
-    with open(os.path.join(here, 'HISTORY.rst')) as doc:
-        history = doc.read()
-except Exception:
-    history = ''
-
-long_description = readme + '\n\n' + history
-
-if __name__ == '__main__':
-    setup(
-        name='servo_tidy',
-        version=VERSION,
-        description='The servo-tidy is used to check licenses, '
-                    'line lengths, whitespace, flake8 on Python files, lock file versions, and more.',
-        long_description=long_description,
-        keywords='mozilla servo tidy ',
-        author='The Servo Project Developers',
-        author_email='dev-servo@lists.mozilla.org',
-        url='https://github.com/servo/servo',
-        packages=find_packages(exclude=['ez_setup', 'examples', 'tests']),
-        package_data={},
-        install_requires=install_requires,
-        zip_safe=False,
-        entry_points={
-            'console_scripts': [
-                'servo-tidy=servo_tidy.tidy:scan',
-            ],
-        },
-    )

python/tidy/test.py

@@ -10,13 +10,14 @@
 import os
 import unittest
 
-from servo_tidy import tidy
+from . import tidy
 
-base_path = 'servo_tidy_tests/' if os.path.exists('servo_tidy_tests/') else 'python/tidy/servo_tidy_tests/'
+
+BASE_PATH = 'python/tidy/tests/'
 
 
 def iterFile(name):
-    return iter([os.path.join(base_path, name)])
+    return iter([os.path.join(BASE_PATH, name)])
 
 
 class CheckTidiness(unittest.TestCase):
@@ -25,7 +26,7 @@ class CheckTidiness(unittest.TestCase):
             next(errors)
 
     def test_tidy_config(self):
-        errors = tidy.check_config_file(os.path.join(base_path, 'servo-tidy.toml'), print_text=False)
+        errors = tidy.check_config_file(os.path.join(BASE_PATH, 'servo-tidy.toml'), print_text=False)
         self.assertEqual("invalid config key 'key-outside'", next(errors)[2])
         self.assertEqual("invalid config key 'wrong-key'", next(errors)[2])
         self.assertEqual('invalid config table [wrong]', next(errors)[2])
@@ -38,17 +39,17 @@ class CheckTidiness(unittest.TestCase):
         errors = tidy.check_manifest_dirs(wrong_path, print_text=False)
         self.assertEqual("%s manifest file is required but was not found" % wrong_path, next(errors)[2])
         self.assertNoMoreErrors(errors)
-        errors = tidy.check_manifest_dirs(os.path.join(base_path, 'manifest-include.ini'), print_text=False)
+        errors = tidy.check_manifest_dirs(os.path.join(BASE_PATH, 'manifest-include.ini'), print_text=False)
         self.assertTrue(next(errors)[2].endswith("never_going_to_exist"))
         self.assertNoMoreErrors(errors)
 
     def test_directory_checks(self):
         dirs = {
-            os.path.join(base_path, "dir_check/webidl_plus"): ['webidl', 'test'],
-            os.path.join(base_path, "dir_check/only_webidl"): ['webidl']
-            }
+            os.path.join(BASE_PATH, "dir_check/webidl_plus"): ['webidl', 'test'],
+            os.path.join(BASE_PATH, "dir_check/only_webidl"): ['webidl']
+        }
         errors = tidy.check_directory_files(dirs)
-        error_dir = os.path.join(base_path, "dir_check/webidl_plus")
+        error_dir = os.path.join(BASE_PATH, "dir_check/webidl_plus")
         self.assertEqual("Unexpected extension found for test.rs. We only expect files with webidl, test extensions in {0}".format(error_dir), next(errors)[2])
         self.assertEqual("Unexpected extension found for test2.rs. We only expect files with webidl, test extensions in {0}".format(error_dir), next(errors)[2])
         self.assertNoMoreErrors(errors)
@@ -136,7 +137,7 @@ class CheckTidiness(unittest.TestCase):
         self.assertNoMoreErrors(ban_errors)
 
     def test_spec_link(self):
-        tidy.SPEC_BASE_PATH = base_path
+        tidy.SPEC_BASE_PATH = BASE_PATH
         errors = tidy.collect_errors_for_files(iterFile('speclink.rs'), [], [tidy.check_spec], print_text=False)
         self.assertEqual('method declared in webidl is missing a comment with a specification link', next(errors)[2])
         self.assertEqual('method declared in webidl is missing a comment with a specification link', next(errors)[2])
@@ -173,7 +174,7 @@ class CheckTidiness(unittest.TestCase):
         self.assertNoMoreErrors(errors)
 
     def test_json_with_unordered_keys(self):
-        tidy.config["check-ordered-json-keys"].append('python/tidy/servo_tidy_tests/unordered_key.json')
+        tidy.config["check-ordered-json-keys"].append('python/tidy/tests/unordered_key.json')
         errors = tidy.collect_errors_for_files(iterFile('unordered_key.json'), [tidy.check_json], [], print_text=False)
         self.assertEqual('Unordered key (found b before a)', next(errors)[2])
         self.assertNoMoreErrors(errors)
@@ -246,38 +247,15 @@ class CheckTidiness(unittest.TestCase):
         # needed to not raise errors in other test cases
         tidy.config["blocked-packages"]["rand"] = []
 
-    def test_lint_runner(self):
-        test_path = base_path + 'lints/'
-        runner = tidy.LintRunner(only_changed_files=False, progress=False)
-        runner.path = test_path + 'some-fictional-file'
-        self.assertEqual([(runner.path, 0, "file does not exist")], list(runner.check()))
-        runner.path = test_path + 'not_script'
-        self.assertEqual([(runner.path, 0, "lint should be a python script")],
-                         list(runner.check()))
-        runner.path = test_path + 'not_inherited.py'
-        self.assertEqual([(runner.path, 1, "class 'Lint' should inherit from 'LintRunner'")],
-                         list(runner.check()))
-        runner.path = test_path + 'no_lint.py'
-        self.assertEqual([(runner.path, 1, "script should contain a class named 'Lint'")],
-                         list(runner.check()))
-        runner.path = test_path + 'no_run.py'
-        self.assertEqual([(runner.path, 0, "class 'Lint' should implement 'run' method")],
-                         list(runner.check()))
-        runner.path = test_path + 'invalid_error_tuple.py'
-        self.assertEqual([(runner.path, 1, "errors should be a tuple of (path, line, reason)")],
-                         list(runner.check()))
-        runner.path = test_path + 'proper_file.py'
-        self.assertEqual([('path', 0, "foobar")], list(runner.check()))
-
     def test_file_list(self):
-        base_path='./python/tidy/servo_tidy_tests/test_ignored'
-        file_list = tidy.FileList(base_path, only_changed_files=False, exclude_dirs=[])
+        file_path = os.path.join(BASE_PATH, 'test_ignored')
+        file_list = tidy.FileList(file_path, only_changed_files=False, exclude_dirs=[])
         lst = list(file_list)
-        self.assertEqual([os.path.join(base_path, 'whee', 'test.rs'), os.path.join(base_path, 'whee', 'foo', 'bar.rs')], lst)
-        file_list = tidy.FileList(base_path, only_changed_files=False,
-                                  exclude_dirs=[os.path.join(base_path, 'whee', 'foo')])
+        self.assertEqual([os.path.join(file_path, 'whee', 'test.rs'), os.path.join(file_path, 'whee', 'foo', 'bar.rs')], lst)
+        file_list = tidy.FileList(file_path, only_changed_files=False,
+                                  exclude_dirs=[os.path.join(file_path, 'whee', 'foo')])
         lst = list(file_list)
-        self.assertEqual([os.path.join(base_path, 'whee', 'test.rs')], lst)
+        self.assertEqual([os.path.join(file_path, 'whee', 'test.rs')], lst)
 
     def test_multiline_string(self):
         errors = tidy.collect_errors_for_files(iterFile('multiline_string.rs'), [], [tidy.check_rust], print_text=True)

python/tidy/tidy.py

@@ -7,11 +7,8 @@
 # option. This file may not be copied, modified, or distributed
 # except according to those terms.
 
-from __future__ import print_function
-
 import fnmatch
 import glob
-import imp
 import itertools
 import json
 import os
@@ -23,13 +20,16 @@ import colorama
 import toml
 import voluptuous
 import yaml
+
 from .licenseck import OLD_MPL, MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml
-topdir = os.path.abspath(os.path.dirname(sys.argv[0]))
-wpt = os.path.join(topdir, "tests", "wpt")
+
+TOPDIR = os.path.abspath(os.path.dirname(sys.argv[0]))
+WPT_PATH = os.path.join(".", "tests", "wpt")
+SUITES = ["web-platform-tests", os.path.join("mozilla", "tests")]
 
 
 def wpt_path(*args):
-    return os.path.join(wpt, *args)
+    return os.path.join(WPT_PATH, *args)
 
 
 CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml")
@@ -1084,27 +1084,10 @@ class LintRunner(object):
         self.stylo = stylo
         self.no_wpt = no_wpt
 
-    def check(self):
-        if not os.path.exists(self.path):
-            yield (self.path, 0, "file does not exist")
-            return
-        if not self.path.endswith('.py'):
-            yield (self.path, 0, "lint should be a python script")
-            return
-        dir_name, filename = os.path.split(self.path)
-        sys.path.append(dir_name)
-        module = imp.load_source(filename[:-3], self.path)
-        sys.path.remove(dir_name)
-        if not hasattr(module, 'Lint'):
-            yield (self.path, 1, "script should contain a class named 'Lint'")
-            return
-
-        if not issubclass(module.Lint, LintRunner):
-            yield (self.path, 1, "class 'Lint' should inherit from 'LintRunner'")
-            return
-
-        lint = module.Lint(self.path, self.only_changed_files,
-                           self.exclude_dirs, self.progress, stylo=self.stylo, no_wpt=self.no_wpt)
+    def check(self, lint_cls):
+        lint = lint_cls(self.path, self.only_changed_files,
+                        self.exclude_dirs, self.progress,
+                        stylo=self.stylo, no_wpt=self.no_wpt)
         for error in lint.run():
             if type(error) is not tuple or (type(error) is tuple and len(error) != 3):
                 yield (self.path, 1, "errors should be a tuple of (path, line, reason)")
@@ -1122,10 +1105,8 @@ class LintRunner(object):
 
 def run_lint_scripts(only_changed_files=False, progress=True, stylo=False, no_wpt=False):
     runner = LintRunner(only_changed_files=only_changed_files, progress=progress, stylo=stylo, no_wpt=no_wpt)
-    for path in config['lint-scripts']:
-        runner.path = path
-        for error in runner.check():
-            yield error
+    for error in runner.check(WPTLint):
+        yield error
 
 
 def scan(only_changed_files=False, progress=True, stylo=False, no_wpt=False):
@@ -1162,3 +1143,30 @@ def scan(only_changed_files=False, progress=True, stylo=False, no_wpt=False):
         print("\033[92mtidy reported no errors.\033[0m")
 
     return int(error is not None)
+
+
+class WPTLint(LintRunner):
+    def _get_wpt_files(self, suite):
+        working_dir = os.path.join(WPT_PATH, suite, '')
+        file_iter = self.get_files(working_dir, exclude_dirs=[])
+        print('\nRunning the WPT lint on %s...' % working_dir)
+        for f in file_iter:
+            if filter_file(f):
+                yield f[len(working_dir):]
+
+    def run(self):
+        if self.stylo or self.no_wpt:
+            return
+
+        wpt_working_dir = os.path.abspath(os.path.join(WPT_PATH, "web-platform-tests"))
+        for suite in SUITES:
+            files = list(self._get_wpt_files(suite))
+            if not files:
+                continue
+            sys.path.insert(0, wpt_working_dir)
+            from tools.lint import lint
+            file_dir = os.path.abspath(os.path.join(WPT_PATH, suite))
+            returncode = lint.lint(file_dir, files, output_format="json")
+            sys.path.remove(wpt_working_dir)
+            if returncode:
+                yield ("WPT Lint Tool", "", "lint error(s) in Web Platform Tests: exit status %s" % returncode)

servo-tidy.toml

@@ -3,7 +3,6 @@ skip-check-length = false
 skip-check-licenses = false
 check-alphabetical-order = true
 check-ordered-json-keys = ["./resources/prefs.json"]
-lint-scripts = ["./python/servo/lints/wpt_lint.py"]
 
 # Packages which we avoid using in Servo.
 # For each blocked package, we can list the exceptions,
@@ -123,6 +122,8 @@ files = [
   "./tests/wpt/mozilla/tests/mozilla/textarea_placeholder.html",
   # Python 3 syntax causes "E901 SyntaxError" when flake8 runs in Python 2
   "./components/style/properties/build.py",
+  # The tidy tests currently don't pass tidy.
+  "./python/tidy/test.py",
 ]
 # Directories that are ignored for the non-WPT tidy check.
 directories = [
@@ -133,7 +134,7 @@ directories = [
   "./tests/wpt/web-platform-tests",
   "./tests/wpt/mozilla/tests/mozilla/referrer-policy",
   "./tests/wpt/mozilla/tests/webgl",
-  "./python/tidy/servo_tidy_tests",
+  "./python/tidy/tests",
   "./components/script/dom/bindings/codegen/parser",
   "./components/script/dom/bindings/codegen/ply",
   "./python/_virtualenv*",