Bug 937431 - [Tooling] make jshint a requirement on pre-commit r=rik

* changes how `make lint` works * introduces a jshint-blacklisted reporter * changes the pre-commit hook to run both jshint and gjslint * changes the linters Travis jobs * remove all predefined globals from the main .jshintrc default global * add new jshintrc files for subdirectories * add a README file in build/jshint/
apc-io · Jan 22, 2014 · a2ae854 · a2ae854
1 parent 90de6ce
commit a2ae854
Show file tree

Hide file tree

Showing 54 changed files with 2,356 additions and 35 deletions.
diff --git a/.jshintignore b/.jshintignore
@@ -4,6 +4,7 @@ apps/email/build/**
 apps/email/built/**
 apps/email/js/ext/**
 apps/calendar/js/ext/**
+apps/system/camera/**
 build/r.js
 apps/communications/contacts/oauth2/js/parameters.js
 apps/calendar/js/presets.js
@@ -12,8 +13,16 @@ apps/email/js/tmpl_builder.js
 apps/clock/js/alameda.js
 shared/js/opensearch.js
 apps/keyboard/js/imes/jspinyin/libpinyin.js
+apps/keyboard/js/imes/jszhuyin/lib/**
 shared/js/setImmediate.js
 apps/gallery/test/unit/setImmediate_test.js
 apps/wappush/js/ext/**
 apps/keyboard/js/imes/jsavrophonetic/jsavrophonetic.js
 apps/camera/js/vendor/alameda.js
+apps/camera/js/config.js
+apps/gallery/js/config.js
+apps/gallery/js/frame_scripts.js
+apps/gallery/js/metadata_scripts.js
+apps/keyboard/js/layouts/**
+apps/music/js/metadata_scripts.js
+apps/keyboard/js/imes/jszhuyin/tools/cook.js
diff --git a/.jshintrc b/.jshintrc
@@ -26,18 +26,5 @@
   "browser": true,
   "devel": true,
   "nonstandard": true,
-  "worker": true,
-  "predef": [
-    "suite",
-    "test",
-    "setup",
-    "teardown",
-    "suiteSetup",
-    "suiteTeardown",
-    "assert",
-    "require",
-    "define",
-    "requireApp",
-    "sinon"
-  ]
+  "worker": true
 }
diff --git a/.travis.yml b/.travis.yml
@@ -11,21 +11,19 @@ branches:
   - master
 env:
   matrix:
-  - CI_ACTION=gjslint
+  - CI_ACTION=linters
   - CI_ACTION=unit-tests-in-b2g
   - CI_ACTION=unit-tests-in-firefox
   - CI_ACTION=marionette_js
   - CI_ACTION=gaia_ui_tests
   - CI_ACTION=build_tests
-  - CI_ACTION=jshint
   global:
   - ARTIFACTS_S3_BUCKET=mozilla-releng-travis-uploads
   - secure: Gf2qm+xhLRTotuIvDD7HrNRmng3/dctGdsgF5P4aEu3nDujfxtZTp4sGirTqIngbfVzh9WkJ9eoT0yaDXtvWkr40Df6vwAUyI3P1x+NBOa2CkLz7U+f9j1yaJ3pJ121+L3o+7eTzJy/mvXHJYXAfa7rxcmjrCHpH5BSm+9/OZ4s=
   - secure: JgNTK2gfJY1UcpXU/j0MFUvB1P7GaQyOVKBT19uwE2xCsO+T00YWnEhDGFZ9CxYDsATQ5EG5cW/eaoqXOpI9QQsR5kEJp1N1rLvKliqtGcm/ocVjWl/dhsG1i+ksJSCm1oDwduEuLFZ7tChrXihiZ2Sn1RAWns0mjZw9+8+tDhU=
 matrix:
   fast_finish: true
   allow_failures:
-    - env: CI_ACTION=jshint
     - env: CI_ACTION=unit-tests-in-b2g
 notifications:
   email: false

diff --git a/Makefile b/Makefile
@@ -30,9 +30,12 @@
 #                                                                             #
 # Lint your code                                                              #
 #                                                                             #
-# use "make hint" and "make lint" to lint using respectively jshint and       #
+# use "make hint" and "make gjslint" to lint using respectively jshint and    #
 # gjslint.                                                                    #
 #                                                                             #
+# Use "make lint" to lint using gjslint for blacklisted files, and jshint for #
+# other files.                                                                #
+#                                                                             #
 # APP=<app name> will hint/lint only this app.                                #
 # LINTED_FILES=<list of files> will (h/l)int only these space-separated files #
 # JSHINTRC=<path> will use this config file when running jshint               #
@@ -831,32 +834,46 @@ endif
 # Utils                                                                       #
 ###############################################################################
 
-.PHONY: lint hint
+.PHONY: lint gjslint hint
 
 # Lint apps
+## only gjslint files from build/jshint-xfail.list - files not yet safe to jshint
+## "ls" is used to filter the existing files only, in case the xfail.list is not maintained well enough.
 ifndef LINTED_FILES
 ifdef APP
   JSHINTED_PATH = apps/$(APP)
-  GJSLINTED_PATH = -r apps/$(APP)
+  GJSLINTED_PATH = $(shell grep "^apps/$(APP)" build/jshint/xfail.list | ( while read file ; do test -f "$$file" && echo $$file ; done ) )
 else
   JSHINTED_PATH = apps shared
-  GJSLINTED_PATH = -r apps -r shared
+  GJSLINTED_PATH = $(shell ( while read file ; do test -f "$$file" && echo $$file ; done ) < build/jshint/xfail.list )
 endif
 endif
 
-lint: GJSLINT_EXCLUDED_DIRS = $(shell grep '\/\*\*$$' .jshintignore | sed 's/\/\*\*$$//' | paste -s -d, -)
-lint: GJSLINT_EXCLUDED_FILES = $(shell egrep -v '(\/\*\*|^\s*)$$' .jshintignore | paste -s -d, -)
 lint:
-	# --disable 210,217,220,225 replaces --nojsdoc because it's broken in closure-linter 2.3.10
+	NO_XFAIL=1 $(MAKE) -k gjslint hint
+
+gjslint: GJSLINT_EXCLUDED_DIRS = $(shell grep '\/\*\*$$' .jshintignore | sed 's/\/\*\*$$//' | paste -s -d, -)
+gjslint: GJSLINT_EXCLUDED_FILES = $(shell egrep -v '(\/\*\*|^\s*)$$' .jshintignore | paste -s -d, -)
+gjslint:
+	# gjslint --disable 210,217,220,225 replaces --nojsdoc because it's broken in closure-linter 2.3.10
 	# http://code.google.com/p/closure-linter/issues/detail?id=64
-	gjslint --disable 210,217,220,225 --custom_jsdoc_tags="event,example,mixes,mixin,fires,inner,todo,access,namespace,listens,module,memberOf,property" $(GJSLINTED_PATH) -e '$(GJSLINT_EXCLUDED_DIRS)' -x '$(GJSLINT_EXCLUDED_FILES)' $(LINTED_FILES)
+	@echo Running gjslint...
+	@gjslint --disable 210,217,220,225 --custom_jsdoc_tags="event,example,mixes,mixin,fires,inner,todo,access,namespace,listens,module,memberOf,property" -e '$(GJSLINT_EXCLUDED_DIRS)' -x '$(GJSLINT_EXCLUDED_FILES)' $(GJSLINTED_PATH) $(LINTED_FILES)
+	@echo Note: gjslint only checked the files that are xfailed for jshint.
+
+JSHINT_ARGS := --reporter=build/jshint/xfail $(JSHINT_ARGS)
 
 ifdef JSHINTRC
 	JSHINT_ARGS := $(JSHINT_ARGS) --config $(JSHINTRC)
 endif
 
+ifdef VERBOSE
+	JSHINT_ARGS := $(JSHINT_ARGS) --verbose
+endif
+
 hint: node_modules/.bin/jshint
-	./node_modules/.bin/jshint $(JSHINT_ARGS) $(JSHINTED_PATH) $(LINTED_FILES)
+	@echo Running jshint...
+	@./node_modules/.bin/jshint $(JSHINT_ARGS) $(JSHINTED_PATH) $(LINTED_FILES) || (echo Please consult https://github.com/mozilla-b2g/gaia/tree/master/build/jshint/README.md to get some information about how to fix jshint issues. && exit 1)
 
 # Erase all the indexedDB databases on the phone, so apps have to rebuild them.
 delete-databases:

diff --git a/apps/browser/test/marionette/.jshintrc b/apps/browser/test/marionette/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "marionette",
+    "marionetteScriptFinished",
+    "require",
+    "define",
+    "requireApp",
+    "sinon"
+  ]
+}
diff --git a/apps/browser/test/unit/.jshintrc b/apps/browser/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/calendar/test/marionette/.jshintrc b/apps/calendar/test/marionette/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "marionette",
+    "marionetteScriptFinished",
+    "require",
+    "define",
+    "requireApp",
+    "sinon"
+  ]
+}
diff --git a/apps/calendar/test/unit/.jshintrc b/apps/calendar/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/camera/.jshintrc b/apps/camera/.jshintrc
@@ -0,0 +1,7 @@
+{
+  "extends": "../../.jshintrc",
+  "predef": [
+    "define",
+    "require"
+  ]
+}
diff --git a/apps/camera/test/unit/.jshintrc b/apps/camera/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/clock/.jshintrc b/apps/clock/.jshintrc
@@ -0,0 +1,7 @@
+{
+  "extends": "../../.jshintrc",
+  "predef": [
+    "define",
+    "require"
+  ]
+}
diff --git a/apps/clock/test/unit/.jshintrc b/apps/clock/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/communications/contacts/test/marionette/.jshintrc b/apps/communications/contacts/test/marionette/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "marionette",
+    "marionetteScriptFinished",
+    "require",
+    "define",
+    "requireApp",
+    "sinon"
+  ]
+}
diff --git a/apps/communications/contacts/test/unit/.jshintrc b/apps/communications/contacts/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/communications/dialer/test/marionette/.jshintrc b/apps/communications/dialer/test/marionette/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "marionette",
+    "marionetteScriptFinished",
+    "require",
+    "define",
+    "requireApp",
+    "sinon"
+  ]
+}
diff --git a/apps/communications/dialer/test/unit/.jshintrc b/apps/communications/dialer/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/communications/facebook/test/unit/.jshintrc b/apps/communications/facebook/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}
diff --git a/apps/communications/ftu/test/marionette/.jshintrc b/apps/communications/ftu/test/marionette/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "marionette",
+    "marionetteScriptFinished",
+    "require",
+    "define",
+    "requireApp",
+    "sinon"
+  ]
+}
diff --git a/apps/communications/ftu/test/unit/.jshintrc b/apps/communications/ftu/test/unit/.jshintrc
@@ -0,0 +1,17 @@
+{
+  "extends": "../../../../../.jshintrc",
+  "predef": [
+    "assert",
+    "suite",
+    "test",
+    "setup",
+    "teardown",
+    "suiteSetup",
+    "suiteTeardown",
+    "require",
+    "requireApp",
+    "sinon",
+    "mocha",
+    "loadBodyHTML"
+  ]
+}