ksx: Partially implement recursive descent compilation

This commit implements the relatively easier part of the recursive descent
compiler which is to just continually compile the given lines until there are no
more ksx statements.

This does choke however when two files create a circular reference. I have
added a recursion limit of 6 (which may actually be too low) to catch this
possible condition. Python actually chokes at some point too, but it takes a
while and I'd rather catch it sooner (also prefer giving a more friendly error
message).

I attempted to use a hash of the file contents to track whether we have already
compiled that file, but that is not working because the thing being hashed is
accumulating each compilation step, so it changes.

Anyways, recursion is hard, and this still needs some work, but it's usable
enough as long as you don't have any circular references.

Related to issue #7

ksx.py

@@ -15,6 +15,11 @@ class ImportNotFoundError(ImportError):
     pass
 
 
+class CircularImportError(ImportError):
+    """Raised when the compiler detects that it is in a circular reference import loop"""
+    pass
+
+
 def min_strip_comments(file_lines, *args, **kwargs):
     """Comments are only needed for weak Kerbals, remove them"""
     def comment_filter(line):
@@ -244,8 +249,13 @@ def function_from_files(include_files, function_name):
 
 
 def ksx_remove_lines(file_lines, *args, **kwargs):
-    """Remove any remaining @ksx lines, this is the last ksx rule executed"""
-    return (l for l in file_lines if not l.strip().startswith("@ksx"))
+    """Remove any no-effect @ksx directives"""
+    to_remove = ['ensure', 'executed']
+    def line_filter(line):
+        l = line.strip()
+        return l.startswith("@ksx") and l.split(' ')[1] in to_remove
+
+    return (f'{l.rstrip()}\n' for l in file_lines if not line_filter(l))
 
 
 def walkpath_with_action(path, action):
@@ -315,6 +325,62 @@ def file_has_ksx_directive(file_lines, specifically=None):
     return any(line_has_ksx_directive(l, specifically) for l in file_lines)
 
 
+def hash_file_contents(file_lines):
+    import hashlib
+    m = hashlib.sha256()
+
+    for line in file_lines:
+        m.update(line.encode('utf-8'))
+
+    return m.hexdigest()
+
+
+RECURSION_DESCENT_LIMIT = 6
+
+
+def compile_recursive_descent(file_lines, *args, **kwargs):
+    """Given a file and its lines, recursively compile until no ksx statements remain"""
+    visited_files = kwargs.get('visited_files', set())
+
+    # calculate a hash of the file_lines and check if we have already compiled
+    # this one
+    file_hash = hash_file_contents(file_lines)
+
+    if len(visited_files) > RECURSION_DESCENT_LIMIT:
+        msg = (
+            "Compiler appears to be in a circular reference loop, "
+            "this is currently non-recoverable and is a known issue.\n\n"
+            "See: https://github.com/LeonardMH/kos-scripts/issues/7 \n\n"
+            "In the meantime check your library for files which import a "
+            "file, where that file imports the original (A->B->A).\n\n"
+            "You might also attempt using the 'from x import y' syntax which "
+            "has slightly narrower scope."
+        )
+
+        raise CircularImportError(msg)
+
+    if file_hash in visited_files:
+        # we have already compiled this file, no need to do so again
+        return ""
+    else:
+        # we will now compile the file, mark that it has been visited
+        visited_files.add(file_hash)
+
+    # compile and split back out to individual lines
+    file_oneline = compile_single_file_lines(file_lines, *args, **kwargs)
+    file_lines = file_oneline.split('\n')
+
+    # if there are no more ksx directives in the lines compiled we are done,
+    # return the stringified compile result
+    if not file_has_ksx_directive(file_lines):
+        return file_oneline
+
+    # if there are still more ksx directives in the lines compiled so far, run
+    # again
+    kwargs['visited_files'] = visited_files
+    return compile_recursive_descent(file_lines, *args, **kwargs).rstrip() + '\n'
+
+
 def compile_single_file_lines(file_lines, minifier_actions,
                               transpile_only=False,
                               safe_only=False,
@@ -375,7 +441,20 @@ def compile_single_file(file_path, minifier_actions, **kwargs):
     with open(file_path, 'r') as rf:
         file_lines = rf.readlines()
 
-    file_oneline = compile_single_file_lines(file_lines, minifier_actions, **kwargs)
+    # ksx statement expansion needs to happen recursively, and is best handled
+    # by not performing other minification actions first
+    actual_transpile_only = kwargs.get('transpile_only', False)
+    kwargs['transpile_only'] = True
+    file_oneline = compile_recursive_descent(file_lines, minifier_actions, **kwargs)
+    kwargs['transpile_only'] = actual_transpile_only
+
+    # do a final pass with non-recursive compiler to perform minification (I
+    # suppose it could use the recursive version too?)
+    if not actual_transpile_only:
+        file_oneline = compile_single_file_lines(
+            file_oneline.split('\n'),
+            minifier_actions,
+            **kwargs)
 
     with open(dest_path, 'w') as wf:
         wf.write(file_oneline)
@@ -422,12 +501,12 @@ def main_generate_parser():
         [ksx_expand_import, ["transpile-only", "safe"]],
         [ksx_expand_from_import, ["transpile-only", "safe"]],
         [ksx_remove_lines, ["transpile-only", "safe"]],
-        [min_strip_comments, ["safe"]],
-        [min_remove_whitespace, []],
-        [min_remove_blank_lines, ["safe"]],
+        [min_strip_comments, ["minify-only", "safe"]],
+        [min_remove_whitespace, ["minify-only"]],
+        [min_remove_blank_lines, ["minify-only", "safe"]],
     ],
     "oneline": [
-        [min_remove_useless_space, []],
+        [min_remove_useless_space, ["minify-only"]],
     ],
 }
 

tests/assertfiles/gh_issue_7p0_target.ks

@@ -0,0 +1,35 @@
+// this file imports another file
+function test_language_features {
+  parameter vessel is ship.
+
+  // get shortcut variables to vessel velocity and altitude
+  set v to vessel:velocity:surface:mag.
+  set h to vessel:altitude.
+
+  // always be moving
+  if v < 0.1 {
+    stage.
+  }
+
+  // if vessel has reached target altitude, reboot and allow bootloader to
+  // decide next action
+  if h > 80000 {
+    reboot.
+  }
+
+  set steering to heading(90, get_pitch_for_state()).
+  set throttle to get_throttle_for_twr(1.9).
+}
+
+// yes this is stupid, no I do not care
+function make_assertion {
+  parameter testParam.
+
+  return testParam = True.
+}
+
+function make_neg_assertion {
+  parameter testParam.
+
+  return testParam <> True.
+}

tests/assertfiles/gh_issue_7p1_target.ks

@@ -0,0 +1,10 @@
+
+print test_function().
+
+function test_function {
+  // need to make this do something...
+}
+
+function dont_really_care_function {
+  // this function intentionally blank
+}

tests/assertfiles/import_an_import.ksx

@@ -0,0 +1,8 @@
+// this file imports another file
+@ksx import ("import_pure_kerboscript").
+
+function make_neg_assertion {
+  parameter testParam.
+
+  return testParam <> True.
+}

tests/assertfiles/import_pure_kerboscript.ksx

@@ -0,0 +1,8 @@
+@ksx import ("pure_kerboscript").
+
+// yes this is stupid, no I do not care
+function make_assertion {
+  parameter testParam.
+
+  return testParam = True.
+}

tests/assertfiles/reference_loop_1.ksx

@@ -0,0 +1,9 @@
+@ksx import ("reference_loop_2").
+
+function test_function {
+  // need to make this do something...
+}
+
+function dont_really_care_function {
+  // this function intentionally blank
+}

tests/assertfiles/reference_loop_2.ksx

@@ -0,0 +1,3 @@
+@ksx import ("reference_loop_1").
+
+print test_function().

tests/test_known_issues.py

@@ -1,6 +1,7 @@
 # TODO: This should not be necessary, figure out how to get pytest to include
 # the root path in it search path, rather than descending into tests/
 import os; import sys; sys.path.append(os.path.abspath("."))
+import pytest
 import ksx
 
 
@@ -21,10 +22,9 @@ def assert_source_to_target(source, target, **kwargs):
         include_paths=kwargs.get('include_paths', ['tests/assertfiles']),
         **kwargs)
 
-    comparison = filecmp.cmp(
-        f'tests/assertfiles/{target}',
-        f'tests/genfiles/tests/assertfiles/{os.path.splitext(source)[0]}.ks',
-        shallow=False)
+    a = f'tests/assertfiles/{target}'
+    b = f'tests/genfiles/tests/assertfiles/{os.path.splitext(source)[0]}.ks'
+    comparison = filecmp.cmp(a, b, shallow=False)
 
     if not kwargs.get('skip_assert', False):
         assert comparison
@@ -71,3 +71,39 @@ def test_gh_issue_4p1():
         "gh_issue_4p1_source.ksx",
         "gh_issue_4_target.ksx",
         transpile_only=True)
+
+
+def test_gh_issue_7p0():
+    """Add recursive descent expansion of import statements
+
+    Currently, if you import a file that imports another file, only the first
+    file will be included, this is functionality breaking behavior and needs to
+    be fixed ASAP. Every import and from x import y statement should be
+    expanded recursively until there are no more statements to expand.
+
+    I can easily see this leading to duplicate imports though, so that needs to
+    be considered, it would be a shame if the 'minifier' tool ended up with
+    exploding file sizes because of duplicate imports.
+    """
+    assert_source_to_target(
+        "import_an_import.ksx",
+        "gh_issue_7p0_target.ks",
+        transpile_only=True)
+
+
+@pytest.mark.skip(reason="Circular imports are still a known issue")
+def test_gh_issue_7p1():
+    """File 1 imports File 2, which in turn imports File 1"""
+    assert_source_to_target(
+        "reference_loop_1.ksx",
+        "gh_issue_7p1_target.ks",
+        transpile_only=True)
+
+
+@pytest.mark.skip(reason="Circular imports are still a known issue")
+def test_gh_issue_7p2():
+    """File 2 imports File 1, which in turn imports File 2 (inverse of 7p1)"""
+    assert_source_to_target(
+        "reference_loop_2.ksx",
+        "gh_issue_7p2_target.ks",
+        transpile_only=True)