From cab57d1fdfed8581618b846bf3c04c3d29a0b2b7 Mon Sep 17 00:00:00 2001
From: Rene Saarsoo <nene@triin.net>
Date: Wed, 22 Aug 2012 12:48:10 +0300
Subject: [PATCH] Replace FunctionAst#returns with #chainable?

As we're only detecting 'return this;' using a predicate is better
than a function returning 'this' or nil.

FunctionAst is now also a singleton, but that fact is hidden away behind
static #chainable? method.
---
 lib/jsduck/ast.rb          |  2 +-
 lib/jsduck/function_ast.rb | 22 ++++++++++++++--------
 spec/function_ast_spec.rb  | 16 ++++++++--------
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/lib/jsduck/ast.rb b/lib/jsduck/ast.rb
index 7bd4a5e8..1b1dc1ed 100644
--- a/lib/jsduck/ast.rb
+++ b/lib/jsduck/ast.rb
@@ -407,7 +407,7 @@ module JsDuck
     end
 
     def chainable?(ast)
-      FunctionAst.new.returns(ast) == "this"
+      FunctionAst.chainable?(ast)
     end
 
     def make_property(name=nil, ast=nil, tagname=:property)
diff --git a/lib/jsduck/function_ast.rb b/lib/jsduck/function_ast.rb
index 58121a3a..fc348788 100644
--- a/lib/jsduck/function_ast.rb
+++ b/lib/jsduck/function_ast.rb
@@ -1,3 +1,4 @@
+require "singleton"
 require "jsduck/serializer"
 require "jsduck/evaluator"
 
@@ -5,12 +6,21 @@ module JsDuck
 
   # Analyzes the AST of a FunctionDeclaration or FunctionExpression.
   class FunctionAst
-    # Detects from function body what the function returns.
-    def returns(ast)
+    include Singleton
+
+    # True when function always finishes by returning this.  False
+    # doesn't neccessarily mean that the function doesn't return this
+    # - rather it means our static analyzes wasn't able to determine
+    # what the function returns.
+    def self.chainable?(ast)
+      FunctionAst.instance.chainable?(ast)
+    end
+
+    def chainable?(ast)
       if ast && function?(ast)
         body_returns(ast["body"]["body"])
       else
-        nil
+        false
       end
     end
 
@@ -23,11 +33,7 @@ module JsDuck
     def body_returns(body)
       body = skip_non_control_flow_statements(body)
 
-      if body.length > 0 && return_this?(body[0])
-        "this"
-      else
-        nil
-      end
+      return body.length > 0 && return_this?(body[0])
     end
 
     def return_this?(ast)
diff --git a/spec/function_ast_spec.rb b/spec/function_ast_spec.rb
index 1c27968a..bc587c97 100644
--- a/spec/function_ast_spec.rb
+++ b/spec/function_ast_spec.rb
@@ -2,29 +2,29 @@ require "jsduck/js_parser"
 require "jsduck/function_ast"
 
 describe "JsDuck::FunctionAst#returns" do
-  def returns(string)
+  def chainable?(string)
     node = JsDuck::JsParser.new(string).parse[0]
-    return JsDuck::FunctionAst.new.returns(node[:code])
+    return JsDuck::FunctionAst.chainable?(node[:code])
   end
 
   it "fails when no AST given at all" do
-    returns("/** */").should == nil
+    chainable?("/** */").should == false
   end
 
   it "fails when no function AST given" do
-    returns("/** */ Ext.emptyFn;").should == nil
+    chainable?("/** */ Ext.emptyFn;").should == false
   end
 
   it "fails when body has no return statement." do
-    returns("/** */ function foo() {}").should == nil
+    chainable?("/** */ function foo() {}").should == false
   end
 
   it "returns this when single return this statement in body" do
-    returns("/** */ function foo() {return this;}").should == "this"
+    chainable?("/** */ function foo() {return this;}").should == true
   end
 
   it "returns this when return this after a few expression statements" do
-    returns(<<-EOJS).should == "this"
+    chainable?(<<-EOJS).should == true
       /** */
       function foo() {
           doSomething();
@@ -36,7 +36,7 @@ describe "JsDuck::FunctionAst#returns" do
   end
 
   it "returns this when return this after a few declarations" do
-    returns(<<-EOJS).should == "this"
+    chainable?(<<-EOJS).should == true
       /** */
       function foo() {
           var x = 10;
-- 
GitLab