More robust troubleshooting (scottrogowski#34)

* more robust troubleshooting and add js fix

* more js fixes and coverage back to 100%

* slightly better debugging graphviz

Author: scottrogowski

MANIFEST.in

@@ -1 +1,2 @@
 include LICENSE, CHANGELOG.md
+include code2flow/get_ast.js

code2flow/engine.py

@@ -16,7 +16,9 @@
 
 VERSION = '2.2.0'
 
-VALID_EXTENSIONS = {'png', 'svg', 'dot', 'gv', 'json'}
+IMAGE_EXTENSIONS = ('png', 'svg')
+TEXT_EXTENSIONS = ('dot', 'gv', 'json')
+VALID_EXTENSIONS = IMAGE_EXTENSIONS + TEXT_EXTENSIONS
 
 DESCRIPTION = "Generate flow charts from your source code. " \
               "See the README at https://github.com/scottrogowski/code2flow."
@@ -458,8 +460,11 @@ def _generate_graphviz(output_file, extension, final_img_filename):
     logging.info("Running graphviz to make the image...")
     command = ["dot", "-T" + extension, output_file]
     with open(final_img_filename, 'w') as f:
-        subprocess.run(command, stdout=f, check=True)
-    logging.info("Graphviz finished in %.2f seconds." % (time.time() - start_time))
+        try:
+            subprocess.run(command, stdout=f, check=True)
+            logging.info("Graphviz finished in %.2f seconds." % (time.time() - start_time))
+        except subprocess.CalledProcessError:
+            logging.warning("*** Graphviz returned non-zero exit code! Try running %r for more detail ***", ' '.join(command + ['-v', '-O']))
 
 
 def _generate_final_img(output_file, extension, final_img_filename, num_edges):
@@ -512,18 +517,18 @@ def code2flow(raw_source_paths, output_file, language=None, hide_legend=True,
 
     output_ext = None
     if isinstance(output_file, str):
-        assert '.' in output_file, "Output filename must end in one of: %r." % VALID_EXTENSIONS
+        assert '.' in output_file, "Output filename must end in one of: %r." % set(VALID_EXTENSIONS)
         output_ext = output_file.rsplit('.', 1)[1] or ''
-        assert output_ext in VALID_EXTENSIONS, "Output filename must end in one of: %r." % VALID_EXTENSIONS
+        assert output_ext in VALID_EXTENSIONS, "Output filename must end in one of: %r." % set(VALID_EXTENSIONS)
 
     final_img_filename = None
-    if output_ext and output_ext in ('png', 'svg'):
+    if output_ext and output_ext in IMAGE_EXTENSIONS:
         if not is_installed('dot') and not is_installed('dot.exe'):
             raise AssertionError(
                 "Can't generate a flowchart image because neither `dot` nor "
                 "`dot.exe` was found. Either install graphviz (see the README) "
-                "or set your --output argument to a 'dot' filename like out.dot "
-                "or out.gv.")
+                "or, if you just want an intermediate text file, set your --output "
+                "file to use a supported text extension: %r" % set(TEXT_EXTENSIONS))
         final_img_filename = output_file
         output_file, extension = output_file.rsplit('.', 1)
         output_file += '.gv'

code2flow/javascript.py

@@ -66,7 +66,10 @@ def resolve_owner(callee):
         return OWNER_CONST.UNKNOWN_VAR
 
     if callee['object']['type'] == 'NewExpression':
-        return callee['object']['callee']['name']
+        if 'name' in callee['object']['callee']:
+            return callee['object']['callee']['name']
+        return djoin(callee['object']['callee']['object']['name'],
+                     callee['object']['callee']['property']['name'])
 
     return OWNER_CONST.UNKNOWN_VAR
 
@@ -209,7 +212,7 @@ def children(tree):
         if type(v) == dict and v.get('type'):
             ret.append(v)
         if type(v) == list:
-            ret += v
+            ret += filter(None, v)
     return ret
 
 
@@ -231,7 +234,8 @@ def get_acorn_version():
     Get the version of installed acorn
     :rtype: str
     """
-    return subprocess.check_output(['node', '-p', 'require(\'acorn/package.json\').version'])
+    outp = subprocess.check_output(['node', '-p', 'require(\'acorn/package.json\').version'])
+    return outp.decode().strip()
 
 
 class Javascript(BaseLanguage):
@@ -241,11 +245,12 @@ def assert_dependencies():
         assert is_installed('acorn'), "Acorn is required to parse javascript files " \
                                       "but was not found on the path. Install it " \
                                       "from npm and try again."
-
-        if not get_acorn_version().startswith(b'8.'):
+        version = get_acorn_version()
+        if not version.startswith('8.'):
             logging.warning("Acorn is required to parse javascript files. "
                             "Version %r was found but code2flow has only been "
-                            "tested on 8.*", get_acorn_version())
+                            "tested on 8.*", version)
+        logging.info("Using Acorn %s" % version)
 
     @staticmethod
     def get_tree(filename, lang_params):
@@ -266,7 +271,8 @@ def get_tree(filename, lang_params):
                 "Acorn could not parse file %r. You may have a JS syntax error or "
                 "if this is an es6-style source, you may need to run code2flow "
                 "with --source-type=module. "
-                "For more detail, try running the command `acorn %s`. "
+                "For more detail, try running the command "
+                "\n  acorn %s\n"
                 "Warning: Acorn CANNOT parse all javascript files. See their docs. " %
                 (filename, filename)) from None
         tree = json.loads(output)
@@ -331,7 +337,12 @@ def make_nodes(tree, parent):
             full_node_body = tree['value']
 
         subgroup_trees, subnode_trees, this_scope_body = Javascript.separate_namespaces(full_node_body)
-        assert not subgroup_trees
+        if subgroup_trees:
+            # TODO - this is when a class is defined within a function
+            # It's unusual but should probably be handled in the future.
+            # Handling this use case would require some code reorganziation.
+            # Take a look at class_in_function.js to better understand.
+            logging.warning("Skipping class defined within a function!")
 
         line_number = lineno(tree)
         calls = make_calls(this_scope_body)

setup.py

@@ -21,6 +21,7 @@
     download_url=download_url,
     packages=['code2flow'],
     python_requires='>=3.6',
+    include_package_data=True,
     classifiers=[
         'Natural Language :: English',
         "Programming Language :: Python :: 3",

tests/test_code/js/class_in_function/class_in_function.js

@@ -0,0 +1,11 @@
+function rectangleClassFactory() {
+    class Rectangle {
+        constructor(height, width) {
+            this.height = height;
+            this.width = width;
+        }
+    }
+    return Rectangle
+}
+
+rectangleClassFactory()

tests/test_code/js/ternary_new/ternary_new.js

@@ -0,0 +1,29 @@
+class Abra {
+    init() {
+        return this.init()
+    }
+}
+
+class Cadabra {
+    init() {
+        return this.init()
+    }
+}
+
+function abra_fact() {
+    return Abra;
+}
+
+function cadabra_fact() {
+    return Cadabra;
+}
+
+
+class ClassMap {
+    fact(which) {
+        return which == "abra" ? abra_fact : cadabra_fact;
+    }
+}
+
+obj = true ? new ClassMap.fact("abra").init(n) : new ClassMap.fact("cadabra").init(n)
+obj = true ? new ClassMap.fact("abra").fact().init(n) : new ClassMap.fact("cadabra").fact().init(n)

tests/test_interface.py

@@ -9,7 +9,7 @@
 
 sys.path.append(os.getcwd().split('/tests')[0])
 
-from code2flow.engine import code2flow, main
+from code2flow.engine import code2flow, main, _generate_graphviz
 from code2flow import model
 
 IMG_PATH = '/tmp/code2flow/output.png'
@@ -57,6 +57,13 @@ def test_no_files():
                   output_file=IMG_PATH)
 
 
+def test_graphviz_error(caplog):
+    caplog.set_level(logging.DEBUG)
+    _generate_graphviz("/tmp/code2flow/nothing", "/tmp/code2flow/nothing",
+                       "/tmp/code2flow/nothing")
+    assert "non-zero exit" in caplog.text
+
+
 def test_no_files_2():
     if not os.path.exists('/tmp/code2flow/no_source_dir'):
         os.mkdir('/tmp/code2flow/no_source_dir')
@@ -130,7 +137,7 @@ def test_repr():
 
 def test_bad_acorn(mocker, caplog):
     caplog.set_level(logging.DEBUG)
-    mocker.patch('code2flow.javascript.get_acorn_version', return_value=b'7.6.9')
+    mocker.patch('code2flow.javascript.get_acorn_version', return_value='7.6.9')
     code2flow("test_code/js/simple_a_js", "/tmp/code2flow/out.json")
     assert "Acorn" in caplog.text and "8.*" in caplog.text
 

tests/testdata.py

@@ -598,6 +598,27 @@
             "expected_edges": [["file_a_good::(global)", "file_a_good::a"]],
             "expected_nodes": ["file_a_good::(global)", "file_a_good::a"]
         },
+        {
+            "test_name": "class_in_function",
+            "directory": "class_in_function",
+            "comments": "when a function defines a class within it",
+            "expected_edges": [["class_in_function::(global)",
+                                "class_in_function::rectangleClassFactory"]],
+            "expected_nodes": ["class_in_function::rectangleClassFactory",
+                               "class_in_function::(global)"]
+        },
+        {
+            "test_name": "ternary_new",
+            "directory": "ternary_new",
+            "comments": "Ignore the name. This is for complex multi-layered object instantiation.",
+            "expected_edges": [["ternary_new::Cadabra.init", "ternary_new::Cadabra.init"],
+                               ["ternary_new::Abra.init", "ternary_new::Abra.init"],
+                               ["ternary_new::(global)", "ternary_new::ClassMap.fact"]],
+            "expected_nodes": ["ternary_new::Cadabra.init",
+                               "ternary_new::ClassMap.fact",
+                               "ternary_new::(global)",
+                               "ternary_new::Abra.init"]
+        },
         {
             "test_name": "moment.js",
             "directory": "moment",