[[FIX]] Do not accept empty values for directives

Previously, when an directive was specified without a value, JSHint would interpret this as the empty string and apply linting rules accordingly. The empty string is never a valid value for any directive, so the generated warnings were not appropriate. If a directive is specified without a value, trigger an explicit error describing the invalid configuration.
jshint · Jun 13, 2015 · a5bfefb · a5bfefb
1 parent 8f82d0d
commit a5bfefb
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 2 deletions.
diff --git a/src/jshint.js b/src/jshint.js
@@ -489,11 +489,20 @@ var JSHINT = (function() {
     var body = nt.body.split(",").map(function(s) { return s.trim(); });
     var predef = {};
     if (nt.type === "globals") {
-      body.forEach(function(g) {
+      body.forEach(function(g, idx) {
         g = g.split(":");
         var key = (g[0] || "").trim();
         var val = (g[1] || "").trim();
 
+        if (key === "-" || !key.length) {
+          // Ignore trailing comma
+          if (idx > 0 && idx === body.length - 1) {
+            return;
+          }
+          error("E002", nt);
+          return;
+        }
+
         if (key.charAt(0) === "-") {
           key = key.slice(1);
           val = false;
@@ -515,7 +524,16 @@ var JSHINT = (function() {
     }
 
     if (nt.type === "exported") {
-      body.forEach(function(e) {
+      body.forEach(function(e, idx) {
+        if (!e.length) {
+          // Ignore trailing comma
+          if (idx > 0 && idx === body.length - 1) {
+            return;
+          }
+          error("E002", nt);
+          return;
+        }
+
         exported[e] = true;
       });
     }

diff --git a/tests/unit/parser.js b/tests/unit/parser.js
@@ -213,6 +213,47 @@ exports.options = function (test) {
   test.done();
 };
 
+exports.emptyDirectives = function (test) {
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* global */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* global : */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* global -: */');
+
+  TestRun(test)
+    .test('/* global foo, bar, baz, */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* globals */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* globals : */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* globals -: */');
+
+  TestRun(test)
+    .test('/* globals foo, bar, baz, */');
+
+  TestRun(test)
+    .addError(1, "Bad option value.")
+    .test('/* exported */');
+
+  TestRun(test)
+    .test('/* exported foo, bar, baz, */');
+
+  test.done();
+};
+
 exports["jshint option comments single line"] = function (test) {
   var src = fs.readFileSync(__dirname + "/fixtures/gh1768-1.js", "utf8");