From fa1174aa7b73ea814e023deda7375cb8e49996e8 Mon Sep 17 00:00:00 2001 From: Derek Bailey Date: Sun, 14 Aug 2022 11:34:19 -0700 Subject: [PATCH] [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 | 58 +++++++++------- tests/flatc/flatc_test.py | 71 ++++++++++++-------- tests/flatc/flatc_ts_tests.py | 69 +++++++++++++++++++ tests/flatc/foo_with_ns.fbs | 9 +++ tests/flatc/main.py | 24 ++++++- tests/monster_test_generated.ts | 18 +++++ tests/optional_scalars_generated.ts | 2 + tests/union_vector/union_vector_generated.ts | 8 +++ 8 files changed, 204 insertions(+), 55 deletions(-) create mode 100755 tests/flatc/flatc_ts_tests.py create mode 100644 tests/flatc/foo_with_ns.fbs create mode 100644 tests/monster_test_generated.ts create mode 100644 tests/optional_scalars_generated.ts create mode 100644 tests/union_vector/union_vector_generated.ts diff --git a/src/idl_gen_ts.cpp b/src/idl_gen_ts.cpp index b9670b919..1533825a9 100644 --- a/src/idl_gen_ts.cpp +++ b/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; - auto filename = - NamespaceDir(*definition.defined_namespace, true) + - ConvertCase(definition.name, Case::kDasher, Case::kUpperCamel) + ".ts"; + code += class_code; + 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()) { - continue; - } + if (file.empty()) { 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 = - "this." + field_name + "()"; + const std::string field_accessor = "this." + field_name + "()"; field_val = GenNullCheckConditional(field_accessor, field_accessor + "!.unpack()"); auto packing = GenNullCheckConditional( "this." + field_name_escaped, - "this." + field_name_escaped + "!.pack(builder)", - "0"); + "this." + field_name_escaped + "!.pack(builder)", "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" + - GetPrefixedName(struct_def) + "(builder);\n"; + code += " " + object_name + ".start" + GetPrefixedName(struct_def) + + "(builder);\n"; std::string methodPrefix = object_name; for (auto it = struct_def.fields.vec.begin(); diff --git a/tests/flatc/flatc_test.py b/tests/flatc/flatc_test.py index f315d68f1..a86e432a2 100755 --- a/tests/flatc/flatc_test.py +++ b/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): - assert needle in open(file).read(), ( - "coudn't find '" + needle + "' in file: " + str(file) - ) +def assert_file_doesnt_exists(filename, path=script_path): + file = Path(path, filename) + assert not file.exists(), "file exists but shouldn't: " + filename return file -def assert_file_and_contents(file, needle, path=script_path): - assert_file_contains(assert_file_exists(file, path), needle).unlink() +def assert_file_contains(file, needles): + 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): - methods = [ - func - for func in dir(module) - if callable(getattr(module, func)) and not func.startswith("__") - ] +def assert_file_and_contents(file, needle, path=script_path, unlink=True): + assert_file_contains(assert_file_exists(file, path), needle) + if unlink: + Path(path, file).unlink() + + +def run_all(*modules): failing = 0 passing = 0 - for method in methods: - try: - print(method) - getattr(module, method)(module) - print(" [PASSED]") - passing = passing + 1 - except Exception as e: - print(" [FAILED]: " + str(e)) - failing = failing + 1 - print("{0}: {1} of {2} passsed".format(module.__name__, passing, passing + failing)) + for module in modules: + methods = [ + func + for func in dir(module) + if callable(getattr(module, func)) and not func.startswith("__") + ] + module_failing = 0 + module_passing = 0 + for method in methods: + 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 diff --git a/tests/flatc/flatc_ts_tests.py b/tests/flatc/flatc_ts_tests.py new file mode 100755 index 000000000..3617c449c --- /dev/null +++ b/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") diff --git a/tests/flatc/foo_with_ns.fbs b/tests/flatc/foo_with_ns.fbs new file mode 100644 index 000000000..dd36fdc8c --- /dev/null +++ b/tests/flatc/foo_with_ns.fbs @@ -0,0 +1,9 @@ +include "bar/bar.fbs"; + +namespace something; + +table Foo { + bar:Bar; +} + +root_type Foo; \ No newline at end of file diff --git a/tests/flatc/main.py b/tests/flatc/main.py index da3653186..1b6bee90d 100755 --- a/tests/flatc/main.py +++ b/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) diff --git a/tests/monster_test_generated.ts b/tests/monster_test_generated.ts new file mode 100644 index 000000000..56b4cc14f --- /dev/null +++ b/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'; diff --git a/tests/optional_scalars_generated.ts b/tests/optional_scalars_generated.ts new file mode 100644 index 000000000..9a2d71e58 --- /dev/null +++ b/tests/optional_scalars_generated.ts @@ -0,0 +1,2 @@ +export { ScalarStuff } from './optional-scalars/scalar-stuff'; +export { OptionalByte } from './optional-scalars/optional-byte'; diff --git a/tests/union_vector/union_vector_generated.ts b/tests/union_vector/union_vector_generated.ts new file mode 100644 index 000000000..f3a118b7d --- /dev/null +++ b/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';