GitHub user 1ambda opened a pull request: https://github.com/apache/zeppelin/pull/1887
[ZEPPELIN-1940] most of eslint rules are NOT applied at all ### What is this PR for? **Most fixes are about applying lint rules. It's automatically fixed using `eslint --fix` command.** **So please review `Gruntfile.js`, `.eslint`, `package.json` in `zeppelin-web` directory** As a result of the PR, - Restored javascript lint system - Fixed 1500+ lint errors - Found some buggy code (e.g `Unexpected 'this' no-invalid-this` See the screenshot section) We are using google style guide but it is not used at all because of invalid `.eslint` configuration. Thus I made some changes to fix it. - Moved eslint from grunt to webpack (faster) - Changed invalid config `"preset": "google"` to `"extends": ["google"]` - Fixed 1500+ lint errors automatically using `eslint --fix` command - Left some lint errors i cannot fix immediately as warnings or disabled ``` "guard-for-in": [1], // warning "no-invalid-this": [1], // warning "prefer-rest-params": [1], // warning "require-jsdoc": [0], // disabled "valid-jsdoc": [0] // disabled ``` - Also, Modified 20+ lint errors by hand ``` @ ./src/index.js 29:0-65 ERROR in ./src/app/visualization/builtins/visualization-areachart.js Module build failed: Duplicate declaration "self" 73 | this.chart.style(this.config.style || 'stack'); 74 | > 75 | let self = this; | ^ /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/visualization.js 134:14 error Unexpected var, use let or const instead no-var 138:14 error Unexpected var, use let or const instead no-var ... ``` - Additionally, introduced `eslint:recommended` ruleset for several rules which google style is not opinionated so that we can keep clear, unified rules. ### What type of PR is it? [Bug Fix | Improvement] ### Todos * [x] - Moved eslint (build) task from grunt to npm (package.json) * [x] - Brought eslint (dev) task from grunt to webpack * [x] - Fixed `.eslint` to applied google ruleset * [x] - Modifed some lint errors while leaving others as warning which i cannot fix right now (e.g docs) * [x] - Introduced eslint recommended ruleset ### What is the Jira issue? [ZEPPELIN-1940](https://issues.apache.org/jira/browse/ZEPPELIN-1940) ### How should this be tested? - execute `npm run dev` and `npm run build` to check whether lint works well ### Screenshots (if appropriate) ### Image: found some invalid code using restored lint ![image](https://cloud.githubusercontent.com/assets/4968473/21866851/2937c408-d88f-11e6-9385-b9fcca6ce477.png) ### Image: found some errors using restored lint ![image](https://cloud.githubusercontent.com/assets/4968473/21866822/10966dfa-d88f-11e6-803c-bfe6f12960dd.png) ### Image: Some code fixed by hand ``` â zeppelin-web git:(remove-grunt-eslint) npm run lint:js -- --quiet > zeppelin-web@0.0.0 lint:js /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web > eslint src test Gruntfile.js "--quiet" /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/handsontable/handsonHelper.js 159:58 error Use the rest parameters instead of 'arguments' prefer-rest-params 163:55 error Use the rest parameters instead of 'arguments' prefer-rest-params /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/notebook/notebook.controller.js 635:20 error Unexpected var, use let or const instead no-var 648:20 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js 929:28 error Use the rest parameters instead of 'arguments' prefer-rest-params 930:66 error Use the rest parameters instead of 'arguments' prefer-rest-params /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js 386:9 error Unexpected var, use let or const instead no-var 433:9 error Unexpected var, use let or const instead no-var 453:9 error Unexpected var, use let or const instead no-var 867:28 error Use the rest parameters instead of 'arguments' prefer-rest-params 868:66 error Use the rest parameters instead of 'arguments' prefer-rest-params /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/tabledata/transformation.js 52:7 error Unexpected var, use let or const instead no-var 54:14 error Unexpected var, use let or const instead no-var 58:14 error Unexpected var, use let or const instead no-var 77:7 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/builtins/visualization-areachart.js 61:5 error Unexpected var, use let or const instead no-var 73:5 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/builtins/visualization-barchart.js 59:5 error Unexpected var, use let or const instead no-var 67:5 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/builtins/visualization-nvd3chart.js 225:12 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/builtins/visualization-scatterchart.js 134:10 error Unexpected var, use let or const instead no-var /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/visualization.js 134:14 error Unexpected var, use let or const instead no-var 138:14 error Unexpected var, use let or const instead no-var â 23 problems (23 errors, 0 warnings) ERROR in ./src/app/visualization/builtins/visualization-barchart.js Module build failed: Duplicate declaration "self" 65 | this.chart.stacked(this.config.stacked); 66 | > 67 | let self = this; | ^ 68 | this.chart.dispatch.on('stateChange', function(s) { 69 | self.config.stacked = s.stacked; 70 | @ ./src/index.js 29:0-65 ERROR in ./src/app/visualization/builtins/visualization-areachart.js Module build failed: Duplicate declaration "self" 73 | this.chart.style(this.config.style || 'stack'); 74 | > 75 | let self = this; | ^ 76 | this.chart.dispatch.on('stateChange', function(s) { 77 | self.config.style = s.style; 78 | /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/src/app/visualization/builtins/visualization-nvd3chart.js 52:25 error Empty block statement no-empty â 1 problem (1 error, 0 warnings) ``` ### Questions: * Does the licenses files need update? - NO * Is there breaking changes for older versions? - NO * Does this needs documentation? - NO You can merge this pull request into a Git repository by running: $ git pull https://github.com/1ambda/zeppelin ZEPPELIN-1940/apply-google-eslint-rule Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/1887.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1887 ---- commit 9680f968eab4a428924992d13cea8d7037121d7e Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T10:30:40Z fix: Add lint:js commit f4c37185c3d65e4185e4b196fa19ed77dc1f650f Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T10:41:37Z fix: Add eslint-loader commit 3230d5252f70939b3079ebf85496e2676b69505f Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T10:47:28Z fix: Lint errors > zeppelin-web@0.0.0 lint:js /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web > eslint src test Gruntfile.js /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/karma.conf.js 9:3 error 'use strict' is unnecessary inside of modules strict /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/controllers/main.js 7:14 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/controllers/nav.js 7:14 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/controllers/notebook.js 25:14 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/controllers/notename.js 8:14 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/controllers/paragraph.js 18:14 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/directives/ngenter.js 9:14 error 'inject' is not defined no-undef 13:26 error 'inject' is not defined no-undef /Users/lambda/github/apache-zeppelin/zeppelin-master/zeppelin-web/test/spec/factory/noteList.js 8:5 error 'inject' is not defined no-undef â 9 problems (9 errors, 0 warnings) commit 92ce71fd767ba095939d2d593135e6dbf7e86730 Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T11:13:49Z fix: lint errors in src, test commit 840cd456125fcfe23e110dc8ad0b82d8e0e6900e Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T11:22:04Z fix: no-var, duplicated-self rules commit e4aa14e27a9e2eb874bb0366a9be1e3ea693a285 Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T11:34:51Z fix: Add eslint:recommended config commit be0ec5f476d97c97e771a09bdde74b6b7a4b04c8 Author: 1ambda <1am...@gmail.com> Date: 2017-01-11T19:59:02Z fix: Remove lint js fix command ---- --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---