From d8361cb5cfb4ade0f9288bccd68e7962516e3754 Mon Sep 17 00:00:00 2001 From: Craig Silverstein Date: Sun, 19 Apr 2020 23:16:55 -0700 Subject: [PATCH] Allow repos to override khan-linter's linting on a per-language basis. Summary: Khan-linter now looks for a config file in a repo, before deciding how to lint any files in that repo. If it finds one, it sees if the config file has its own rules for how to lint that file. If so, it does those rules rather than its built-in ones. This will let us migrate linters from khan-linter to webapp piece-by-piece, without having to change khan-linter each time we do so. It also means we don't have to worry about forward- and backward-compability as people change branches, since any new webapp linters will be committed in the same commit as the config-file change. Issue: TODO Test Plan: I created this lintconfig.yaml in webapp: ``` khan-linter-overrides: yaml: - ["echo", "hi""] ``` and ran ka-lint lintconfig.yaml and it emitted: hi lintconfig.yaml like the new rule said! I then ran `ka-lint main.py` and it did not print `hi`. I then ran cd assignments; ka-lint ../lintconfig.yaml and it said ka-lint lintconfig.yaml again, because we run stuff from webapp-root. That may yield a bit of a surprise to folks that lint is being run in a different directory than they're in, but hopefully it won't be too bad. Reviewers: benkraft, aviddy Reviewed By: benkraft Subscribers: davidbraley, kevinb Differential Revision: https://phabricator.khanacademy.org/D62448 --- linters.py | 28 ++++++++++++++++++++++++++++ runlint.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/linters.py b/linters.py index 3335b93d..083b1210 100644 --- a/linters.py +++ b/linters.py @@ -120,6 +120,34 @@ def report(self, lint): lint_util.print_(lint) +class DelegatingLinter(Linter): + """A linter that just calls out to a binary to do its actual work.""" + def __init__(self, argv, cwd, logger): + """argv is the command to run (as a lint). cwd is where to run it.""" + super(DelegatingLinter, self).__init__(logger) + self._argv = argv + self._cwd = cwd + + def process_files(self, files): + # We need to refer to all filenames relative to self._cwd. + files = [f if os.path.isabs(f) else os.path.relpath(f, self._cwd) + for f in files] + + pipe = subprocess.Popen( + self._argv + files, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=self._cwd) + stdout, stderr = pipe.communicate() + + if stderr: + raise RuntimeError("Unexpected stderr from linter:\n%s" % stderr) + lines = stdout.rstrip('\n').splitlines() + for line in lines: + self.report(line) + return len(lines) + + def _capture_stdout_of(fn, *args, **kwargs): """Call fn(*args, **kwargs) and return (fn_retval, fn_stdout_output_fp).""" try: diff --git a/runlint.py b/runlint.py index fd32836f..d03aafc0 100755 --- a/runlint.py +++ b/runlint.py @@ -615,6 +615,23 @@ def _find_base_config(file_to_lint, config_filename): return None +_REPO_CONFIG_CACHE = {} + + +def _find_repo_config(file_to_lint): + """Look for lintconfig.yaml for this file, and return it if found.""" + repo_config_file = _find_base_config(file_to_lint, 'lintconfig.yaml') + if not repo_config_file: + return (None, {}) + + if repo_config_file not in _REPO_CONFIG_CACHE: + with open(repo_config_file) as f: + _REPO_CONFIG_CACHE[repo_config_file] = linters.yaml.safe_load(f) + + return (os.path.dirname(repo_config_file), + _REPO_CONFIG_CACHE[repo_config_file]) + + def _get_linters_for_file(file_to_lint, lang, propose_arc_fixes): """Return the linters we wish to run for this file. @@ -671,6 +688,17 @@ def _get_linters_for_file(file_to_lint, lang, propose_arc_fixes): file_lang = _lang(file_to_lint, lang) + # We allow a repository to override linters for a particular + # language. See if our repository does so. + (repo_config_dir, repo_config) = _find_repo_config(file_to_lint) + if repo_config: + cmds = repo_config.get('khan-linter-overrides', {}).get(file_lang) + if cmds is not None: + return [ + linters.DelegatingLinter(cmd, repo_config_dir, _get_logger()) + for cmd in cmds + ] + # We support multiple configuration files for eslint and our graphql # schema linter, , which allows runlint to run against subrepos with # different configurations.