Bug 1591991 Add lint Fixed count support to linters r=sylvestre

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

python/mozlint/mozlint/formatters/stylish.py

@@ -30,7 +30,9 @@ class StylishFormatter(object):
 {diff}""".lstrip(
         "\n"
     )
-    fmt_summary = "{t.bold}{c}\u2716 {problem} ({error}, {warning}{failure}){t.normal}"
+    fmt_summary = (
+        "{t.bold}{c}\u2716 {problem} ({error}, {warning}{failure}, {fixed}){t.normal}"
+    )
 
     def __init__(self, disable_colors=False):
         self.term = Terminal(disable_styling=disable_colors)
@@ -77,6 +79,7 @@ class StylishFormatter(object):
 
         num_errors = 0
         num_warnings = 0
+        num_fixed = result.fixed
         for path, errors in sorted(result.issues.items()):
             self._reset_max()
 
@@ -139,6 +142,7 @@ class StylishFormatter(object):
                 failure=", {}".format(pluralize("failure", len(failed)))
                 if failed
                 else "",
+                fixed="{} fixed".format(num_fixed),
             )
         )
 

python/mozlint/mozlint/result.py

@@ -28,6 +28,7 @@ class ResultSummary(object):
         self.failed_run = set()
         self.failed_setup = set()
         self.suppressed_warnings = defaultdict(int)
+        self.fixed = 0
 
     @property
     def returncode(self):
@@ -47,6 +48,10 @@ class ResultSummary(object):
     def total_suppressed_warnings(self):
         return sum(self.suppressed_warnings.values())
 
+    @property
+    def total_fixed(self):
+        return self.fixed
+
     def update(self, other):
         """Merge results from another ResultSummary into this one."""
         for path, obj in other.issues.items():
@@ -54,6 +59,7 @@ class ResultSummary(object):
 
         self.failed_run |= other.failed_run
         self.failed_setup |= other.failed_setup
+        self.fixed += other.fixed
         for k, v in other.suppressed_warnings.items():
             self.suppressed_warnings[k] += v
 

python/mozlint/mozlint/roller.py

@@ -61,7 +61,17 @@ def _run_worker(config, paths, **lintargs):
     func = supported_types[config["type"]]
     start_time = time.time()
     try:
-        res = func(paths, config, **lintargs) or []
+        res = func(paths, config, **lintargs)
+        # Some linters support fixed operations
+        # dict returned - {"results":results,"fixed":fixed}
+        if isinstance(res, dict):
+            result.fixed += res["fixed"]
+            res = res["results"] or []
+        elif isinstance(res, list):
+            res = res or []
+        else:
+            print("Unexpected type received")
+            assert False
     except Exception:
         traceback.print_exc()
         res = 1

python/mozlint/mozlint/types.py

@@ -45,7 +45,7 @@ class BaseType(object):
             del config["exclude"]
 
         if not paths:
-            return []
+            return {"results": [], "fixed": 0}
 
         log.debug(
             "Passing the following paths:\n{paths}".format(

python/mozlint/test/test_formatters.py

@@ -45,7 +45,7 @@ EXPECTED = {
 /fake/root/d/e/f.txt
   4:2  warning  oh no bar  bar-not-allowed (bar)
 
-\u2716 4 problems (3 errors, 1 warning)
+\u2716 4 problems (3 errors, 1 warning, 0 fixed)
 """.strip(),
     },
     "treeherder": {

tools/lint/clang-format/__init__.py

@@ -149,10 +149,7 @@ def lint(paths, config, fix=None, **lintargs):
     version = run_process(config, base_command)
     log.debug("Version: {}".format(version))
 
-    if fix:
-        cmd_args.append("-i")
-    else:
-        cmd_args.append("--dry-run")
+    cmd_args.append("--dry-run")
     base_command = cmd_args + paths
     log.debug("Command: {}".format(" ".join(cmd_args)))
     output = run_process(config, base_command)
@@ -163,6 +160,16 @@ def lint(paths, config, fix=None, **lintargs):
             "clang-format output should be a multiple of 3. Output: %s" % output
         )
 
+    fixed = (int)(len(output) / 3)
+    if fix:
+        cmd_args.remove("--dry-run")
+        cmd_args.append("-i")
+        base_command = cmd_args + paths
+        log.debug("Command: {}".format(" ".join(cmd_args)))
+        output = run_process(config, base_command)
+    else:
+        fixed = 0
+
     for i in range(0, len(output), 3):
         # Merge the element 3 by 3 (clang-format output)
         line = output[i]
@@ -172,5 +179,5 @@ def lint(paths, config, fix=None, **lintargs):
 
     if fix:
         # clang-format is able to fix all issues so don't bother parsing the output.
-        return []
-    return parse_issues(config, output_list, paths, log)
+        return {"results": [], "fixed": fixed}
+    return {"results": parse_issues(config, output_list, paths, log), "fixed": fixed}

tools/lint/eslint/__init__.py

@@ -86,10 +86,19 @@ def lint(paths, config, binary=None, fix=None, setup=None, **lintargs):
         + paths
     )
     log.debug("Command: {}".format(" ".join(cmd_args)))
-
+    results = run(cmd_args, config)
+    fixed = 0
     # eslint requires that --fix be set before the --ext argument.
     if fix:
+        fixed += len(results)
         cmd_args.insert(2, "--fix")
+        results = run(cmd_args, config)
+        fixed = fixed - len(results)
+
+    return {"results": results, "fixed": fixed}
+
+
+def run(cmd_args, config):
 
     shell = False
     if os.environ.get("MSYSTEM") in ("MINGW32", "MINGW64"):

tools/lint/file-perm/__init__.py

@@ -11,11 +11,12 @@ from mozlint.pathutils import expand_exclusions
 
 def lint(paths, config, fix=None, **lintargs):
     results = []
+    fixed = 0
 
     if platform.system() == "Windows":
         # Windows doesn't have permissions in files
         # Exit now
-        return results
+        return {"results": results, "fixed": fixed}
 
     files = list(expand_exclusions(paths, config, lintargs["root"]))
     for f in files:
@@ -32,6 +33,7 @@ def lint(paths, config, fix=None, **lintargs):
             if fix:
                 # We want to fix it, do it and leave
                 os.chmod(f, 0o644)
+                fixed += 1
                 continue
 
             res = {
@@ -40,4 +42,4 @@ def lint(paths, config, fix=None, **lintargs):
                 "level": "error",
             }
             results.append(result.from_config(config, **res))
-    return results
+    return {"results": results, "fixed": fixed}

tools/lint/file-whitespace/__init__.py

@@ -11,6 +11,7 @@ results = []
 def lint(paths, config, fix=None, **lintargs):
     files = list(expand_exclusions(paths, config, lintargs["root"]))
     log = lintargs["log"]
+    fixed = 0
 
     for f in files:
         with open(f, "rb") as open_file:
@@ -24,6 +25,7 @@ def lint(paths, config, fix=None, **lintargs):
                     # return file pointer to first
                     open_file.seek(0)
                     if fix:
+                        fixed += 1
                         # fix Empty lines at end of file
                         for i, line in reversed(list(enumerate(open_file))):
                             # determine if line is empty
@@ -55,6 +57,7 @@ def lint(paths, config, fix=None, **lintargs):
             # Detect missing newline at the end of the file
             if lines[:].__len__() != 0 and not lines[-1].endswith(b"\n"):
                 if fix:
+                    fixed += 1
                     with open(f, "wb") as write_file:
                         # add a newline character at end of file
                         lines[-1] = lines[-1] + b"\n"
@@ -79,6 +82,7 @@ def lint(paths, config, fix=None, **lintargs):
                     if fix:
                         # We want to fix it, strip the trailing spaces
                         content_to_write.append(line.rstrip() + b"\n")
+                        fixed += 1
                         hasFix = True
                     else:
                         res = {
@@ -105,7 +109,7 @@ def lint(paths, config, fix=None, **lintargs):
 
             if b"\r\n" in content:
                 if fix:
-                    # replace \r\n by \n
+                    fixed += 1
                     content = content.replace(b"\r\n", b"\n")
                     with open(f, "wb") as open_file_to_write:
                         open_file_to_write.write(content)
@@ -117,4 +121,4 @@ def lint(paths, config, fix=None, **lintargs):
                     }
                     results.append(result.from_config(config, **res))
 
-    return results
+    return {"results": results, "fixed": fixed}

tools/lint/license/__init__.py

@@ -172,13 +172,14 @@ def fix_me(log, filename):
 def lint(paths, config, fix=None, **lintargs):
     log = lintargs["log"]
     files = list(expand_exclusions(paths, config, lintargs["root"]))
+    fixed = 0
 
     licenses = load_valid_license()
-
     for f in files:
         if is_test(f):
             # For now, do not do anything with test (too many)
             continue
+
         if not is_valid_license(licenses, f):
             res = {
                 "path": f,
@@ -188,4 +189,6 @@ def lint(paths, config, fix=None, **lintargs):
             results.append(result.from_config(config, **res))
             if fix:
                 fix_me(log, f)
-    return results
+                fixed += 1
+
+    return {"results": results, "fixed": fixed}

tools/lint/python/black.py

@@ -117,6 +117,7 @@ def setup(root, **lintargs):
 
 
 def run_black(config, paths, fix=None, *, log, virtualenv_bin_path):
+    fixed = 0
     binary = os.path.join(virtualenv_bin_path or default_bindir(), "black")
 
     log.debug("Black version {}".format(get_black_version(binary)))
@@ -124,9 +125,17 @@ def run_black(config, paths, fix=None, *, log, virtualenv_bin_path):
     cmd_args = [binary]
     if not fix:
         cmd_args.append("--check")
+
     base_command = cmd_args + paths
     log.debug("Command: {}".format(" ".join(base_command)))
-    return parse_issues(config, run_process(config, base_command), paths, log=log)
+    output = parse_issues(config, run_process(config, base_command), paths, log=log)
+
+    # black returns an issue for fixed files as well
+    for eachIssue in output:
+        if eachIssue.message == "reformatted":
+            fixed += 1
+
+    return {"results": output, "fixed": fixed}
 
 
 def lint(paths, config, fix=None, **lintargs):

tools/lint/python/flake8.py

@@ -86,14 +86,17 @@ def setup(root, **lintargs):
 
 
 def lint(paths, config, **lintargs):
-    from flake8.main.application import Application
 
-    log = lintargs["log"]
     root = lintargs["root"]
     virtualenv_bin_path = lintargs.get("virtualenv_bin_path")
     config_path = os.path.join(root, ".flake8")
 
+    results = run(paths, config, **lintargs)
+    fixed = 0
+
     if lintargs.get("fix"):
+        # fix and run again to count remaining issues
+        fixed = len(results)
         fix_cmd = [
             os.path.join(virtualenv_bin_path or default_bindir(), "autopep8"),
             "--global-config",
@@ -106,6 +109,19 @@ def lint(paths, config, **lintargs):
             fix_cmd.extend(["--exclude", ",".join(config["exclude"])])
 
         subprocess.call(fix_cmd + paths)
+        results = run(paths, config, **lintargs)
+
+        fixed = fixed - len(results)
+
+    return {"results": results, "fixed": fixed}
+
+
+def run(paths, config, **lintargs):
+    from flake8.main.application import Application
+
+    log = lintargs["log"]
+    root = lintargs["root"]
+    config_path = os.path.join(root, ".flake8")
 
     # Run flake8.
     app = Application()

tools/lint/rust/__init__.py

@@ -73,7 +73,7 @@ def parse_issues(config, output, paths):
                 "lineno": issue.line,
             }
             results.append(result.from_config(config, **res))
-    return results
+    return {"results": results, "fixed": 0}
 
 
 class RustfmtProcess(ProcessHandler):
@@ -159,13 +159,20 @@ def lint(paths, config, fix=None, **lintargs):
         return 1
 
     cmd_args = [binary]
-    if not fix:
-        cmd_args.append("--check")
+    cmd_args.append("--check")
     base_command = cmd_args + paths
     log.debug("Command: {}".format(" ".join(cmd_args)))
     output = run_process(config, base_command)
 
+    issues = parse_issues(config, output, paths)
+
     if fix:
-        # Rustfmt is able to fix all issues so don't bother parsing the output.
-        return []
-    return parse_issues(config, output, paths)
+        issues["fixed"] = len(issues["results"])
+        issues["results"] = []
+        cmd_args.remove("--check")
+
+        base_command = cmd_args + paths
+        log.debug("Command: {}".format(" ".join(cmd_args)))
+        output = run_process(config, base_command)
+
+    return issues

tools/lint/spell/__init__.py

@@ -43,6 +43,9 @@ CODESPELL_FORMAT_REGEX = re.compile(r"(.*):(.*): (.*) ==> (.*)$")
 
 
 class CodespellProcess(LintProcess):
+    fixed = 0
+    _fix = None
+
     def process_line(self, line):
         try:
             match = CODESPELL_FORMAT_REGEX.match(line)
@@ -56,6 +59,8 @@ class CodespellProcess(LintProcess):
         m = re.match(r"^[a-z][A-Z][a-z]*", typo)
         if m:
             return
+        if CodespellProcess._fix:
+            CodespellProcess.fixed += 1
         res = {
             "path": abspath,
             "message": typo.strip() + " ==> " + correct,
@@ -116,10 +121,6 @@ def lint(paths, config, fix=None, **lintargs):
 
     config["root"] = lintargs["root"]
 
-    skip_files = ""
-    if "exclude" in config:
-        skip_files = "--skip=*.dic,{}".format(",".join(config["exclude"]))
-
     exclude_list = os.path.join(here, "exclude-list.txt")
     cmd_args = [
         which("python"),
@@ -132,16 +133,30 @@ def lint(paths, config, fix=None, **lintargs):
         #    that were disabled in dictionary.
         "--quiet-level=7",
         "--ignore-words=" + exclude_list,
-        skip_files,
     ]
 
-    if fix:
-        cmd_args.append("--write-changes")
-    log.debug("Command: {}".format(" ".join(cmd_args)))
+    if "exclude" in config:
+        cmd_args.append("--skip=*.dic,{}".format(",".join(config["exclude"])))
 
+    log.debug("Command: {}".format(" ".join(cmd_args)))
     log.debug("Version: {}".format(get_codespell_version(binary)))
 
-    base_command = cmd_args + paths
+    if fix:
+        CodespellProcess._fix = True
 
+    base_command = cmd_args + paths
     run_process(config, base_command)
-    return results
+
+    if fix:
+        global results
+        results = []
+        cmd_args.append("--write-changes")
+        log.debug("Command: {}".format(" ".join(cmd_args)))
+        log.debug("Version: {}".format(get_codespell_version(binary)))
+        base_command = cmd_args + paths
+        run_process(config, base_command)
+        CodespellProcess.fixed = CodespellProcess.fixed - len(results)
+    else:
+        CodespellProcess.fixed = 0
+
+    return {"results": results, "fixed": CodespellProcess.fixed}

tools/lint/test/conftest.py

@@ -98,7 +98,7 @@ def run_setup(config):
 
 
 @pytest.fixture
-def lint(config, root):
+def lint(config, root, request):
     """Find and return the 'lint' function for the external linter named in the
     LINTER global variable.
 
@@ -119,7 +119,13 @@ def lint(config, root):
         lintargs["log"] = logging.LoggerAdapter(
             logger, {"lintname": config.get("name"), "pid": os.getpid()}
         )
+
         results = func(paths, config, root=root, **lintargs)
+        if hasattr(request.module, "fixed") and isinstance(results, dict):
+            request.module.fixed += results["fixed"]
+
+        if isinstance(results, dict):
+            results = results["results"]
 
         if isinstance(results, (list, tuple)):
             results = sorted(results)

tools/lint/test/test_black.py

@@ -7,13 +7,40 @@
 import mozunit
 
 LINTER = "black"
+fixed = 0
+
+
+def test_lint_fix(lint, create_temp_file):
+
+    contents = """def is_unique(
+               s
+               ):
+    s = list(s
+                )
+    s.sort()
+
+
+    for i in range(len(s) - 1):
+        if s[i] == s[i + 1]:
+            return 0
+    else:
+        return 1
+
+
+if __name__ == "__main__":
+    print(
+          is_unique(input())
+         ) """
+
+    path = create_temp_file(contents, "bad.py")
+    lint([path], fix=True)
+    assert fixed == 1
 
 
 def test_lint_black(lint, paths):
     results = lint(paths())
     assert len(results) == 2
 
-    print(results)
     assert results[0].level == "error"
     assert results[0].relpath == "bad.py"
 

tools/lint/test/test_clang_format.py

@@ -3,6 +3,7 @@ import mozunit
 from conftest import build
 
 LINTER = "clang-format"
+fixed = 0
 
 
 def test_good(lint, config, paths):
@@ -10,6 +11,9 @@ def test_good(lint, config, paths):
     print(results)
     assert len(results) == 0
 
+    results = lint(paths("good/"), root=build.topsrcdir, use_filters=False, fix=True)
+    assert fixed == len(results)
+
 
 def test_basic(lint, config, paths):
     results = lint(paths("bad/bad.cpp"), root=build.topsrcdir, use_filters=False)
@@ -44,5 +48,18 @@ def test_dir(lint, config, paths):
     assert "#include" in results[5].diff
 
 
+def test_fixed(lint, create_temp_file):
+
+    contents = """int  main ( ) { \n
+return 0; \n
+
+}"""
+
+    path = create_temp_file(contents, "ignore.cpp")
+    lint([path], use_filters=False, fix=True)
+
+    assert fixed == 5
+
+
 if __name__ == "__main__":
     mozunit.main()

tools/lint/test/test_codespell.py

@@ -3,6 +3,21 @@ from __future__ import absolute_import, print_function
 import mozunit
 
 LINTER = "codespell"
+fixed = 0
+
+
+def test_lint_codespell_fix(lint, create_temp_file):
+    contents = """This is a file with some typos and informations.
+But also testing false positive like optin (because this isn't always option)
+or stuff related to our coding style like:
+aparent (aParent).
+but detects mistakes like mozila
+""".lstrip()
+
+    path = create_temp_file(contents, "ignore.rst")
+    lint([path], fix=True)
+
+    assert fixed == 2
 
 
 def test_lint_codespell(lint, paths):

tools/lint/test/test_eslint.py

@@ -3,6 +3,7 @@ import mozunit
 from conftest import build
 
 LINTER = "eslint"
+fixed = 0
 
 
 def test_lint_with_global_exclude(lint, config, paths):
@@ -26,5 +27,25 @@ def test_bad_import(lint, config, paths):
     assert results == 1
 
 
+def test_fix(lint, config, create_temp_file):
+    contents = """/*eslint no-regex-spaces: "error"*/
+
+    var re = /foo   bar/;
+    var re = new RegExp("foo   bar");
+
+
+    var re = /foo   bar/;
+    var re = new RegExp("foo   bar");
+
+    var re = /foo   bar/;
+    var re = new RegExp("foo   bar");
+
+"""
+    path = create_temp_file(contents, "bad.js")
+    lint([path], config=config, root=build.topsrcdir, fix=True)
+
+    assert fixed == 6
+
+
 if __name__ == "__main__":
     mozunit.main()

tools/lint/test/test_file_perm.py

@@ -10,7 +10,6 @@ LINTER = "file-perm"
 @pytest.mark.lint_config(name="file-perm")
 def test_lint_file_perm(lint, paths):
     results = lint(paths("no-shebang"), collapse_results=True)
-    print(results)
 
     assert results.keys() == {
         "no-shebang/bad.c",
@@ -27,7 +26,7 @@ def test_lint_file_perm(lint, paths):
 @pytest.mark.lint_config(name="maybe-shebang-file-perm")
 def test_lint_shebang_file_perm(config, lint, paths):
     results = lint(paths("maybe-shebang"))
-    print(results)
+
     assert len(results) == 1
 
     assert "permissions on a source" in results[0].message

tools/lint/test/test_file_whitespace.py

@@ -2,7 +2,9 @@ from __future__ import absolute_import, print_function
 
 import mozunit
 
+
 LINTER = "file-whitespace"
+fixed = 0
 
 
 def test_lint_file_whitespace(lint, paths):
@@ -33,5 +35,20 @@ def test_lint_file_whitespace(lint, paths):
     assert results[4].lineno == 2
 
 
+def test_lint_file_whitespace_fix(lint, paths, create_temp_file):
+
+    contents = """int main() {  \n
+    return 0;  \n
+}
+
+
+"""
+
+    path = create_temp_file(contents, "bad.cpp")
+    lint([path], fix=True)
+    # Gives a different answer on Windows. Probably because of Windows CR
+    assert fixed == 3 or fixed == 2
+
+
 if __name__ == "__main__":
     mozunit.main()

tools/lint/test/test_flake8.py

@@ -3,6 +3,7 @@ import os
 import mozunit
 
 LINTER = "flake8"
+fixed = 0
 
 
 def test_lint_single_file(lint, paths):
@@ -26,6 +27,7 @@ def test_lint_custom_config_ignored(lint, paths):
 
 
 def test_lint_fix(lint, create_temp_file):
+    global fixed
     contents = """
 import distutils
 
@@ -40,12 +42,16 @@ def foobar():
     # Make sure the missing blank line is fixed, but the unused import isn't.
     results = lint([path], fix=True)
     assert len(results) == 1
+    assert fixed == 1
+
+    fixed = 0
 
     # Also test with a directory
     path = os.path.dirname(create_temp_file(contents, name="bad2.py"))
     results = lint([path], fix=True)
     # There should now be two files with 2 combined errors
     assert len(results) == 2
+    assert fixed == 1
     assert all(r.rule != "E501" for r in results)
 
 

tools/lint/test/test_rustfmt.py

@@ -2,6 +2,7 @@ import mozunit
 
 
 LINTER = "rustfmt"
+fixed = 0
 
 
 def test_good(lint, config, paths):
@@ -40,5 +41,31 @@ def test_dir(lint, config, paths):
     assert "Print text to the console" in results[1].diff
 
 
+def test_fix(lint, create_temp_file):
+
+    contents = """fn main() {
+    // Statements here are executed when the compiled binary is called
+
+    // Print text to the console
+    println!("Hello World!");
+    let mut a;
+    let mut b=1;
+    let mut vec = Vec::new();
+    vec.push(1);
+    vec.push(2);
+
+
+    for x in 5..10 - 5 {
+        a = x;
+    }
+
+    }
+"""
+
+    path = create_temp_file(contents, "bad.rs")
+    lint([path], fix=True)
+    assert fixed == 3
+
+
 if __name__ == "__main__":
     mozunit.main()