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

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

Revision: 227
          http://javascriptlint.svn.sourceforge.net/javascriptlint/?rev=227&view=rev
Author:   matthiasmiller
Date:     2008-08-28 00:02:50 +0000 (Thu, 28 Aug 2008)

Log Message:
-----------
improve code flow warnings
-change _get_exit_points to return the actual nodes
-change the no_return_value, anon_no_return_value, and unreachable_code warnings to ignore declarations

Modified Paths:
--------------
    trunk/pyjsl/lint.py
    trunk/pyjsl/warnings.py
    trunk/tests/warnings/anon_no_return_value.js
    trunk/tests/warnings/missing_break.js
    trunk/tests/warnings/no_return_value.js
    trunk/tests/warnings/unreachable_code.js

Modified: trunk/pyjsl/lint.py
===================================================================
--- trunk/pyjsl/lint.py	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/pyjsl/lint.py	2008-08-28 00:02:50 UTC (rev 227)
@@ -194,7 +194,8 @@
 
 def _lint_script(script, script_cache, lint_error, conf, import_callback):
     def parse_error(row, col, msg):
-        if not msg in ('redeclared_var', 'var_hides_arg'):
+        if not msg in ('anon_no_return_value', 'no_return_value',
+                       'redeclared_var', 'var_hides_arg'):
             parse_errors.append((jsparse.NodePos(row, col), msg))
 
     def report(node, errname):

Modified: trunk/pyjsl/warnings.py
===================================================================
--- trunk/pyjsl/warnings.py	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/pyjsl/warnings.py	2008-08-28 00:02:50 UTC (rev 227)
@@ -69,7 +69,9 @@
     'dup_option_explicit': 'duplicate "option explicit" control comment',
     'invalid_fallthru': 'unexpected "fallthru" control comment',
     'invalid_pass': 'unexpected "pass" control comment',
-    'want_assign_or_call': 'expected an assignment or function call'
+    'want_assign_or_call': 'expected an assignment or function call',
+    'no_return_value': 'function {0} does not always return a value',
+    'anon_no_return_value': 'anonymous function does not always return value'
 }
 
 _visitors = []
@@ -94,15 +96,22 @@
         return None
 
 def _get_exit_points(node):
+    """ Returns a set of exit points, which may be:
+        * None, indicating a code path with no specific exit point.
+        * a node of type tok.BREAK, tok.RETURN, tok.THROW.
+    """
     if node.kind == tok.LC:
-        # Only if the last child contains it
         exit_points = set([None])
         for kid in node.kids:
-            # "None" is only a valid exit point for the last statement.
-            if None in exit_points:
-                exit_points.remove(None)
             if kid:
-                exit_points |= _get_exit_points(kid)
+                # Merge in the kid's exit points.
+                kid_exit_points = _get_exit_points(kid)
+                exit_points |= kid_exit_points
+
+                # Stop if the kid always exits.
+                if not None in kid_exit_points:
+                    exit_points.discard(None)
+                    break
     elif node.kind == tok.IF:
         # Only if both branches have an exit point
         cond_, if_, else_ = node.kids
@@ -126,10 +135,9 @@
         # Correct the "None" exit point.
         exit_points.remove(None)
 
-        # Check if the switch contained any break
-        if tok.BREAK in exit_points:
-            exit_points.remove(tok.BREAK)
-            exit_points.add(None)
+        # Convert "break" into None
+        exit_points = set(map(lambda node: node \
+                if node and node.kind != tok.BREAK else None, exit_points))
 
         # Check if the switch had a default case
         if not switch_has_default:
@@ -139,13 +147,13 @@
         if switch_has_final_fallthru:
             exit_points.add(None)
     elif node.kind == tok.BREAK:
-        exit_points = set([tok.BREAK])
+        exit_points = set([node])
     elif node.kind == tok.WITH:
         exit_points = _get_exit_points(node.kids[-1])
     elif node.kind == tok.RETURN:
-        exit_points = set([tok.RETURN])
+        exit_points = set([node])
     elif node.kind == tok.THROW:
-        exit_points = set([tok.THROW])
+        exit_points = set([node])
     elif node.kind == tok.TRY:
         try_, catch_, finally_ = node.kids
 
@@ -369,9 +377,10 @@
 
 @lookfor(tok.BREAK, tok.CONTINUE, tok.RETURN, tok.THROW)
 def unreachable_code(node):
-    if node.parent.kind == tok.LC and \
-        node.node_index != len(node.parent.kids)-1:
-        raise LintWarning, node.parent.kids[node.node_index+1]
+    if node.parent.kind == tok.LC:
+        for sibling in node.parent.kids[node.node_index+1:]:
+            if not sibling.kind in (tok.VAR, tok.FUNCTION):
+                raise LintWarning, sibling
 
 #TODO: @lookfor(tok.IF)
 def meaningless_block(node):
@@ -446,6 +455,36 @@
             return
     raise LintWarning, child
 
+def _check_return_value(node):
+    def is_return_with_val(node):
+        return node and node.kind == tok.RETURN and node.kids[0]
+    def is_return_without_val(node):
+        return node and node.kind == tok.RETURN and not node.kids[0]
+
+    node, = node.kids
+    assert node.kind == tok.LC
+
+    exit_points = _get_exit_points(node)
+    if filter(is_return_with_val, exit_points):
+        # If the function returns a value, find all returns without a value.
+        returns = filter(is_return_without_val, exit_points)
+        returns.sort(key=lambda node: node.start_pos())
+        if returns:
+            raise LintWarning, returns[0]
+        # Warn if the function sometimes exits naturally.
+        if None in exit_points:
+            raise LintWarning, node
+
+@lookfor(tok.FUNCTION)
+def no_return_value(node):
+    if node.fn_name:
+        _check_return_value(node)
+
+@lookfor(tok.FUNCTION)
+def anon_no_return_value(node):
+    if not node.fn_name:
+        _check_return_value(node)
+
 @lookfor()
 def mismatch_ctrl_comments(node):
     pass

Modified: trunk/tests/warnings/anon_no_return_value.js
===================================================================
--- trunk/tests/warnings/anon_no_return_value.js	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/tests/warnings/anon_no_return_value.js	2008-08-28 00:02:50 UTC (rev 227)
@@ -9,10 +9,10 @@
 
     var error2 = function(b) {
         if (b) {
-            return;
+            return; /*warning:anon_no_return_value*/
         }
         else {
-            return ""; /*warning:anon_no_return_value*/
+            return "";
         }
     };
 

Modified: trunk/tests/warnings/missing_break.js
===================================================================
--- trunk/tests/warnings/missing_break.js	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/tests/warnings/missing_break.js	2008-08-28 00:02:50 UTC (rev 227)
@@ -92,7 +92,13 @@
             return i;
         }
 
-      default:
+      case 9:
+        /* test a break inside a loop */
+        for (i = 0; i < 100; i++) {
+            break;
+        }
+
+      default: /*warning:missing_break*/
         break;
     }
 

Modified: trunk/tests/warnings/no_return_value.js
===================================================================
--- trunk/tests/warnings/no_return_value.js	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/tests/warnings/no_return_value.js	2008-08-28 00:02:50 UTC (rev 227)
@@ -9,10 +9,10 @@
     
     function error2(b) {
         if (b) {
-            return;
+            return; /*warning:no_return_value*/
         }
         else {
-            return ""; /*warning:no_return_value*/
+            return "";
         }
     }
     

Modified: trunk/tests/warnings/unreachable_code.js
===================================================================
--- trunk/tests/warnings/unreachable_code.js	2008-08-27 16:11:25 UTC (rev 226)
+++ trunk/tests/warnings/unreachable_code.js	2008-08-28 00:02:50 UTC (rev 227)
@@ -26,4 +26,16 @@
         throw i;
         i -= 1; /*warning:unreachable_code*/
     }
+
+    function var_test() {
+        return undef;
+        var undef;
+    }
+
+    function func_test() {
+        return fortytwo();
+        function fortytwo() {
+            return 42;
+        }
+    }
 }


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