Skip to content

Commit

Permalink
Handle an arc-autofix-script in lintconfig.yaml.
Browse files Browse the repository at this point in the history
Summary:
This is a continuing part of our effort to move logic from khan-linter
into the repos.  In this case the logic is the data we append to some
lint lines to tell `arc lint` how to fix the lint error.  If the
repo's lintconfig.yaml says it has such a script, we use it for
autofixing.

This will allow us to move linters with autofixing, such as our eslint
linter, into other repos, which was not possible before.

Test Plan:
First, I made sure webapp's lintconfig.yaml did //not// have an
`arc-autofix-script`, to test legacy behavior.  I added this "bad"
line to the end of webapp's pubsub.yaml:
    - t: "4"%
I then ran
   ka-lint pubsub.yaml
and it said:
    Error  (S&RX) E=yaml=
    bad indentation of a sequence entry at line 345, column 9:
so it properly ran when no autofix-script was specified.

I then added this line to lintconfig.yaml:
   arc-autofix-script: ["sed", "s/q/QQQ/"]
and re-ran `ka-lint pubsub.yaml`.  This time it emitted
      Error  (S&RX) E=yaml=
      bad indentation of a seQQQuence entry at line 345, column 9:
showing that the pipe works!

Reviewers: benkraft

Reviewed By: benkraft

Subscribers: kevinb

Differential Revision: https://phabricator.khanacademy.org/D63014
  • Loading branch information
csilvers committed May 9, 2020
1 parent ee024e2 commit 7c8bf0e
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
20 changes: 19 additions & 1 deletion linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ def report(self, lint):

class DelegatingLinter(Linter):
"""A linter that just calls out to a binary to do its actual work."""
def __init__(self, argv, cwd, logger):
def __init__(self, argv, cwd, arc_autofix_script, 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
self._arc_autofix_script = arc_autofix_script

def process_files(self, files):
# We need to refer to all filenames relative to self._cwd.
Expand All @@ -142,6 +143,23 @@ def process_files(self, files):

if stderr:
raise RuntimeError("Unexpected stderr from linter:\n%s" % stderr)

if stdout.strip() and self._arc_autofix_script:
# Run through the autofix-script filter to modify each
# lint-line to include arc fixing information.
autofix_pipe = subprocess.Popen(
self._arc_autofix_script,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=self._cwd)
autofix_stdout, autofix_stderr = autofix_pipe.communicate(
input=stdout)
if autofix_stderr:
raise RuntimeError("Unexpected stderr from autofixer:\n%s"
% autofix_stderr)
stdout = autofix_stdout

lines = stdout.rstrip('\n').splitlines()
for line in lines:
self.report(line)
Expand Down
11 changes: 8 additions & 3 deletions runlint.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,12 +635,12 @@ def _find_repo_config(file_to_lint):
_DELEGATING_LINTER_CACHE = {}


def _get_delegating_linter(cmd, repo_config_dir):
def _get_delegating_linter(cmd, repo_config_dir, arc_autofix_script):
"""Return a (hopefully cached) linter running this cmd from this dir."""
key = (tuple(cmd), repo_config_dir)
if key not in _DELEGATING_LINTER_CACHE:
_DELEGATING_LINTER_CACHE[key] = linters.DelegatingLinter(
cmd, repo_config_dir, _get_logger())
cmd, repo_config_dir, arc_autofix_script, _get_logger())
return _DELEGATING_LINTER_CACHE[key]


Expand Down Expand Up @@ -704,9 +704,14 @@ def _get_linters_for_file(file_to_lint, lang, propose_arc_fixes):
# language. See if our repository does so.
(repo_config_dir, repo_config) = _find_repo_config(file_to_lint)
if repo_config:
if propose_arc_fixes:
arc_autofix_script = repo_config.get('arc-autofix-script')
else:
arc_autofix_script = None
cmds = repo_config.get('khan-linter-overrides', {}).get(file_lang)
if cmds is not None:
return [_get_delegating_linter(cmd, repo_config_dir)
return [_get_delegating_linter(cmd, repo_config_dir,
arc_autofix_script)
for cmd in cmds]

# We support multiple eslint configuration files as well as eslint
Expand Down

0 comments on commit 7c8bf0e

Please sign in to comment.