[[CHORE]] Refactor internals and limit warnings

Defer parameter validation until after functions are parsed so that the process can be applied according to the strictness of the function body. Define this via a `validateParams` method, and do not trigger warnings related to variable shadowing in cases where the same code triggers an error.
jshint · Jul 19, 2015 · 33749f6 · 33749f6
1 parent 7d25451
commit 33749f6
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 41 deletions.
diff --git a/src/jshint.js b/src/jshint.js
@@ -1642,13 +1642,13 @@ var JSHINT = (function() {
           p = pn;
         } else if (pn.value === "[" || pn.value === ".") {
           // string -> [ | . is a valid production
-          return;
+          break;
         } else if (!state.option.asi || pn.value === "(") {
           // string -> ( is not a valid production
           warning("W033", state.tokens.next);
         }
       } else if (p.id === "." || p.id === "[") {
-        return;
+        break;
       } else if (p.id !== ";") {
         warning("W033", p);
       }
@@ -1669,8 +1669,6 @@ var JSHINT = (function() {
     }
 
     if (state.isStrict()) {
-      state.funct["(scope)"].inStrictMode();
-
       if (!state.option["(explicitNewcap)"]) {
         state.option.newcap = true;
       }
@@ -1739,15 +1737,18 @@ var JSHINT = (function() {
 
         metrics.statementCount += a.length;
 
-        if (isfunc) {
-          state.directive = m;
-        }
-
         indent -= state.option.indent;
       }
 
       advance("}", t);
 
+      if (isfunc) {
+        state.funct["(scope)"].validateParams();
+        if (m) {
+          state.directive = m;
+        }
+      }
+
       state.funct["(scope)"].unstack();
 
       indent = old_indent;

diff --git a/src/scope-manager.js b/src/scope-manager.js
@@ -433,24 +433,20 @@ var scopeManager = function(state, predefined, exported, declared) {
         }
       }
 
-      var declaredInCurrentScope = _.has(_current["(labels)"], labelName);
+      // The variable was declared in the current scope
+      if (_.has(_current["(labels)"], labelName)) {
+        _current["(labels)"][labelName].duplicated = true;
 
-      // if this scope has the variable defined, its a re-definition error
-      var isStrict = state.isStrict();
-      if (declaredInCurrentScope) {
-        if (isStrict) {
-          warning("E011", token, labelName);
-        } else {
-          if (state.option.shadow !== true) {
-            warning("W004", token, labelName);
-          }
-          if (!_current["(unraisedStrictWarnings)"]) {
-            _current["(unraisedStrictWarnings)"] = [];
-          }
-          _current["(unraisedStrictWarnings)"].push(["E011", token, labelName]);
-        }
+      // The variable was declared in an outer scope
       } else {
+        // if this scope has the variable defined, it's a re-definition error
         _checkOuterShadow(labelName, token, type);
+
+        _current["(labels)"][labelName] = {
+          "(type)" : type,
+          "(token)": token,
+          "(unused)": true };
+        _current["(params)"].push(labelName);
       }
 
       if (_.has(_current["(usages)"], labelName)) {
@@ -463,28 +459,28 @@ var scopeManager = function(state, predefined, exported, declared) {
           warning("E056", token, labelName, type);
         }
       }
-
-      _current["(labels)"][labelName] = {
-        "(type)" : type,
-        "(token)": token,
-        "(unused)": true };
-      _current["(params)"].push(labelName);
     },
 
-    inStrictMode: function() {
-      // we only care about duplicate params, so return if we are not in a function
+    validateParams: function() {
+      // This method only concerns errors for function parameters
       if (_currentFunct["(global)"]) {
         return;
       }
 
+      var isStrict = state.isStrict();
       var currentFunctParamScope = _currentFunct["(parent)"];
-      var warnings = currentFunctParamScope["(unraisedStrictWarnings)"];
-      if (!warnings) {
-        return;
-      }
-      for (var i = 0; i < warnings.length; i++) {
-        warning.apply(null, warnings[i]);
-      }
+
+      currentFunctParamScope["(params)"].forEach(function(labelName) {
+        var label = currentFunctParamScope["(labels)"][labelName];
+
+        if (label && label.duplicated) {
+          if (isStrict) {
+            warning("E011", label["(token)"], labelName);
+          } else if (state.option.shadow !== true) {
+            warning("W004", label["(token)"], labelName);
+          }
+        }
+      });
     },
 
     /**

diff --git a/tests/unit/core.js b/tests/unit/core.js
@@ -1507,7 +1507,6 @@ exports.testDuplicateParamNames = function (test) {
   TestRun(test)
     .addError(2, "'a' is already defined.")
     .addError(7, "'a' has already been declared.")
-    .addError(11, "'a' is already defined.")
     .addError(11, "'a' has already been declared.")
     .addError(17, "'a' has already been declared.")
     .addError(18, "Unnecessary directive \"use strict\".")
@@ -1516,7 +1515,6 @@ exports.testDuplicateParamNames = function (test) {
   TestRun(test)
     .addError(2, "'a' is already defined.")
     .addError(7, "'a' has already been declared.")
-    .addError(11, "'a' is already defined.")
     .addError(11, "'a' has already been declared.")
     .addError(17, "'a' has already been declared.")
     .addError(18, "Unnecessary directive \"use strict\".")
@@ -1525,7 +1523,6 @@ exports.testDuplicateParamNames = function (test) {
   TestRun(test)
     .addError(2, "'a' is already defined.")
     .addError(7, "'a' has already been declared.")
-    .addError(11, "'a' is already defined.")
     .addError(11, "'a' has already been declared.")
     .addError(17, "'a' has already been declared.")
     .addError(18, "Unnecessary directive \"use strict\".")