Bug 1856535 - allow linters to work by excluding extensions instead of including them, r=ahal

Differential Revision: https://phabricator.services.mozilla.com/D202635

docs/code-quality/lint/create.rst

@@ -80,11 +80,15 @@ Each ``.yml`` file must have at least one linter defined in it. Here are the sup
 * include - A list of file paths that will be considered (optional)
 * exclude - A list of file paths or glob patterns that must not be matched (optional)
 * extensions - A list of file extensions to be considered (optional)
+* exclude_extensions - A list of file extensions to be excluded (optional)
 * setup - A function that sets up external dependencies (optional)
 * support-files - A list of glob patterns matching configuration files (optional)
 * find-dotfiles - If set to ``true``, run on dot files (.*) (optional)
 * ignore-case - If set to ``true`` and ``type`` is regex, ignore the case (optional)
 
+Note that a linter may not have both ``extensions`` and ``exclude_extensions`` specified at the
+same time.
+
 In addition to the above, some ``.yml`` files correspond to a single lint rule. For these, the
 following additional keys may be specified:
 

python/mozlint/mozlint/parser.py

@@ -91,8 +91,15 @@ class Parser(object):
                     "form 'module:object'".format(linter["setup"]),
                 )
 
-        if "extensions" in linter:
+        if "extensions" in linter and "exclude_extensions" in linter:
-            linter["extensions"] = [e.strip(".") for e in linter["extensions"]]
+            raise LinterParseError(
+                relpath,
+                "Can't have both 'extensions' and 'exclude_extensions'!",
+            )
+
+        for prop in ["extensions", "exclude_extensions"]:
+            if prop in linter:
+                linter[prop] = [e.strip(".") for e in linter[prop]]
 
     def parse(self, path):
         """Read a linter and return its LINTER definition.

python/mozlint/mozlint/pathutils.py

@@ -139,16 +139,20 @@ def collapse(paths, base=None, dotfiles=False):
     return list(covered)
 
 
-def filterpaths(root, paths, include, exclude=None, extensions=None):
+def filterpaths(
+    root, paths, include, exclude=None, extensions=None, exclude_extensions=None
+):
     """Filters a list of paths.
 
     Given a list of paths and some filtering rules, return the set of paths
-    that should be linted.
+    that should be linted. Note that at most one of extensions or
+    exclude_extensions should be provided (ie not both).
 
     :param paths: A starting list of paths to possibly lint.
     :param include: A list of paths that should be included (required).
     :param exclude: A list of paths that should be excluded (optional).
     :param extensions: A list of file extensions which should be considered (optional).
+    :param exclude_extensions: A list of file extensions which should not be considered (optional).
     :returns: A tuple containing a list of file paths to lint and a list of
               paths to exclude.
     """
@@ -173,6 +177,8 @@ def filterpaths(root, paths, include, exclude=None, extensions=None):
         # Exclude bad file extensions
         if extensions and path.isfile and path.ext not in extensions:
             continue
+        elif exclude_extensions and path.isfile and path.ext in exclude_extensions:
+            continue
 
         if path.match(excludeglobs):
             continue
@@ -280,6 +286,9 @@ def expand_exclusions(paths, config, root):
         Generator which generates list of paths that weren't excluded.
     """
     extensions = [e.lstrip(".") for e in config.get("extensions", [])]
+    exclude_extensions = [e.lstrip(".") for e in config.get("exclude_extensions", [])]
+    if extensions and exclude_extensions:
+        raise ValueError("Can't specify both extensions and exclude_extensions.")
     find_dotfiles = config.get("find-dotfiles", False)
 
     def normalize(path):
@@ -289,6 +298,13 @@ def expand_exclusions(paths, config, root):
         return mozpath.join(root, path)
 
     exclude = list(map(normalize, config.get("exclude", [])))
+    # We need excluded extensions in both the ignore for the FileFinder and in
+    # the exclusion set. If we don't put it in the exclusion set, we would
+    # return files that are passed explicitly and whose extensions are in the
+    # exclusion set. If we don't put it in the ignore set, the FileFinder
+    # would return files in (sub)directories passed to us.
+    base_ignore = ["**/*.{}".format(ext) for ext in exclude_extensions]
+    exclude += base_ignore
     for path in paths:
         path = mozpath.normsep(path)
         if os.path.isfile(path):
@@ -301,16 +317,27 @@ def expand_exclusions(paths, config, root):
             yield path
             continue
 
-        ignore = [
+        # If there are neither extensions nor exclude_extensions, we can't do
+        # anything useful with a directory. Skip:
+        if not extensions and not exclude_extensions:
+            continue
+
+        # This is a directory. Check we don't have excludes for ancestors of
+        # this path. Mess with slashes to avoid "foo/bar" matching "foo/barry".
+        parent_path = os.path.dirname(path.rstrip("/")) + "/"
+        assert not any(parent_path.startswith(e.rstrip("/") + "/") for e in exclude)
+
+        ignore = base_ignore + [
             e[len(path) :].lstrip("/")
             for e in exclude
             if mozpath.commonprefix((path, e)) == path
         ]
+
         finder = FileFinder(path, ignore=ignore, find_dotfiles=find_dotfiles)
-
+        if extensions:
-        _, ext = os.path.splitext(path)
+            for ext in extensions:
-        ext.lstrip(".")
+                for p, f in finder.find("**/*.{}".format(ext)):
-
+                    yield os.path.join(path, p)
-        for ext in extensions:
+        else:
-            for p, f in finder.find("**/*.{}".format(ext)):
+            for p, f in finder.find("**/*.*"):
                 yield os.path.join(path, p)

python/mozlint/mozlint/types.py

@@ -39,6 +39,7 @@ class BaseType(object):
                 config["include"],
                 config.get("exclude", []),
                 config.get("extensions", []),
+                config.get("exclude_extensions", []),
             )
             config["exclude"] = exclude
         elif config.get("exclude"):

python/mozlint/test/linters/invalid_ext_and_exclude_ext.yml

@@ -0,0 +1,7 @@
+---
+InvalidExtensionAndExcludedExtensions:
+    type: string
+    payload: foobar
+    description: Having both extensions and exclude_extensions is not allowed.
+    extensions: ["*.js"]
+    exclude_extensions: ["*.png"]

python/mozlint/test/test_parser.py

@@ -59,6 +59,7 @@ def test_parser_valid_multiple(parse):
         "invalid_include_with_glob.yml",
         "invalid_exclude.yml",
         "invalid_support_files.yml",
+        "invalid_ext_and_exclude_ext.yml",
         "missing_attrs.yml",
         "missing_definition.yml",
         "non_existing_include.yml",

python/mozlint/test/test_pathutils.py

@@ -59,6 +59,43 @@ def assert_paths(a, b):
             "extensions": ["py"],
             "expected": ["a.py", "subdir1/b.py"],
         },
+        pytest.param(
+            {
+                "paths": [
+                    "a.py",
+                    "a.js",
+                    "subdir1/b.py",
+                    "subdir2/c.py",
+                    "subdir1/subdir3/d.py",
+                ],
+                "include": ["."],
+                "exclude": [],
+                "exclude_extensions": ["py"],
+                "expected": ["a.js"],
+            },
+            id="Excluding .py should only return .js file.",
+        ),
+        pytest.param(
+            {
+                "paths": [
+                    "a.py",
+                    "a.js",
+                    "subdir1/b.py",
+                    "subdir2/c.py",
+                    "subdir1/subdir3/d.py",
+                ],
+                "include": ["."],
+                "exclude": [],
+                "exclude_extensions": ["js"],
+                "expected": [
+                    "a.py",
+                    "subdir1/b.py",
+                    "subdir2/c.py",
+                    "subdir1/subdir3/d.py",
+                ],
+            },
+            id="Excluding .js should only return .py files.",
+        ),
         {
             "paths": ["a.py", "a.js", "subdir2"],
             "include": ["."],
@@ -104,24 +141,47 @@ def test_filterpaths(test):
             "paths": ["subdir1/b.js"],
             "config": {
                 "exclude": ["subdir1"],
-                "extensions": "js",
+                "extensions": ["js"],
             },
             "expected": [],
         },
         {
-            "paths": ["subdir1/subdir3"],
+            "paths": ["subdir1"],
             "config": {
                 "exclude": ["subdir1"],
-                "extensions": "js",
+                "extensions": ["js"],
             },
             "expected": [],
         },
+        pytest.param(
+            {
+                "paths": ["a.py", "subdir1"],
+                "config": {
+                    "exclude": ["subdir1"],
+                    "exclude_extensions": ["gob"],
+                },
+                "expected": ["a.py"],
+            },
+            id="Excluding both subdirs and nonsense extensions returns other files.",
+        ),
+        pytest.param(
+            {
+                "paths": ["a.py", "a.js", "subdir1"],
+                "config": {
+                    "exclude": [],
+                    "exclude_extensions": ["py"],
+                },
+                "expected": ["a.js", "subdir1/subdir3/d.js", "subdir1/b.js"],
+            },
+            id="Excluding .py files returns only non-.py files, also from subdirs.",
+        ),
     ),
 )
 def test_expand_exclusions(test):
     expected = test.pop("expected", [])
 
-    paths = list(pathutils.expand_exclusions(test["paths"], test["config"], root))
+    input_paths = [os.path.join(root, p) for p in test["paths"]]
+    paths = list(pathutils.expand_exclusions(input_paths, test["config"], root))
     assert_paths(paths, expected)