Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref: workaround flake8 B314 #18116

Merged
merged 2 commits into from
Apr 7, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 87 additions & 82 deletions src/sentry/lint/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
For the js only path, we should not depend on any packages outside the
python stdlib to prevent the need to install the world just to run eslint.
"""
from __future__ import absolute_import, print_function
from __future__ import absolute_import


import os
Expand All @@ -17,28 +17,28 @@

from subprocess import check_output, Popen

os.environ['PYFLAKES_NODOCTEST'] = '1'
os.environ['SENTRY_PRECOMMIT'] = '1'
os.environ["PYFLAKES_NODOCTEST"] = "1"
os.environ["SENTRY_PRECOMMIT"] = "1"


def get_project_root():
return os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, os.pardir)


def get_sentry_bin(name):
return os.path.join(get_project_root(), 'bin', name)
return os.path.join(get_project_root(), "bin", name)


def get_node_modules():
return os.path.join(get_project_root(), 'node_modules')
return os.path.join(get_project_root(), "node_modules")


def get_node_modules_bin(name):
return os.path.join(get_node_modules(), '.bin', name)
return os.path.join(get_node_modules(), ".bin", name)


def get_prettier_path():
return get_node_modules_bin('prettier')
return get_node_modules_bin("prettier")


def get_files(path):
Expand All @@ -52,14 +52,14 @@ def get_files(path):
def get_modified_files(path):
return [
s
for s in check_output(['git', 'diff-index', '--cached', '--name-only', 'HEAD']).split('\n')
for s in check_output(["git", "diff-index", "--cached", "--name-only", "HEAD"]).split("\n")
if s
]


def get_files_for_list(file_list):
if file_list is None:
files_to_check = get_files('.')
files_to_check = get_files(".")

else:
files_to_check = []
Expand All @@ -73,49 +73,46 @@ def get_files_for_list(file_list):

def get_js_files(file_list=None, snapshots=False):
if snapshots:
extensions = ('.js', '.jsx', '.ts', '.tsx', '.jsx.snap', '.js.snap')
extensions = (".js", ".jsx", ".ts", ".tsx", ".jsx.snap", ".js.snap")
else:
extensions = ('.js', '.jsx', '.ts', '.tsx')
extensions = (".js", ".jsx", ".ts", ".tsx")

if file_list is None:
file_list = ['tests/js', 'src/sentry/static/sentry/app']
return [
x for x in get_files_for_list(file_list)
if x.endswith(extensions)
]
file_list = ["tests/js", "src/sentry/static/sentry/app"]
return [x for x in get_files_for_list(file_list) if x.endswith(extensions)]


def get_less_files(file_list=None):
if file_list is None:
file_list = ['src/sentry/static/sentry/less', 'src/sentry/static/sentry/app']
return [x for x in get_files_for_list(file_list) if x.endswith(('.less'))]
file_list = ["src/sentry/static/sentry/less", "src/sentry/static/sentry/app"]
return [x for x in get_files_for_list(file_list) if x.endswith((".less"))]


def js_lint(file_list=None, parseable=False, format=False):

# We require eslint in path but we actually call an eslint wrapper
eslint_path = get_node_modules_bin('eslint')
eslint_path = get_node_modules_bin("eslint")

# Note, in CI, we run a relaxed version of our eslint rules (.eslint.relax.js)
eslint_wrapper_path = get_sentry_bin('eslint-travis-wrapper')
eslint_wrapper_path = get_sentry_bin("eslint-travis-wrapper")

if not os.path.exists(eslint_path):
print('!! Skipping JavaScript linting because eslint is not installed.') # noqa: B314
sys.stdout.write("!! Skipping JavaScript linting because eslint is not installed.\n")
return False

js_file_list = get_js_files(file_list, snapshots=True)

has_errors = False
if js_file_list:
if os.environ.get('CI'):
cmd = [eslint_wrapper_path, '--ext', '.js,.jsx,.ts,.tsx']
if os.environ.get("CI"):
cmd = [eslint_wrapper_path, "--ext", ".js,.jsx,.ts,.tsx"]
else:
cmd = [eslint_path, '--ext', '.js,.jsx,.ts,.tsx']
cmd = [eslint_path, "--ext", ".js,.jsx,.ts,.tsx"]

if format:
cmd.append('--fix')
cmd.append("--fix")
if parseable:
cmd.append('--format=checkstyle')
cmd.append("--format=checkstyle")

status = Popen(cmd + js_file_list).wait()
has_errors = status != 0
Expand All @@ -128,10 +125,12 @@ def js_stylelint(file_list=None, parseable=False, format=False):
stylelint for styled-components
"""

stylelint_path = get_node_modules_bin('stylelint')
stylelint_path = get_node_modules_bin("stylelint")

if not os.path.exists(stylelint_path):
print('!! Skipping JavaScript styled-components linting because "stylelint" is not installed.') # noqa: B314
sys.stdout.write(
'!! Skipping JavaScript styled-components linting because "stylelint" is not installed.\n'
)
return False

js_file_list = get_js_files(file_list, snapshots=False)
Expand All @@ -155,45 +154,49 @@ def yarn_check(file_list):
without a Yarn lockfile change, e.g. Jest config changes, license changes, etc.
"""

if file_list is None or os.environ.get('SKIP_YARN_CHECK'):
if file_list is None or os.environ.get("SKIP_YARN_CHECK"):
return False

if 'package.json' in file_list and 'yarn.lock' not in file_list:
print( # noqa: B314
'\033[33m' + """Warning: package.json modified without accompanying yarn.lock modifications.
if "package.json" in file_list and "yarn.lock" not in file_list:
sys.stdout.write(
"\033[33m"
+ """Warning: package.json modified without accompanying yarn.lock modifications.

If you updated a dependency/devDependency in package.json, you must run `yarn install` to update the lockfile.

To skip this check, run `SKIP_YARN_CHECK=1 git commit [options]`""" + '\033[0m')
To skip this check, run `SKIP_YARN_CHECK=1 git commit [options]`"""
+ "\033[0m"
+ "\n"
)
return True

return False


def is_prettier_valid(project_root, prettier_path):
if not os.path.exists(prettier_path):
print('[sentry.lint] Skipping JavaScript formatting because prettier is not installed.', file=sys.stderr) # noqa: B314
sys.stderr.write(
"[sentry.lint] Skipping JavaScript formatting because prettier is not installed.\n"
)
return False

# Get Prettier version from package.json
package_version = None
package_json_path = os.path.join(project_root, 'package.json')
package_json_path = os.path.join(project_root, "package.json")
with open(package_json_path) as package_json:
try:
package_version = json.load(package_json)[
'devDependencies']['prettier']
package_version = json.load(package_json)["devDependencies"]["prettier"]
except KeyError:
print('!! Prettier missing from package.json', file=sys.stderr) # noqa: B314
sys.stderr.write("!! Prettier missing from package.json\n")
return False

prettier_version = subprocess.check_output(
[prettier_path, '--version']).rstrip()
prettier_version = subprocess.check_output([prettier_path, "--version"]).rstrip()
if prettier_version != package_version:
print( # noqa: B314
u'[sentry.lint] Prettier is out of date: {} (expected {}). Please run `yarn install`.'.format(
prettier_version,
package_version),
file=sys.stderr)
sys.stderr.write(
u"[sentry.lint] Prettier is out of date: {} (expected {}). Please run `yarn install`.\n".format(
prettier_version, package_version
)
)
return False

return True
Expand All @@ -204,12 +207,14 @@ def js_lint_format(file_list=None):
We only format JavaScript code as part of this pre-commit hook. It is not part
of the lint engine. This uses eslint's `--fix` formatting feature.
"""
eslint_path = get_node_modules_bin('eslint')
eslint_path = get_node_modules_bin("eslint")
project_root = get_project_root()
prettier_path = get_prettier_path()

if not os.path.exists(eslint_path):
print('!! Skipping JavaScript linting and formatting because eslint is not installed.') # noqa: B314
sys.stdout.write(
"!! Skipping JavaScript linting and formatting because eslint is not installed.\n"
)
return False

if not is_prettier_valid(project_root, prettier_path):
Expand All @@ -218,14 +223,13 @@ def js_lint_format(file_list=None):
js_file_list = get_js_files(file_list)

# manually exclude some bad files
js_file_list = [x for x in js_file_list if '/javascript/example-project/' not in x]
cmd = [eslint_path, '--fix', ]

has_package_json_errors = False if 'package.json' not in file_list else run_formatter(
[
prettier_path,
'--write',
], ['package.json']
js_file_list = [x for x in js_file_list if "/javascript/example-project/" not in x]
cmd = [eslint_path, "--fix"]

has_package_json_errors = (
False
if "package.json" not in file_list
else run_formatter([prettier_path, "--write"], ["package.json"])
)

has_errors = run_formatter(cmd, js_file_list)
Expand All @@ -237,17 +241,19 @@ def js_test(file_list=None):
"""
Run JavaScript unit tests on relevant files ONLY as part of pre-commit hook
"""
jest_path = get_node_modules_bin('jest')
jest_path = get_node_modules_bin("jest")

if not os.path.exists(jest_path):
print('[sentry.test] Skipping JavaScript testing because jest is not installed.') # noqa: B314
sys.stdout.write(
"[sentry.test] Skipping JavaScript testing because jest is not installed.\n"
)
return False

js_file_list = get_js_files(file_list)

has_errors = False
if js_file_list:
status = Popen(['./bin/yarn', 'test-precommit'] + js_file_list).wait()
status = Popen(["./bin/yarn", "test-precommit"] + js_file_list).wait()
has_errors = status != 0

return has_errors
Expand All @@ -265,12 +271,7 @@ def less_format(file_list=None):
return False

less_file_list = get_less_files(file_list)
return run_formatter(
[
prettier_path,
'--write',
], less_file_list
)
return run_formatter([prettier_path, "--write"], less_file_list)


def run_formatter(cmd, file_list, prompt_on_changes=True):
Expand All @@ -285,35 +286,39 @@ def run_formatter(cmd, file_list, prompt_on_changes=True):
return True

# this is not quite correct, but it at least represents what would be staged
output = subprocess.check_output(['git', 'diff', '--color'] + file_list)
output = subprocess.check_output(["git", "diff", "--color"] + file_list)
if output:
print('[sentry.lint] applied changes from autoformatting') # noqa: B314
print(output) # noqa: B314
sys.stdout.write("[sentry.lint] applied changes from autoformatting\n")
sys.stdout.write(output)
if prompt_on_changes:
with open('/dev/tty') as fp:
print('\033[1m' + 'Stage this patch and continue? [Y/n] ' + '\033[0m') # noqa: B314
if fp.readline().strip() not in ('Y', 'y', ''):
print( # noqa: B314
'[sentry.lint] Unstaged changes have not been staged.', file=sys.stderr)
if not os.environ.get('SENTRY_SKIP_FORCE_PATCH'):
print('[sentry.lint] Aborted!', file=sys.stderr) # noqa: B314
with open("/dev/tty") as fp:
sys.stdout.write("\033[1m" + "Stage this patch and continue? [Y/n] " + "\033[0m\n")
if fp.readline().strip() not in ("Y", "y", ""):
sys.stderr.write("[sentry.lint] Unstaged changes have not been staged.\n")
if not os.environ.get("SENTRY_SKIP_FORCE_PATCH"):
sys.stderr.write("[sentry.lint] Aborted!\n")
sys.exit(1)
else:
status = subprocess.Popen(
['git', 'update-index', '--add'] + file_list).wait()
status = subprocess.Popen(["git", "update-index", "--add"] + file_list).wait()
has_errors = status != 0
return has_errors


def run(file_list=None, format=True, lint=True, js=True, py=True,
less=True, yarn=True, test=False, parseable=False):
def run(
file_list=None,
format=True,
lint=True,
js=True,
py=True,
less=True,
yarn=True,
test=False,
parseable=False,
):
old_sysargv = sys.argv

try:
sys.argv = [
os.path.join(os.path.dirname(__file__),
os.pardir, os.pardir, os.pardir)
]
sys.argv = [os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, os.pardir)]
results = []

# packages
Expand Down