diff --git a/.eslintignore b/.eslintignore new file mode 100644 index 0000000000..77ab0c22a5 --- /dev/null +++ b/.eslintignore @@ -0,0 +1,57 @@ +# Vendor files and generated test artifacts +**/vendor +test_root/staticfiles + + +# Vendor files living outside the /vendor/ dir +*.min.js +*-min.js +*.nocache.js +**/bootstrap*.js +**/jquery*.js +**/d3*.js + + +# Translations files +**/static/js/i18n + + +# Gitignored xmodule stuff +common/static/xmodule + + +# Coffeescript directories (don't lint autogenerated files) +cms/static/coffee +lms/static/coffee +common/static/coffee +common/lib/capa/capa/tests/test_files/js + + +# Symlinks into common/lib/xmodule/xmodule/js +cms/static/xmodule_js +lms/static/xmodule_js + + +# This directory is about half Coffee and half JS, things get messy here so just ignore all existing coffee paths +common/lib/xmodule/xmodule/js/spec/annotatable/display_spec.js +common/lib/xmodule/xmodule/js/spec/capa/display_spec.js +common/lib/xmodule/xmodule/js/spec/html/edit_spec.js +common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js +common/lib/xmodule/xmodule/js/spec/problem/edit_spec_hint.js +common/lib/xmodule/xmodule/js/spec/tabs/edit.js + +common/lib/xmodule/xmodule/js/src/annotatable/display.js +common/lib/xmodule/xmodule/js/src/capa/display.js +common/lib/xmodule/xmodule/js/src/conditional/display.js +common/lib/xmodule/xmodule/js/src/discussion/display.js +common/lib/xmodule/xmodule/js/src/html/display.js +common/lib/xmodule/xmodule/js/src/html/edit.js +common/lib/xmodule/xmodule/js/src/javascript_loader.js +common/lib/xmodule/xmodule/js/src/problem/edit.js +common/lib/xmodule/xmodule/js/src/raw/edit/json.js +common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.js +common/lib/xmodule/xmodule/js/src/raw/edit/xml.js +common/lib/xmodule/xmodule/js/src/sequence/display.js +common/lib/xmodule/xmodule/js/src/sequence/edit.js +common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.js +common/lib/xmodule/xmodule/js/src/vertical/edit.js diff --git a/.eslintrc.json b/.eslintrc.json new file mode 100644 index 0000000000..f858880c0f --- /dev/null +++ b/.eslintrc.json @@ -0,0 +1,3 @@ +{ + "extends": "eslint-config-edx" +} diff --git a/package.json b/package.json index 3ab1034742..03b5ba12fa 100644 --- a/package.json +++ b/package.json @@ -18,8 +18,8 @@ }, "devDependencies": { "edx-custom-a11y-rules": "0.1.2", - "pa11y": "3.6.0", - "pa11y-reporter-1.0-json": "1.0.2", + "eslint": "^2.13.1", + "eslint-config-edx": "^1.2.0", "jasmine-core": "^2.4.1", "jasmine-jquery": "^2.1.1", "jquery": "^2.1.4", @@ -31,8 +31,10 @@ "karma-jasmine": "^0.3.8", "karma-jasmine-html-reporter": "^0.2.0", "karma-junit-reporter": "^0.4.1", - "karma-spec-reporter": "^0.0.20", "karma-requirejs": "^0.2.6", + "karma-spec-reporter": "^0.0.20", + "pa11y": "3.6.0", + "pa11y-reporter-1.0-json": "1.0.2", "plato": "1.2.2" } } diff --git a/pavelib/paver_tests/test_jshint.py b/pavelib/paver_tests/test_eslint.py similarity index 72% rename from pavelib/paver_tests/test_jshint.py rename to pavelib/paver_tests/test_eslint.py index 1b683eeb01..3d4c7b4086 100644 --- a/pavelib/paver_tests/test_jshint.py +++ b/pavelib/paver_tests/test_eslint.py @@ -8,16 +8,16 @@ import pavelib.quality from paver.easy import BuildFailure -class TestPaverJsHint(unittest.TestCase): +class TestPaverESLint(unittest.TestCase): """ - For testing run_jshint + For testing run_eslint """ def setUp(self): - super(TestPaverJsHint, self).setUp() + super(TestPaverESLint, self).setUp() # Mock the paver @needs decorator - self._mock_paver_needs = patch.object(pavelib.quality.run_jshint, 'needs').start() + self._mock_paver_needs = patch.object(pavelib.quality.run_eslint, 'needs').start() self._mock_paver_needs.return_value = 0 # Mock shell commands @@ -31,20 +31,20 @@ class TestPaverJsHint(unittest.TestCase): @patch.object(pavelib.quality, '_write_metric') @patch.object(pavelib.quality, '_prepare_report_dir') @patch.object(pavelib.quality, '_get_count_from_last_line') - def test_jshint_violation_number_not_found(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument + def test_eslint_violation_number_not_found(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument """ - run_jshint encounters an error parsing the jshint output log + run_eslint encounters an error parsing the eslint output log """ mock_count.return_value = None with self.assertRaises(BuildFailure): - pavelib.quality.run_jshint("") + pavelib.quality.run_eslint("") @patch.object(pavelib.quality, '_write_metric') @patch.object(pavelib.quality, '_prepare_report_dir') @patch.object(pavelib.quality, '_get_count_from_last_line') - def test_jshint_vanilla(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument + def test_eslint_vanilla(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument """ - jshint finds violations, but a limit was not set + eslint finds violations, but a limit was not set """ mock_count.return_value = 1 - pavelib.quality.run_jshint("") + pavelib.quality.run_eslint("") diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 80073bc596..92606f481f 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -61,7 +61,7 @@ class TestPaverQualityViolations(unittest.TestCase): class TestPaverReportViolationsCounts(unittest.TestCase): """ For testing utility functions for getting counts from reports for - run_jshint, run_complexity, run_safelint, and run_safecommit_report. + run_eslint, run_complexity, run_safelint, and run_safecommit_report. """ def setUp(self): @@ -79,16 +79,16 @@ class TestPaverReportViolationsCounts(unittest.TestCase): self.addCleanup(self._mock_paver_needs.stop) self.addCleanup(os.remove, self.f.name) - def test_get_jshint_violations_count(self): + def test_get_eslint_violations_count(self): with open(self.f.name, 'w') as f: f.write("3000 violations found") - actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "jshint") # pylint: disable=protected-access + actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # pylint: disable=protected-access self.assertEqual(actual_count, 3000) def test_get_violations_no_number_found(self): with open(self.f.name, 'w') as f: f.write("Not expected string regex") - actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "jshint") # pylint: disable=protected-access + actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # pylint: disable=protected-access self.assertEqual(actual_count, None) def test_get_violations_count_truncated_report(self): @@ -97,7 +97,7 @@ class TestPaverReportViolationsCounts(unittest.TestCase): """ with open(self.f.name, 'w') as f: f.write("foo/bar/js/fizzbuzz.js: line 45, col 59, Missing semicolon.") - actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "jshint") # pylint: disable=protected-access + actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # pylint: disable=protected-access self.assertEqual(actual_count, None) def test_complexity_value(self): @@ -274,7 +274,7 @@ class TestPaverRunQuality(unittest.TestCase): @patch('__builtin__.open', mock_open()) def test_failure_on_diffquality_pep8(self): """ - If pep8 finds errors, pylint and jshint should still be run + If pep8 finds errors, pylint and eslint should still be run """ # Mock _get_pep8_violations to return a violation _mock_pep8_violations = MagicMock( @@ -284,8 +284,8 @@ class TestPaverRunQuality(unittest.TestCase): with self.assertRaises(SystemExit): pavelib.quality.run_quality("") - # Test that pep8, pylint, and jshint were called by counting the calls to - # _get_pep8_violations (for pep8) and sh (for diff-quality pylint & jshint) + # Test that pep8, pylint, and eslint were called by counting the calls to + # _get_pep8_violations (for pep8) and sh (for diff-quality pylint & eslint) self.assertEqual(_mock_pep8_violations.call_count, 1) self.assertEqual(self._mock_paver_sh.call_count, 2) @@ -305,18 +305,18 @@ class TestPaverRunQuality(unittest.TestCase): # Test that both pep8 and pylint were called by counting the calls # Assert that _get_pep8_violations (which calls "pep8") is called once self.assertEqual(_mock_pep8_violations.call_count, 1) - # And assert that sh was called twice (for the calls to pylint & jshint). This means that even in - # the event of a diff-quality pylint failure, jshint is still called. + # And assert that sh was called twice (for the calls to pylint & eslint). This means that even in + # the event of a diff-quality pylint failure, eslint is still called. self.assertEqual(self._mock_paver_sh.call_count, 2) @patch('__builtin__.open', mock_open()) - def test_failure_on_diffquality_jshint(self): + def test_failure_on_diffquality_eslint(self): """ - If diff-quality fails on jshint, the paver task should also fail + If diff-quality fails on eslint, the paver task should also fail """ - # Underlying sh call must fail when it is running the jshint diff-quality task - self._mock_paver_sh.side_effect = CustomShMock().fail_on_jshint + # Underlying sh call must fail when it is running the eslint diff-quality task + self._mock_paver_sh.side_effect = CustomShMock().fail_on_eslint _mock_pep8_violations = MagicMock(return_value=(0, [])) with patch('pavelib.quality._get_pep8_violations', _mock_pep8_violations): with self.assertRaises(SystemExit): @@ -349,7 +349,7 @@ class TestPaverRunQuality(unittest.TestCase): pavelib.quality.run_quality("") # Assert that _get_pep8_violations (which calls "pep8") is called once self.assertEqual(_mock_pep8_violations.call_count, 1) - # And assert that sh was called twice (for the call to "pylint" & "jshint") + # And assert that sh was called twice (for the call to "pylint" & "eslint") self.assertEqual(self._mock_paver_sh.call_count, 2) @@ -370,12 +370,12 @@ class CustomShMock(object): else: return - def fail_on_jshint(self, arg): + def fail_on_eslint(self, arg): """ For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what is going to fail when we pass in a percentage ("p") requirement. """ - if "jshint" in arg: + if "eslint" in arg: # Essentially mock diff-quality exiting with 1 paver.easy.sh("exit 1") else: diff --git a/pavelib/quality.py b/pavelib/quality.py index 3882969eda..59eb5db9c7 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -257,6 +257,7 @@ def run_complexity(): print "ERROR: Unable to calculate python-only code-complexity." +# @TODO: Remove, deprecated in favor of ESLint @task @needs('pavelib.prereqs.install_node_prereqs') @cmdopts([ @@ -301,6 +302,50 @@ def run_jshint(options): ) +@task +@needs('pavelib.prereqs.install_node_prereqs') +@cmdopts([ + ("limit=", "l", "limit for number of acceptable violations"), +]) +def run_eslint(options): + """ + Runs eslint on static asset directories. + If limit option is passed, fails build if more violations than the limit are found. + """ + + eslint_report_dir = (Env.REPORT_DIR / "eslint") + eslint_report = eslint_report_dir / "eslint.report" + _prepare_report_dir(eslint_report_dir) + violations_limit = int(getattr(options, 'limit', -1)) + + sh( + "eslint --format=compact . | tee {eslint_report}".format( + eslint_report=eslint_report + ), + ignore_error=True + ) + + try: + num_violations = int(_get_count_from_last_line(eslint_report, "eslint")) + except TypeError: + raise BuildFailure( + "Error. Number of eslint violations could not be found in {eslint_report}".format( + eslint_report=eslint_report + ) + ) + + # Record the metric + _write_metric(num_violations, (Env.METRICS_DIR / "eslint")) + + # Fail if number of violations is greater than the limit + if num_violations > violations_limit > -1: + raise BuildFailure( + "ESLint Failed. Too many violations ({count}).\nThe limit is {violations_limit}.".format( + count=num_violations, violations_limit=violations_limit + ) + ) + + @task @needs('pavelib.prereqs.install_python_prereqs') @cmdopts([ @@ -451,9 +496,11 @@ def run_safecommit_report(): def _write_metric(metric, filename): """ Write a given metric to a given file - Used for things like reports/metrics/jshint, which will simply tell you the number of - jshint violations found + Used for things like reports/metrics/eslint, which will simply tell you the number of + eslint violations found """ + Env.METRICS_DIR.makedirs_p() + with open(filename, "w") as metric_file: metric_file.write(str(metric)) @@ -485,7 +532,9 @@ def _get_report_contents(filename, last_line_only=False): with open(filename, 'r') as report_file: if last_line_only: lines = report_file.readlines() - return lines[len(lines) - 1] + for line in reversed(lines): + if line != '\n': + return line else: return report_file.read() else: @@ -503,7 +552,7 @@ def _get_count_from_last_line(filename, file_type): # Example of the last line of a complexity report: "Average complexity: A (1.93953443446)" regex = r'\d+.\d+' else: - # Example of the last line of a jshint report (for example): "3482 errors" + # Example of the last line of a compact-formatted eslint report (for example): "62829 problems" regex = r'^\d+' try: @@ -663,9 +712,14 @@ def run_quality(options): pylint_files = get_violations_reports("pylint") pylint_reports = u' '.join(pylint_files) + + # @TODO: Remove, deprecated in favor of ESLint jshint_files = get_violations_reports("jshint") jshint_reports = u' '.join(jshint_files) + eslint_files = get_violations_reports("eslint") + eslint_reports = u' '.join(eslint_files) + pythonpath_prefix = ( "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:cms:cms/djangoapps:" "common:common/djangoapps:common/lib" @@ -682,6 +736,7 @@ def run_quality(options): ): diff_quality_percentage_pass = False + # @TODO: Remove, deprecated in favor of ESLint # run diff-quality for jshint. if not run_diff_quality( violations_type="jshint", @@ -693,6 +748,17 @@ def run_quality(options): ): diff_quality_percentage_pass = False + # run diff-quality for eslint. + if not run_diff_quality( + violations_type="eslint", + prefix=pythonpath_prefix, + reports=eslint_reports, + percentage_string=percentage_string, + branch_string=compare_branch_string, + dquality_dir=dquality_dir + ): + diff_quality_percentage_pass = False + # If one of the quality runs fails, then paver exits with an error when it is finished if not diff_quality_percentage_pass: raise BuildFailure("Diff-quality failure(s).") @@ -702,7 +768,7 @@ def run_diff_quality( violations_type=None, prefix=None, reports=None, percentage_string=None, branch_string=None, dquality_dir=None ): """ - This executes the diff-quality commandline tool for the given violation type (e.g., pylint, jshint). + This executes the diff-quality commandline tool for the given violation type (e.g., pylint, eslint). If diff-quality fails due to quality issues, this method returns False. """ diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d7c793ccb9..cb79d955e5 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -146,7 +146,7 @@ bok-choy==0.5.3 chrono==1.0.2 coverage==4.0.2 ddt==0.8.0 -diff-cover==0.9.0 +diff-cover==0.9.8 django-crum==0.5 django_nose==1.4.1 factory_boy==2.5.1 diff --git a/scripts/all-tests.sh b/scripts/all-tests.sh index d1172e6c69..68cfe355dd 100755 --- a/scripts/all-tests.sh +++ b/scripts/all-tests.sh @@ -12,7 +12,8 @@ set -e # Violations thresholds for failing the build export PYLINT_THRESHOLD=4175 -export JSHINT_THRESHOLD=7550 +export JSHINT_THRESHOLD=7550 # @TODO Remove, deprecated in favor of ESLint +export ESLINT_THRESHOLD=49019 SAFELINT_THRESHOLDS=`cat scripts/safelint_thresholds.json` export SAFELINT_THRESHOLDS=${SAFELINT_THRESHOLDS//[[:space:]]/} diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh index b9e1683cde..e9738a4455 100755 --- a/scripts/circle-ci-tests.sh +++ b/scripts/circle-ci-tests.sh @@ -60,10 +60,15 @@ else paver run_pylint -l $PYLINT_THRESHOLD | tee pylint.log || EXIT=1 mkdir -p reports - echo "Finding jshint violations and storing report..." PATH=$PATH:node_modules/.bin + + # @TODO: Remove, deprecated in favor of ESLint + echo "Finding JSHint violations and storing report..." paver run_jshint -l $JSHINT_THRESHOLD > jshint.log || { cat jshint.log; EXIT=1; } + echo "Finding ESLint violations and storing report..." + paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; } + # Run quality task. Pass in the 'fail-under' percentage to diff-quality paver run_quality -p 100 || EXIT=1 diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index cc44f1e21f..24a289da37 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -80,8 +80,14 @@ case "$TEST_SUITE" in paver run_pylint -l $PYLINT_THRESHOLD > pylint.log || { cat pylint.log; EXIT=1; } mkdir -p reports - echo "Finding jshint violations and storing report..." + + # @TODO: Remove, deprecated in favor of ESLint + echo "Finding JSHint violations and storing report..." paver run_jshint -l $JSHINT_THRESHOLD > jshint.log || { cat jshint.log; EXIT=1; } + + echo "Finding ESLint violations and storing report..." + paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; } + echo "Running code complexity report (python)." paver run_complexity > reports/code_complexity.log || echo "Unable to calculate code complexity. Ignoring error." echo "Running safe template linter report." diff --git a/scripts/safe_template_linter.py b/scripts/safe_template_linter.py index 7885fdde73..9a50339278 100755 --- a/scripts/safe_template_linter.py +++ b/scripts/safe_template_linter.py @@ -543,7 +543,7 @@ class SummaryResults(object): print("{}: {}{} violations".format(rule_id, padding, self.totals_by_rule[rule_id]), file=out) print("", file=out) - # matches output of jshint for simplicity + # matches output of eslint for simplicity print("", file=out) print("{} violations total".format(self.total_violations), file=out)