Skip to content

Commit

Permalink
Fix up khan-linter to be python3-only. (#42)
Browse files Browse the repository at this point in the history
## Summary:
Now that we're on python3 everywhere, we don't need `six` anymore.  Or the `vendored/py2` directory.

I also changed all our shebang lines to say `python3` explicitly,
rather than `python`.  This isn't really needed, but some folks don't
even have a /usr/bin/python on their machine, only /usr/bin/python3!
And we might as well be clear.

While testing this, I discovered a few lint errors that are new to new
versions of python, that I cleaned up.  I also discovered that the
vendored pyflakes we have is too old for modern pythons, so I updated
it, and also mccabe while I was at it.

Issue: https://khanacademy.slack.com/archives/C02NMB1R5/p1710546934835669

## Test plan:
./runlint_test.py

Author: csilvers

Reviewers: MiguelCastillo

Required Reviewers:

Approved By: MiguelCastillo

Checks:

Pull Request URL: #42
  • Loading branch information
csilvers authored Apr 3, 2024
1 parent 229e73f commit b195476
Show file tree
Hide file tree
Showing 492 changed files with 7,022 additions and 105,713 deletions.
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
deps vendor_deps: go_deps check_setup
rm -r vendor/py2/* || true
rm -r vendor/py3/* || true
pip2 install --target=vendor/py2 -r requirements.txt
pip3 install --target=vendor/py3 -r requirements.txt
[ -d vendor/py2/backports ] && ! [ -s vendor/py2/backports/__init__.py ] && echo "from pkgutil import extend_path; __path__ = extend_path(__path__, __name__)" > vendor/py2/backports/__init__.py
npm install
npm update
npm prune
Expand Down
19 changes: 9 additions & 10 deletions githook.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
#!/usr/bin/env python3

"""Hooks for git that perform linting.
This file contains two git hooks:
Expand Down Expand Up @@ -37,8 +38,6 @@
import subprocess
import sys

import six

import hook_lib


Expand All @@ -60,7 +59,7 @@ def commit_msg_hook(commit_message_file):

# If the commit message is empty or unchanged from the template, abort.
if not commit_message:
six.print_("Aborting commit, empty commit message")
print("Aborting commit, empty commit message")
return 1

try:
Expand All @@ -70,7 +69,7 @@ def commit_msg_hook(commit_message_file):
pass
else:
if commit_message == _normalized_commit_message(template):
six.print_("Aborting commit, commit message unchanged")
print("Aborting commit, commit message unchanged")
return 1

# If we're a merge, don't try to do a lint-check.
Expand Down Expand Up @@ -181,21 +180,21 @@ def pre_push_hook(_unused_arg_remote_name, _unused_arg_remote_location):
# Lint the files, if any. If there are any errors, print a helpful
# message, and return a nonzero status code to abort the push.
if files_to_lint:
six.print_("khan-linter: linting {} files with unpushed "
"changes...".format(len(files_to_lint)))
print("khan-linter: linting {} files with unpushed "
"changes...".format(len(files_to_lint)))
num_errors = hook_lib.lint_files(files_to_lint)
if num_errors > 0:
six.print_(
print(
'\n--- %s lint errors. Push aborted. ---' % num_errors,
file=sys.stderr)
six.print_(
print(
'Running `make fixc` may help to autofix the errors.',
file=sys.stderr)
return 1

# Yay, everything went okay! Return a zero status code, in order to
# allow the push.
six.print_('khan-linter: all lint checks passed', file=sys.stderr)
print('khan-linter: all lint checks passed', file=sys.stderr)
return 0


Expand Down
29 changes: 12 additions & 17 deletions hook_lib.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#!/usr/bin/env python

"""Library for code shared by hghook.py and githook.py.
This library does the actual linting, after the VCS-specific commands
Expand All @@ -10,8 +8,6 @@
import subprocess
import sys

import six


def lint_files(files_to_lint):
"""Given a list of filenames in the commit, lint them all.
Expand Down Expand Up @@ -49,18 +45,17 @@ def lint_commit_message(commit_message):
num_errors = 0

if not re.search('^(test plan|review):', commit_message, re.I | re.M):
six.print_('Missing "Test plan:" or "Review:" section '
'in the commit message.', file=sys.stderr)
print('Missing "Test plan:" or "Review:" section '
'in the commit message.', file=sys.stderr)
num_errors += 1

elif re.search('^ <see below>$', commit_message, re.M):
six.print_('Must enter a "Test plan:" (or "Review:") '
'in the commit message.', file=sys.stderr)
print('Must enter a "Test plan:" (or "Review:") '
'in the commit message.', file=sys.stderr)
num_errors += 1

if re.search('^<one-line summary, followed by ', commit_message, re.M):
six.print_('Must enter a summary in the commit message.',
file=sys.stderr)
print('Must enter a summary in the commit message.', file=sys.stderr)
num_errors += 1

# TODO(csilvers): verify the first-line summary is actually 1 line long?
Expand All @@ -77,12 +72,12 @@ def report_errors_and_exit(num_errors, commit_message, save_filename):
# save the commit message so we don't need to retype it
with open(save_filename, 'w') as f:
f.write(commit_message)
six.print_('\n--- %s commit message errors ---\n'
'Commit message saved to %s'
% (num_errors, save_filename),
file=sys.stderr)
six.print_('Use "git commit -a --template .git/commit.save" to commit'
' with a fixed message.', file=sys.stderr)
print('\n--- %s commit message errors ---\n'
'Commit message saved to %s'
% (num_errors, save_filename),
file=sys.stderr)
print('Use "git commit -a --template .git/commit.save" to commit'
' with a fixed message.', file=sys.stderr)
sys.exit(1)
six.print_('khan-linter: commit message passed', file=sys.stderr)
print('khan-linter: commit message passed', file=sys.stderr)
sys.exit(0)
39 changes: 15 additions & 24 deletions lint_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,16 @@
import re
import sys

import six


if six.PY2:
def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if isinstance(text, unicode):
text = text.encode('utf-8')
six.print_(text, end=end)
else:
def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if end is None:
end = os.linesep
if isinstance(text, str):
text = (text + end).encode('utf-8')
else: # already bytes
text += end.encode('utf-8')
sys.stdout.buffer.write(text)

def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if end is None:
end = os.linesep
if isinstance(text, str):
text = (text + end).encode('utf-8')
else: # already bytes
text += end.encode('utf-8')
sys.stdout.buffer.write(text)


def get_real_cwd():
Expand Down Expand Up @@ -56,13 +47,13 @@ def add_arc_fix_str(lintline, bad_line, to_remove, to_add,
# that they can interoperate, and ensures that we use byte indexes instead
# of Unicode character indexes (because the autopatcher expects the
# former).
if isinstance(lintline, unicode):
if isinstance(lintline, str):
lintline = lintline.encode('utf-8')
if isinstance(bad_line, unicode):
if isinstance(bad_line, str):
bad_line = bad_line.encode('utf-8')
if isinstance(to_add, unicode):
if isinstance(to_add, str):
to_add = to_add.encode('utf-8')
if isinstance(to_remove, unicode):
if isinstance(to_remove, str):
to_remove = to_remove.encode('utf-8')

(location, errcode, msg) = lintline.split(' ', 2)
Expand All @@ -76,7 +67,7 @@ def add_arc_fix_str(lintline, bad_line, to_remove, to_add,
if not to_remove:
return lintline + "\0\0%s\0" % to_add

if not isinstance(to_remove, basestring):
if not isinstance(to_remove, str):
# `to_remove` is a regexp.
if search_backwards:
# Match the last instance of `to_remove` before col.
Expand Down
15 changes: 4 additions & 11 deletions linters.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
"""Linters process files or lists of files for correctness."""

import io
import itertools
import json
import logging
Expand All @@ -11,20 +12,12 @@
import sys

import lint_util
import six
import six.moves

from six.moves import cStringIO as StringIO

# Add vendor path so we can find (our packaged versions of) flake8
_CWD = lint_util.get_real_cwd()
_parent_dir = os.path.abspath(_CWD)
if six.PY2:
_vendor_version = 'py2'
else:
_vendor_version = 'py3'

_vendor_dir = os.path.join(_parent_dir, 'vendor', _vendor_version)
_vendor_dir = os.path.join(_parent_dir, 'vendor', 'py3')
sys.path.insert(0, _vendor_dir)

import static_content_refs
Expand Down Expand Up @@ -170,7 +163,7 @@ def _capture_stdout_of(fn, *args, **kwargs):
"""Call fn(*args, **kwargs) and return (fn_retval, fn_stdout_output_fp)."""
try:
orig_stdout = sys.stdout
sys.stdout = StringIO()
sys.stdout = io.StringIO()
retval = fn(*args, **kwargs)
sys.stdout.seek(0) # so new read()/readlines() calls will return
return (retval, sys.stdout)
Expand Down Expand Up @@ -675,7 +668,7 @@ def lint_files(self, files):
stdout_lines = []
# We need to keep sum(|files|) less than about 250k for OS X's
# commandline limit. 2000 files at a time should do that.
for i in six.moves.range(0, len(files), 2000):
for i in range(0, len(files), 2000):
stdout_lines.extend(self._run_eslint(files[i:i + 2000]))

# eslint_reporter specifies that errors are reported on
Expand Down
24 changes: 0 additions & 24 deletions python_version_compat_test.py

This file was deleted.

5 changes: 2 additions & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
# to the /vendor/* directory in with your change as well.

# Required by python lint checking
# This is flake8 3.5.0 with one more commit to support `paths` in `.flake8`.
git+git://github.com/pycqa/flake8.git@dd1e9d1cb7e9a232946c06aca1564d48d4d6f65e
flake8==3.9.2

# Required for yaml lint checking
pyyaml==3.12
pyyaml==6.0.1


15 changes: 6 additions & 9 deletions runlint.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3

"""Run some linters on files of various types."""

Expand Down Expand Up @@ -43,9 +43,6 @@

import linters
import lint_util
import six

from six.moves import xrange


_DEFAULT_BLACKLIST_PATTERN = '<ancestor>/lint_blacklist.txt'
Expand Down Expand Up @@ -158,7 +155,7 @@ def _extended_fnmatch_compile(pattern):

# If the code below this line has horrible syntax highlighting, check
# this out: http://stackoverflow.com/questions/13210816/sublime-texts-syntax-highlighting-of-regexes-in-python-leaks-into-surrounding-c
_METACHAR_RE = re.compile(r'[[*?!]')
_METACHAR_RE = re.compile(r'[*?![]')


def _parse_one_blacklist_line(line):
Expand Down Expand Up @@ -320,7 +317,7 @@ def _file_in_blacklist(fname, blacklist_pattern):
# The blacklist can have regexp patterns in it, so we need to
# check those too, one by one:
for blacklist_entry in blacklist:
if not isinstance(blacklist_entry, six.string_types):
if not isinstance(blacklist_entry, str):
if blacklist_entry.match(fname):
return True

Expand All @@ -335,7 +332,7 @@ def _files_under_directory(rootdir, blacklist_pattern):
# backwards so we can use del. (Weird os.walk() semantics:
# calling del on an element of dirs suppresses os.walk()'s
# traversal into that dir.)
for i in xrange(len(dirs) - 1, -1, -1):
for i in range(len(dirs) - 1, -1, -1):
absdir = os.path.join(root, dirs[i])
if os.path.islink(absdir):
_get_logger().debug('... skipping directory %s: is a symlink'
Expand Down Expand Up @@ -458,7 +455,7 @@ def _run_extra_linter(extra_linter_filename, files, verbose):
if linter:
linter_to_files.setdefault(linter, set()).add(f)

for (linter_filename, files) in six.iteritems(linter_to_files):
for (linter_filename, files) in linter_to_files.items():
if not os.access(linter_filename, os.R_OK | os.X_OK):
continue
files = sorted(files)
Expand Down Expand Up @@ -822,7 +819,7 @@ def main(files_and_directories,
except Exception:
_get_logger().error(u"ERROR linting %r with %s:\n%s" % (
files, type(lint_processor),
traceback.format_exc().decode('utf-8')))
traceback.format_exc()))
num_framework_errors += 1
continue

Expand Down
2 changes: 1 addition & 1 deletion runlint_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3

import os
import unittest
Expand Down
Loading

0 comments on commit b195476

Please sign in to comment.