Commit aca60237 authored by Rene Saarsoo's avatar Rene Saarsoo
Browse files

Replace FunctionAst#chainable? with #return_types.

As a first step in expanding FunctionAst to do full detection of
various return types, not just whether we're returning this or
something else.
parent 50039622
Loading
Loading
Loading
Loading
+5 −1
Original line number Diff line number Diff line
@@ -407,7 +407,11 @@ module JsDuck
    end

    def chainable?(ast)
      FunctionAst.chainable?(ast)
      if function?(ast) && !empty_fn?(ast)
        FunctionAst.return_types(ast) == [:this]
      else
        false
      end
    end

    def make_property(name=nil, ast=nil, tagname=:property)
+10 −24
Original line number Diff line number Diff line
@@ -8,27 +8,7 @@ module JsDuck
  class FunctionAst
    include Util::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 chainable?(ast)
      if ast && function?(ast)
        rvalues = return_values(ast["body"]["body"])
        rvalues.keys == [:this]
      else
        false
      end
    end

    private

    def function?(ast)
      ast["type"] == "FunctionDeclaration" || ast["type"] == "FunctionExpression"
    end

    # Given an array of statements determines the possible return values.
    # Returns a hash with the return values.
    # Detects possible return types of the given function.
    #
    # For now there are three possible detected return values:
    #
@@ -36,7 +16,13 @@ module JsDuck
    # * :this - the code contins 'return this;'
    # * :other - some other value is returned.
    #
    def return_values(body)
    def return_types(ast)
      return_types_hash(ast["body"]["body"]).keys
    end

    private

    def return_types_hash(body)
      rvalues = {}
      body.each do |ast|
        if return_this?(ast)
@@ -47,7 +33,7 @@ module JsDuck
          return rvalues
        elsif possibly_blocking?(ast)
          extract_bodies(ast).each do |b|
            rvalues.merge!(return_values(b))
            rvalues.merge!(return_types_hash(b))
          end
          if !rvalues[:void]
            return rvalues
@@ -56,7 +42,7 @@ module JsDuck
          end
        elsif control_flow?(ast)
          extract_bodies(ast).each do |b|
            rvalues.merge!(return_values(b))
            rvalues.merge!(return_types_hash(b))
          end
          rvalues.delete(:void)
        end
+31 −0
Original line number Diff line number Diff line
@@ -159,6 +159,37 @@ describe JsDuck::Aggregator do
    end
  end

  describe "method without any code" do
    let(:cls) do
      parse(<<-EOS)["MyClass"]
        /** */
        Ext.define("MyClass", {
            /** @method bar */
        });
      EOS
    end

    it "doesn't add @chainable tag" do
      cls[:members][0][:meta][:chainable].should_not == true
    end
  end

  describe "method consisting of Ext.emptyFn in code" do
    let(:cls) do
      parse(<<-EOS)["MyClass"]
        /** */
        Ext.define("MyClass", {
            /** */
            bar: Ext.emptyFn
        });
      EOS
    end

    it "doesn't add @chainable tag" do
      cls[:members][0][:meta][:chainable].should_not == true
    end
  end

  describe "function with 'return this;' in code" do
    let(:cls) do
      parse(<<-EOS)["MyClass"]
+89 −92
Original line number Diff line number Diff line
require "jsduck/js_parser"
require "jsduck/function_ast"

describe "JsDuck::FunctionAst#chainable?" do
  def chainable?(string)
describe "JsDuck::FunctionAst#return_types" do
  def returns(string)
    node = JsDuck::JsParser.new(string).parse[0]
    return JsDuck::FunctionAst.chainable?(node[:code])
    return JsDuck::FunctionAst.return_types(node[:code])
  end

  it "false when no AST given at all" do
    chainable?("/** */").should == false
  describe "returns [:this] when function body" do
    it "has single RETURN THIS statement in body" do
      returns("/** */ function foo() {return this;}").should == [:this]
    end

  it "false when no function AST given" do
    chainable?("/** */ Ext.emptyFn;").should == false
  end

  it "false when body has no return statement." do
    chainable?("/** */ function foo() {}").should == false
  end

  it "false when body has empty return statement" do
    chainable?("/** */ function foo() { return; }").should == false
  end

  it "true when single RETURN THIS statement in body" do
    chainable?("/** */ function foo() {return this;}").should == true
  end

  it "true when RETURN THIS after a few expression statements" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after a few expression statements" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          doSomething();
@@ -39,8 +24,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after a few declarations" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after a few declarations" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          var x = 10;
@@ -51,8 +36,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after an IF without RETURNs" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after an IF without RETURNs" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          if (condition) {
@@ -65,8 +50,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after SWITCH without returns" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after SWITCH without returns" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          switch (x) {
@@ -79,8 +64,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after loops without returns" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after loops without returns" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          for (i=0; i<10; i++) {
@@ -98,8 +83,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after TRY CATCH without returns" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after TRY CATCH without returns" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          try {
@@ -114,8 +99,8 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after WITH & BLOCK without returns" do
    chainable?(<<-EOJS).should == true
    it "has RETURN THIS after WITH & BLOCK without returns" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          with (x) {
@@ -129,14 +114,14 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "false when RETURN THIS after statements containing a RETURN" do
    chainable?(<<-EOJS).should == false
    it "has RETURN THIS after statements also containing a RETURN THIS" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          while (x) {
            if (foo) {
            } else if (ooh) {
              return whoKnowsWhat;
              return this;
            }
          }
          return this;
@@ -144,72 +129,84 @@ describe "JsDuck::FunctionAst#chainable?" do
      EOJS
    end

  it "true when RETURN THIS after statements also containing a RETURN THIS" do
    chainable?(<<-EOJS).should == true
    it "has both branches of IF finishing with RETURN THIS" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          while (x) {
          if (foo) {
            } else if (ooh) {
              blah();
              if (true) {
                  return this;
              } else {
                  chah();
                  return this;
              }
          }
          } else {
              return this;
          }
      }
      EOJS
    end

  it "false when only one branch finishes with RETURN THIS" do
    chainable?(<<-EOJS).should == false
    it "has DO WHILE containing RETURN THIS" do
      returns(<<-EOJS).should == [:this]
      /** */
      function foo() {
          if (foo) {
              doSomething();
          } else {
          do {
              return this;
          }
          } while(true);
      }
      EOJS
    end
  end

  it "true when both branches of IF finish with RETURN THIS" do
    chainable?(<<-EOJS).should == true
  describe "doesn't return [:this] when function body" do
    it "has no return statement." do
      returns("/** */ function foo() {}").should_not == [:this]
    end

    it "has empty return statement" do
      returns("/** */ function foo() { return; }").should_not == [:this]
    end

    it "has RETURN THIS after statements containing a RETURN" do
      returns(<<-EOJS).should_not == [:this]
      /** */
      function foo() {
          while (x) {
            if (foo) {
              blah();
              if (true) {
                  return this;
              } else {
                  chah();
                  return this;
            } else if (ooh) {
              return whoKnowsWhat;
            }
          } else {
              return this;
          }
          return this;
      }
      EOJS
    end

  it "true when DO WHILE contains RETURN THIS" do
    chainable?(<<-EOJS).should == true
    it "has WHILE containing RETURN THIS" do
      returns(<<-EOJS).should_not == [:this]
      /** */
      function foo() {
          do {
          while (condition) {
              return this;
          } while(true);
          };
      }
      EOJS
    end

  it "false when WHILE contains RETURN THIS" do
    chainable?(<<-EOJS).should == false
    it "has only one branch finishing with RETURN THIS" do
      returns(<<-EOJS).should_not == [:this]
      /** */
      function foo() {
          while (condition) {
          if (foo) {
              doSomething();
          } else {
              return this;
          };
          }
      }
      EOJS
    end
  end

end