[javascriptlint-commit] SF.net SVN: javascriptlint:[285] trunk

[<mat...@us...>]

Revision: 285
          http://javascriptlint.svn.sourceforge.net/javascriptlint/?rev=285&view=rev
Author:   matthiasmiller
Date:     2009-10-22 13:30:51 +0000 (Thu, 22 Oct 2009)

Log Message:
-----------
1516656: Add warning about identifiers hiding an identifier in a parent scope.

Modified Paths:
--------------
    trunk/javascriptlint/jsparse.py
    trunk/javascriptlint/lint.py
    trunk/javascriptlint/warnings.py
    trunk/tests/control_comments/declare.js
    trunk/tests/warnings/var_hides_arg.js

Added Paths:
-----------
    trunk/tests/warnings/identifier_hides_another.js

Modified: trunk/javascriptlint/jsparse.py
===================================================================
--- trunk/javascriptlint/jsparse.py	2009-10-22 11:52:52 UTC (rev 284)
+++ trunk/javascriptlint/jsparse.py	2009-10-22 13:30:51 UTC (rev 285)
@@ -174,7 +174,7 @@
         start_pos = node_positions.from_offset(start_offset)
         end_pos = node_positions.from_offset(end_offset)
         kwargs = {
-            'type': 'COMMENT',
+            'kind': 'COMMENT',
             'atom': comment_text,
             'opcode': opcode,
             '_start_line': start_pos.line,

Modified: trunk/javascriptlint/lint.py
===================================================================
--- trunk/javascriptlint/lint.py	2009-10-22 11:52:52 UTC (rev 284)
+++ trunk/javascriptlint/lint.py	2009-10-22 13:30:51 UTC (rev 285)
@@ -117,13 +117,14 @@
         if self._parent:
             return self._parent.resolve_identifier(name)
         return None
-    def get_unreferenced_and_undeclared_identifiers(self):
+    def get_identifier_warnings(self):
         """ Returns a tuple of unreferenced and undeclared, where each is a list
             of (scope, name, node) tuples.
         """
         unreferenced = {}
         undeclared = []
-        self._find_unreferenced_and_undeclared(unreferenced, undeclared, False)
+        obstructive = []
+        self._find_warnings(unreferenced, undeclared, obstructive, False)
 
         # Convert "unreferenced" from a dictionary of:
         #   { (scope, name): node }
@@ -134,15 +135,22 @@
                         in unreferenced.items()]
         unreferenced.sort(key=lambda x: x[2].start_pos())
 
-        return unreferenced, undeclared
-    def _find_unreferenced_and_undeclared(self, unreferenced, undeclared,
-                                          is_in_with_scope):
+        return {
+            'unreferenced': unreferenced,
+            'undeclared': undeclared,
+            'obstructive': obstructive,
+        }
+    def _find_warnings(self, unreferenced, undeclared, obstructive,
+                       is_in_with_scope):
         """ unreferenced is a dictionary, such that:
                 (scope, name): node
             }
             undeclared is a list, such that: [
                 (scope, name, node)
             ]
+            obstructive is a list, such that: [
+                (scope, name, node)
+            ]
         """
         if self._node and self._node.kind == tok.WITH:
             is_in_with_scope = True
@@ -153,6 +161,12 @@
         for name, info in self._identifiers.items():
             unreferenced[(self, name)] = info['node']
 
+        # Check for variables that hide an identifier in a parent scope.
+        if self._parent:
+            for name, info in self._identifiers.items():
+                if self._parent.resolve_identifier(name):
+                    obstructive.append((self, name, info['node']))
+
         # Remove all declared variables from the "unreferenced" set; add all
         # undeclared variables to the "undeclared" list.
         for name, node in self._references:
@@ -170,8 +184,8 @@
                     undeclared.append((self, name, node))
 
         for child in self._kids:
-            child._find_unreferenced_and_undeclared(unreferenced, undeclared,
-                                                    is_in_with_scope)
+            child._find_warnings(unreferenced, undeclared, obstructive,
+                                 is_in_with_scope)
     def find_scope(self, node):
         for kid in self._kids:
             scope = kid.find_scope(node)
@@ -432,10 +446,7 @@
 
     for name, node in declares:
         declare_scope = script_cache.scope.find_scope(node)
-        if declare_scope.get_identifier(name):
-            report(node, 'redeclared_var', name=name)
-        else:
-            declare_scope.add_declaration(name, node, 'var')
+        _warn_or_declare(declare_scope, name, 'var', node, report)
 
 def _lint_script_parts(script_parts, script_cache, lint_error, conf, import_callback):
     def report_lint(node, errname, pos=None, **errargs):
@@ -466,15 +477,15 @@
                           report_native, report_lint, import_callback)
 
     scope = script_cache.scope
-    unreferenced, undeclared = scope.get_unreferenced_and_undeclared_identifiers()
-    for decl_scope, name, node in undeclared:
+    identifier_warnings = scope.get_identifier_warnings()
+    for decl_scope, name, node in identifier_warnings['undeclared']:
         if name in conf['declarations']:
             continue
         if name in _globals:
             continue
         if not script_cache.hasglobal(name):
             report_lint(node, 'undeclared_identifier', name=name)
-    for ref_scope, name, node in unreferenced:
+    for ref_scope, name, node in identifier_warnings['unreferenced']:
         # Ignore the outer scope.
         if ref_scope != scope:
             type_ = ref_scope.get_identifier_type(name)
@@ -486,6 +497,8 @@
                 report_lint(node, 'unreferenced_variable', name=name)
             else:
                 assert False, 'Unrecognized identifier type: %s' % type_
+    for ref_scope, name, node in identifier_warnings['obstructive']:
+        report_lint(node, 'identifier_hides_another', name=name)
 
 def _getreporter(visitor, report):
     def onpush(node):
@@ -504,12 +517,14 @@
 
 def _warn_or_declare(scope, name, type_, node, report):
     parent_scope, other = scope.resolve_identifier(name) or (None, None)
-    if other and other.kind == tok.FUNCTION and name in other.fn_args:
-        report(node, 'var_hides_arg', name=name)
-    elif other and parent_scope == scope:
-        report(node, 'redeclared_var', name=name)
+    if other and parent_scope == scope:
+        # Only warn about duplications in this scope.
+        # Other scopes will be checked later.
+        if other.kind == tok.FUNCTION and name in other.fn_args:
+            report(node, 'var_hides_arg', name=name)
+        else:
+            report(node, 'redeclared_var', name=name)
     else:
-        # TODO: Warn when hiding a variable in a parent scope.
         scope.add_declaration(name, node, type_)
 
 def _get_scope_checks(scope, report):

Modified: trunk/javascriptlint/warnings.py
===================================================================
--- trunk/javascriptlint/warnings.py	2009-10-22 11:52:52 UTC (rev 284)
+++ trunk/javascriptlint/warnings.py	2009-10-22 13:30:51 UTC (rev 285)
@@ -82,6 +82,7 @@
     'nested_comment': 'nested comment',
     'legacy_cc_not_understood': 'couldn\'t understand control comment using /*@keyword@*/ syntax',
     'var_hides_arg': 'variable {name} hides argument',
+    'identifier_hides_another': 'identifer {name} hides an identifier in a parent scope',
     'duplicate_formal': 'duplicate formal argument {name}',
     'missing_semicolon': 'missing semicolon',
     'missing_semicolon_for_lambda': 'missing semicolon for lambda assignment',

Modified: trunk/tests/control_comments/declare.js
===================================================================
--- trunk/tests/control_comments/declare.js	2009-10-22 11:52:52 UTC (rev 284)
+++ trunk/tests/control_comments/declare.js	2009-10-22 13:30:51 UTC (rev 285)
@@ -1,4 +1,5 @@
 /*jsl:option explicit*/
+/*conf:-identifier_hides_another*/
 function declare() {
     window.alert('http://www.javascriptlint.com/');
     /*jsl:declare window*/ /*warning:redeclared_var*/

Added: trunk/tests/warnings/identifier_hides_another.js
===================================================================
--- trunk/tests/warnings/identifier_hides_another.js	                        (rev 0)
+++ trunk/tests/warnings/identifier_hides_another.js	2009-10-22 13:30:51 UTC (rev 285)
@@ -0,0 +1,43 @@
+// Test each combination of arg, var, function, and jsl:declare.
+function identifier_hides_another(arg_hides_arg,
+                                  function_hides_arg,
+                                  var_hides_arg,
+                                  declare_hides_arg) {
+
+    function arg_hides_function() { return true; }
+    function function_hides_function() { return true; }
+    function var_hides_function() { return true; }
+    function declare_hides_function() { return true; }
+
+    var arg_hides_var;
+    var function_hides_var;
+    var var_hides_var;
+    var declare_hides_var;
+
+    /*jsl:declare arg_hides_declare*/
+    /*jsl:declare function_hides_declare*/
+    /*jsl:declare var_hides_declare*/
+    /*jsl:declare declare_hides_declare*/
+
+    function inner(arg_hides_arg,
+                   arg_hides_function,
+                   arg_hides_var,
+                   arg_hides_declare) { /*warning:identifier_hides_another*//*warning:identifier_hides_another*//*warning:identifier_hides_another*//*warning:identifier_hides_another*/
+
+        function function_hides_arg() { return true; } /*warning:identifier_hides_another*/
+        function function_hides_function() { return true; } /*warning:identifier_hides_another*/
+        function function_hides_var() { return true; } /*warning:identifier_hides_another*/
+        function function_hides_declare() { return true; } /*warning:identifier_hides_another*/
+
+        var var_hides_arg; /*warning:identifier_hides_another*/
+        var var_hides_function; /*warning:identifier_hides_another*/
+        var var_hides_var; /*warning:identifier_hides_another*/
+        var var_hides_declare; /*warning:identifier_hides_another*/
+
+        /*jsl:declare declare_hides_arg*/ /*warning:identifier_hides_another*/
+        /*jsl:declare declare_hides_function*/ /*warning:identifier_hides_another*/
+        /*jsl:declare declare_hides_var*/ /*warning:identifier_hides_another*/
+        /*jsl:declare declare_hides_declare*/ /*warning:identifier_hides_another*/
+    }
+}
+


Property changes on: trunk/tests/warnings/identifier_hides_another.js
___________________________________________________________________
Added: svn:mergeinfo
   + 

Modified: trunk/tests/warnings/var_hides_arg.js
===================================================================
--- trunk/tests/warnings/var_hides_arg.js	2009-10-22 11:52:52 UTC (rev 284)
+++ trunk/tests/warnings/var_hides_arg.js	2009-10-22 13:30:51 UTC (rev 285)
@@ -2,6 +2,6 @@
 function var_hides_arg(duplicate1, duplicate2) {
     var duplicate1; /*warning:var_hides_arg*/
     function inner() {
-        var duplicate2; /*warning:var_hides_arg*/
+        var duplicate2; /*warning:identifier_hides_another*/
     }
 }


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.