[TypeScript] Fix namespaceless schema generation (#7432)

* Add tests for ts flatc

* fix typescript generation of namespaceless file

* Add typescript genereated code from generate_code.py

* Support multiple module flatc testing

src/idl_gen_ts.cpp

@@ -26,7 +26,7 @@
 #include "flatbuffers/util.h"
 
 namespace flatbuffers {
-
+namespace {
 struct ImportDefinition {
   std::string name;
   std::string import_statement;
@@ -38,6 +38,7 @@ struct ImportDefinition {
 };
 
 enum AnnotationType { kParam = 0, kType = 1, kReturns = 2 };
+}
 
 namespace ts {
 // Iterate through all definitions we haven't generate code for (enums, structs,
@@ -111,7 +112,7 @@ class TsGenerator : public BaseGenerator {
   }
   bool generate() {
     if (parser_.opts.ts_flat_file && parser_.opts.generate_all) {
-      // Not implemented; warning message should have beem emitted by flatc.
+      // Not implemented; warning message should have been emitted by flatc.
       return false;
     }
     generateEnums();
@@ -122,9 +123,9 @@ class TsGenerator : public BaseGenerator {
 
   // Save out the generated code for a single class while adding
   // declaration boilerplate.
-  bool SaveType(const Definition &definition, const std::string &classcode,
+  bool SaveType(const Definition &definition, const std::string &class_code,
                 import_set &imports, import_set &bare_imports) {
-    if (!classcode.length()) return true;
+    if (!class_code.length()) return true;
 
     std::string code;
 
@@ -144,16 +145,27 @@ class TsGenerator : public BaseGenerator {
       if (!imports.empty()) code += "\n\n";
     }
 
-    code += classcode;
+    code += class_code;
-    auto filename =
+
-        NamespaceDir(*definition.defined_namespace, true) +
-        ConvertCase(definition.name, Case::kDasher, Case::kUpperCamel) + ".ts";
     if (parser_.opts.ts_flat_file) {
       flat_file_ += code;
       flat_file_definitions_.insert(&definition);
       return true;
     } else {
-      return SaveFile(filename.c_str(), code, false);
+      auto basename =
+          NamespaceDir(*definition.defined_namespace, true) +
+          ConvertCase(definition.name, Case::kDasher, Case::kUpperCamel);
+
+      // Special case for the root table, generate an export statement
+      if (&definition == parser_.root_struct_def_) {
+        ImportDefinition import;
+        import.name = definition.name;
+        import.export_statement =
+            "export { " + import.name + " } from './" + basename + "';";
+        imports.insert(std::make_pair(import.name, import));
+      }
+
+      return SaveFile((basename + ".ts").c_str(), code, false);
     }
   }
 
@@ -227,9 +239,7 @@ class TsGenerator : public BaseGenerator {
         // require modifying AddImport to ensure that we don't use
         // namespace-prefixed names anywhere...
         std::string file = it.first;
-        if (file.empty()) {
+        if (file.empty()) { continue; }
-          continue;
-        }
         std::string noext = flatbuffers::StripExtension(file);
         std::string basename = flatbuffers::StripPath(noext);
         std::string include_file = GeneratedFileName(
@@ -239,14 +249,15 @@ class TsGenerator : public BaseGenerator {
         // specified here? Should we always be adding the "./" for a relative
         // path or turn it off if --include-prefix is specified, or something
         // else?
-        std::string include_name = "./" + flatbuffers::StripExtension(include_file);
+        std::string include_name =
+            "./" + flatbuffers::StripExtension(include_file);
         code += "import {";
         for (const auto &pair : it.second) {
           code += EscapeKeyword(pair.first) + " as " +
                   EscapeKeyword(pair.second) + ", ";
         }
         code.resize(code.size() - 2);
-        code += "} from  '" + include_name + "';\n";
+        code += "} from '" + include_name + "';\n";
       }
       code += "\n\n";
       code += flat_file_;
@@ -257,7 +268,8 @@ class TsGenerator : public BaseGenerator {
       for (auto it = imports_all_.begin(); it != imports_all_.end(); it++) {
         code += it->second.export_statement + "\n";
       }
-      std::string path = "./" + path_ + file_name_ + ".ts";
+      const std::string path =
+          GeneratedFileName(path_, file_name_, parser_.opts);
       SaveFile(path.c_str(), code, false);
     }
   }
@@ -1110,14 +1122,12 @@ class TsGenerator : public BaseGenerator {
             field_type += GetObjApiClassName(AddImport(imports, struct_def, sd),
                                              parser.opts);
 
-            const std::string field_accessor =
+            const std::string field_accessor = "this." + field_name + "()";
-                "this." + field_name + "()";
             field_val = GenNullCheckConditional(field_accessor,
                                                 field_accessor + "!.unpack()");
             auto packing = GenNullCheckConditional(
                 "this." + field_name_escaped,
-                "this." + field_name_escaped + "!.pack(builder)",
+                "this." + field_name_escaped + "!.pack(builder)", "0");
-                "0");
 
             if (sd.fixed) {
               field_offset_val = std::move(packing);
@@ -1248,8 +1258,7 @@ class TsGenerator : public BaseGenerator {
       // FIXME: if field_type and field_name_escaped are identical, then
       // this generates invalid typescript.
       constructor_func += "  public " + field_name_escaped + ": " + field_type +
-                          " = " +
+                          " = " + field_default_val;
-                          field_default_val;
 
       if (!struct_def.fixed) {
         if (!field_offset_decl.empty()) {
@@ -1260,7 +1269,8 @@ class TsGenerator : public BaseGenerator {
           pack_func_create_call += field_offset_val;
         } else {
           if (field.IsScalarOptional()) {
-            pack_func_create_call += "  if (" + field_offset_val + " !== null)\n  ";
+            pack_func_create_call +=
+                "  if (" + field_offset_val + " !== null)\n  ";
           }
           pack_func_create_call += "  " + struct_name + ".add" +
                                    ConvertCase(field.name, Case::kUpperCamel) +
@@ -1768,8 +1778,8 @@ class TsGenerator : public BaseGenerator {
         }
 
         code += "):flatbuffers.Offset {\n";
-        code += "  " + object_name + ".start" +
+        code += "  " + object_name + ".start" + GetPrefixedName(struct_def) +
-                GetPrefixedName(struct_def) + "(builder);\n";
+                "(builder);\n";
 
         std::string methodPrefix = object_name;
         for (auto it = struct_def.fields.vec.begin();

tests/flatc/flatc_test.py

@@ -1,5 +1,3 @@
-#!/usr/bin/env python3
-#
 # Copyright 2022 Google Inc. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -15,10 +13,7 @@
 # limitations under the License.
 
 import argparse
-import filecmp
-import glob
 import platform
-import shutil
 import subprocess
 from pathlib import Path
 
@@ -53,7 +48,7 @@ assert flatc_path.exists(), "Cannot find the flatc compiler " + str(flatc_path)
 # Execute the flatc compiler with the specified parameters
 def flatc(options, cwd=script_path):
     cmd = [str(flatc_path)] + options
-    result = subprocess.run(cmd, cwd=str(cwd), check=True)
+    subprocess.check_call(cmd, cwd=str(cwd))
 
 
 def make_absolute(filename, path=script_path):
@@ -66,33 +61,53 @@ def assert_file_exists(filename, path=script_path):
     return file
 
 
-def assert_file_contains(file, needle):
+def assert_file_doesnt_exists(filename, path=script_path):
-    assert needle in open(file).read(), (
+    file = Path(path, filename)
-        "coudn't find '" + needle + "' in file: " + str(file)
+    assert not file.exists(), "file exists but shouldn't: " + filename
-    )
     return file
 
 
-def assert_file_and_contents(file, needle, path=script_path):
+def assert_file_contains(file, needles):
-    assert_file_contains(assert_file_exists(file, path), needle).unlink()
+    with open(file) as file:
+        contents = file.read()
+        for needle in [needles] if isinstance(needles, str) else needles:
+            assert needle in contents, (
+                "coudn't find '" + needle + "' in file: " + str(file)
+            )
+    return file
 
 
-def run_all(module):
+def assert_file_and_contents(file, needle, path=script_path, unlink=True):
-    methods = [
+    assert_file_contains(assert_file_exists(file, path), needle)
-        func
+    if unlink:
-        for func in dir(module)
+        Path(path, file).unlink()
-        if callable(getattr(module, func)) and not func.startswith("__")
+
-    ]
+
+def run_all(*modules):
     failing = 0
     passing = 0
-    for method in methods:
+    for module in modules:
-        try:
+        methods = [
-            print(method)
+            func
-            getattr(module, method)(module)
+            for func in dir(module)
-            print(" [PASSED]")
+            if callable(getattr(module, func)) and not func.startswith("__")
-            passing = passing + 1
+        ]
-        except Exception as e:
+        module_failing = 0
-            print(" [FAILED]: " + str(e))
+        module_passing = 0
-            failing = failing + 1
+        for method in methods:
-    print("{0}: {1} of {2} passsed".format(module.__name__, passing, passing + failing))
+            try:
+                print("{0}.{1}".format(module.__name__, method))
+                getattr(module, method)(module)
+                print(" [PASSED]")
+                module_passing = module_passing + 1
+            except Exception as e:
+                print(" [FAILED]: " + str(e))
+                failingmodule_failing = failingmodule_failing + 1
+        print(
+            "{0}: {1} of {2} passsed".format(
+                module.__name__, module_passing, module_passing + module_failing
+            )
+        )
+        passing = passing + module_passing
+        failing = failing + module_failing
     return passing, failing

tests/flatc/flatc_ts_tests.py

@@ -0,0 +1,69 @@
+# Copyright 2022 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from flatc_test import *
+
+class TsTests():
+
+    def Base(self):
+        # Generate just foo with no extra arguments
+        flatc(["--ts", "foo.fbs"])
+
+        # Should generate the module that exports both foo and its direct
+        # include, bar.
+        assert_file_and_contents(
+            "foo_generated.ts",
+            ["export { Bar } from './bar';", "export { Foo } from './foo';"],
+        )
+
+        # Foo should be generated in place and exports the Foo table.
+        assert_file_and_contents("foo.ts", "export class Foo {")
+
+        # Included files, like bar, should not be generated.
+        assert_file_doesnt_exists("bar.ts")
+
+    def BaseWithNamespace(self):
+        # Generate foo with namespacing, with no extra arguments
+        flatc(["--ts", "foo_with_ns.fbs"])
+
+        # Should generate the module that exports both foo in its namespace
+        # directory and its direct include, bar.
+        assert_file_and_contents(
+            "foo_with_ns_generated.ts",
+            ["export { Bar } from './bar';", "export { Foo } from './something/foo';"],
+        )
+
+        # Foo should be placed in the namespaced directory. It should export
+        # Foo, and the import of Bar should be relative to its location.
+        assert_file_and_contents(
+            "something/foo.ts",
+            ["export class Foo {", "import { Bar } from '../bar';"],
+        )
+
+        # Included files, like bar, should not be generated.
+        assert_file_doesnt_exists("bar.ts")
+
+    def FlatFiles(self):
+        # Generate just foo the flat files option
+        flatc(["--ts", "--ts-flat-files", "foo.fbs"])
+
+        # Should generate a single file that imports bar as a single file, and]
+        # exports the Foo table.
+        assert_file_and_contents(
+            "foo_generated.ts",
+            ["import {Bar as Bar} from './bar_generated';", "export class Foo {"],
+        )
+
+        # The root type Foo should not be generated in its own file.
+        assert_file_doesnt_exists("foo.ts")

tests/flatc/foo_with_ns.fbs

@@ -0,0 +1,9 @@
+include "bar/bar.fbs";
+
+namespace something;
+
+table Foo {
+  bar:Bar;
+}
+
+root_type Foo;

tests/flatc/main.py

@@ -1,11 +1,29 @@
 #!/usr/bin/env python3
+#
+# Copyright 2022 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
 
-import sys;
+import sys
 
 from flatc_test import run_all
 from flatc_cpp_tests import CppTests
+from flatc_ts_tests import TsTests
 
-passing, failing = run_all(CppTests)
+passing, failing = run_all(CppTests, TsTests)
+
+print("")
+print("{0} of {1} tests passed".format(passing, passing + failing))
 
 if failing > 0:
-  sys.exit(1)
+    sys.exit(1)

tests/monster_test_generated.ts

@@ -0,0 +1,18 @@
+export { Monster } from './my-game/example/monster';
+export { Monster as MyGameExample2Monster, MonsterT as MyGameExample2MonsterT } from './my-game/example2/monster';
+export { Ability, AbilityT } from './my-game/example/ability';
+export { Any, unionToAny, unionListToAny } from './my-game/example/any';
+export { AnyAmbiguousAliases, unionToAnyAmbiguousAliases, unionListToAnyAmbiguousAliases } from './my-game/example/any-ambiguous-aliases';
+export { AnyUniqueAliases, unionToAnyUniqueAliases, unionListToAnyUniqueAliases } from './my-game/example/any-unique-aliases';
+export { Color } from './my-game/example/color';
+export { Monster, MonsterT } from './my-game/example/monster';
+export { Race } from './my-game/example/race';
+export { Referrable, ReferrableT } from './my-game/example/referrable';
+export { Stat, StatT } from './my-game/example/stat';
+export { StructOfStructs, StructOfStructsT } from './my-game/example/struct-of-structs';
+export { StructOfStructsOfStructs, StructOfStructsOfStructsT } from './my-game/example/struct-of-structs-of-structs';
+export { Test, TestT } from './my-game/example/test';
+export { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from './my-game/example/test-simple-table-with-enum';
+export { TypeAliases, TypeAliasesT } from './my-game/example/type-aliases';
+export { Vec3, Vec3T } from './my-game/example/vec3';
+export { InParentNamespace, InParentNamespaceT } from './my-game/in-parent-namespace';

tests/optional_scalars_generated.ts

@@ -0,0 +1,2 @@
+export { ScalarStuff } from './optional-scalars/scalar-stuff';
+export { OptionalByte } from './optional-scalars/optional-byte';

tests/union_vector/union_vector_generated.ts

@@ -0,0 +1,8 @@
+export { Attacker, AttackerT } from './attacker';
+export { BookReader, BookReaderT } from './book-reader';
+export { Character, unionToCharacter, unionListToCharacter } from './character';
+export { FallingTub, FallingTubT } from './falling-tub';
+export { Gadget, unionToGadget, unionListToGadget } from './gadget';
+export { HandFan, HandFanT } from './hand-fan';
+export { Movie, MovieT } from './movie';
+export { Rapunzel, RapunzelT } from './rapunzel';