Commit 57ea41e6 authored by Rene Saarsoo's avatar Rene Saarsoo
Browse files

Improve @chainable detection from function body. 

Ignore statements that don't contain a `return` statement.

This means that now a function that contains just one `return this;` at
the end of itself will be detected as chainable; given there's no other
return statements anywhere in the function body.
parent 6f9ae2c8

lib/jsduck/function_ast.rb

+42 −16
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ module JsDuck 
    end



    def body_returns(body)

      body = skip_non_control_flow_statements(body)

      body = skip_returnless_statements(body)



      return body.length > 0 && return_this?(body[0])

    end
@@ -48,8 +48,8 @@ module JsDuck 
      ast["type"] == "ThisExpression"

    end



    def skip_non_control_flow_statements(statements)

      i = statements.find_index {|s| control_flow?(s) }

    def skip_returnless_statements(statements)

      i = statements.find_index {|s| contains_return?(s) }

      if i

        statements.slice(i, statements.length)

      else
@@ -57,21 +57,47 @@ module JsDuck 
      end

    end



    def contains_return?(ast)

      if return?(ast)

        true

      elsif control_flow?(ast)

        extract_body(ast).any? {|s| contains_return?(s) }

      else

        false

      end

    end



    def control_flow?(ast)

      [

        "IfStatement",

        "SwitchStatement",

        "ForStatement",

        "ForInStatement",

        "WhileStatement",

        "DoWhileStatement",

        "ReturnStatement",

        "TryStatement",

        "WithStatement",

        "LabeledStatement",

        "BlockStatement",

      ].include?(ast["type"])

      CONTROL_FLOW[ast["type"]]

    end



    def extract_body(ast)

      body = []

      CONTROL_FLOW[ast["type"]].each do |name|

        statements = ast[name]

        if statements.is_a?(Hash)

          body << statements

        else

          body += Array(statements)

        end

      end

      body

    end



    CONTROL_FLOW = {

      "IfStatement" => ["consequent", "alternate"],

      "SwitchStatement" => ["cases"],

      "SwitchCase" => ["consequent"],

      "ForStatement" => ["body"],

      "ForInStatement" => ["body"],

      "WhileStatement" => ["body"],

      "DoWhileStatement" => ["body"],

      "TryStatement" => ["block", "handlers", "finalizer"],

      "CatchClause" => ["body"],

      "WithStatement" => ["body"],

      "LabeledStatement" => ["body"],

      "BlockStatement" => ["body"],

    }

  end



end

spec/function_ast_spec.rb

+93 −0
Original line number Diff line number Diff line
@@ -47,4 +47,97 @@ describe "JsDuck::FunctionAst#chainable?" do 
    EOJS

  end



  it "true when RETURN THIS after an IF without RETURNs" do

    chainable?(<<-EOJS).should == true

      /** */

      function foo() {

          if (condition) {

              doSomething();

          } else {

              if (cond2) foo();

          }

          return this;

      }

    EOJS

  end



  it "true when RETURN THIS after SWITCH without returns" do

    chainable?(<<-EOJS).should == true

      /** */

      function foo() {

          switch (x) {

              case 1: break;

              case 2: break;

              default: foo();

          }

          return this;

      }

    EOJS

  end



  it "true when RETURN THIS after loops without returns" do

    chainable?(<<-EOJS).should == true

      /** */

      function foo() {

          for (i=0; i<10; i++) {

              for (j in i) {

                  doBlah();

              }

          }

          while (hoo) {

            do {

              sasa();

            } while(boo);

          }

          return this;

      }

    EOJS

  end



  it "true when RETURN THIS after TRY CATCH without returns" do

    chainable?(<<-EOJS).should == true

      /** */

      function foo() {

          try {

            foo();

          } catch (e) {

            bar();

          } finally {

            baz();

          }

          return this;

      }

    EOJS

  end



  it "true when RETURN THIS after WITH & BLOCK without returns" do

    chainable?(<<-EOJS).should == true

      /** */

      function foo() {

          with (x) {

            foo();

          }

          tada: {

            bar();

          }

          return this;

      }

    EOJS

  end



  it "false when RETURN THIS after statements containing a RETURN" do

    chainable?(<<-EOJS).should == false

      /** */

      function foo() {

          while (x) {

            if (foo) {

            } else if (ooh) {

              return whoKnowsWhat;

            }

          }

          return this;

      }

    EOJS

  end



end