Factor in code review comments and refactor functions to be more naturally split.

flask/cli.py

@@ -82,42 +82,67 @@ def find_best_app(script_info, module):
     )
 
 
-def call_factory(func, script_info):
+def call_factory(app_factory, script_info, arguments=()):
-    """Checks if the given app factory function has an argument named 
+    """Takes an app factory, a ``script_info` object and  optionally a tuple
-    ``script_info`` or just a single argument and calls the function passing 
+    of arguments. Checks for the existence of a script_info argument and calls
-    ``script_info`` if so. Otherwise, calls the function without any arguments
+    the app_factory depending on that and the arguments provided.
-    and returns the result.
     """
-    arguments = getargspec(func).args
+    arg_names = getargspec(app_factory).args
-    if 'script_info' in arguments:
+    if 'script_info' in arg_names:
-        return func(script_info=script_info)
+        return app_factory(*arguments, script_info=script_info)
-    elif len(arguments) == 1:
-        return func(script_info)
-    return func()
-
-
-def call_factory_from_regex(match, mod, script_info):
-    """Checks if the given regex match has specified arguments and if the 
-    function takes a ``script_info`` argument and calls the function with
-    the appropriate arguments."""
-    function_name = match.group(1)
-    arguments = match.group(2)
-    if arguments:
-        arguments = ast.literal_eval(
-            "({arguments}, )".format(arguments=arguments))
-    else:
-        arguments = ()
-    app_factory = getattr(mod, function_name, None)
-    app_factory_arg_names = getargspec(app_factory).args
-    if 'script_info' in app_factory_arg_names:
-        app = app_factory(*arguments, script_info=script_info)
     elif arguments:
-        app = app_factory(*arguments)
+        return app_factory(*arguments)
-    elif not arguments and len(app_factory_arg_names) == 1:
+    elif not arguments and len(arg_names) == 1:
-        app = app_factory(script_info)
+        return app_factory(script_info)
+    return app_factory()
+
+
+def find_app_by_string(string, script_info, module):
+    """Checks if the given string is a variable name or a function. If it is 
+    a function, it checks for specified arguments and whether it takes 
+    a ``script_info`` argument and calls the function with the appropriate 
+    arguments. If it is a """
+    from . import Flask
+    function_regex = r'^(?P<name>\w+)(?:\((?P<args>.*)\))?$'
+    match = re.match(function_regex, string)
+    if match:
+        name, args = match.groups()
+        try:
+            if args is not None:
+                args = args.rstrip(' ,')
+                if args:
+                    args = ast.literal_eval(
+                        "({args}, )".format(args=args))
                 else:
-        app = app_factory()
+                    args = ()
+                app_factory = getattr(module, name, None)
+                app = call_factory(app_factory, script_info, args)
+            else:
+                attr = getattr(module, name, None)
+                if inspect.isfunction(attr):
+                    app = call_factory(attr, script_info)
+                else:
+                    app = attr
+
+            if isinstance(app, Flask):
                 return app
+            else:
+                raise RuntimeError('Failed to find application in module '
+                                   '"{name}"'.format(name=module))
+        except TypeError as e:
+            new_error = NoAppException(
+                '{e}\nThe app factory "{factory}" in module "{module}" could'
+                ' not be called with the specified arguments (and a'
+                ' script_info argument automatically added if applicable).'
+                ' Did you make sure to use the right number of arguments as'
+                ' well as not using keyword arguments or'
+                ' non-literals?'.format(e=e, factory=string, module=module))
+            reraise(NoAppException, new_error, sys.exc_info()[2])
+    else:
+        raise NoAppException(
+            'The provided string "{string}" is not a valid variable name'
+            'or function expression.'.format(string=string))
+
 
 def prepare_exec_for_file(filename):
     """Given a filename this will try to calculate the python path, add it
@@ -174,33 +199,9 @@ def locate_app(script_info, app_id):
 
     mod = sys.modules[module]
     if app_obj is None:
-        app = find_best_app(script_info, mod)
+        return find_best_app(script_info, mod)
-    else:
-        function_regex = r'^([\w_][\w_\d]*)\((.*)\)$'
-        match = re.match(function_regex, app_obj)
-        try:
-            if match:
-                app = call_factory_from_regex(match, mod, script_info)
     else:
-                attr = getattr(mod, app_obj, None)
+        return find_app_by_string(app_obj, script_info, mod)
-                if inspect.isfunction(attr):
-                    app = call_factory(attr, script_info)
-                else:
-                    app = attr
-        except TypeError as e:
-            new_error = NoAppException(
-                '{e}\nThe app factory "{factory}" in module "{module}" could'
-                ' not be called with the specified arguments (and a'
-                ' script_info argument automatically added if applicable).'
-                ' Did you make sure to use the right number of arguments as'
-                ' well as not using keyword arguments or'
-                ' non-literals?'.format(e=e, factory=app_obj, module=module))
-            reraise(NoAppException, new_error, sys.exc_info()[2])
-        if app is None:
-            raise RuntimeError('Failed to find application in module '
-                               '"{name}"'.format(name=module))
-
-    return app
 
 
 def find_default_import_path():

tests/test_cli.py

@@ -160,6 +160,9 @@ def test_locate_app(test_apps):
     assert locate_app(
         script_info, "cliapp.factory:create_app2('foo', 'bar')"
     ).name == "create_app2_foo_bar"
+    assert locate_app(
+        script_info, "cliapp.factory:create_app2('foo', 'bar', )"
+    ).name == "create_app2_foo_bar"
     assert locate_app(
         script_info, "cliapp.factory:create_app3('baz', 'qux')"
     ).name == "create_app3_baz_qux"
@@ -173,6 +176,9 @@ def test_locate_app(test_apps):
     pytest.raises(
         NoAppException, locate_app,
         script_info, "cliapp.factory:create_app2('foo')")
+    pytest.raises(
+        NoAppException, locate_app,
+        script_info, "cliapp.factory:create_app ()")
     pytest.raises(
         NoAppException, locate_app, script_info, "cliapp.importerrorapp")