From 7334b4e03aa46d71eb1994bde9d151ee4370a42e Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 23 Jun 2022 17:05:21 +0200 Subject: [PATCH] Swift: autopep8 all python files Additionally set up a pre-commit hook and a CI check for that. --- .github/workflows/swift-codegen.yml | 14 +- .pre-commit-config.yaml | 6 + swift/codegen/.pep8 | 2 + swift/codegen/generators/dbschemegen.py | 8 +- swift/codegen/test/test_cppgen.py | 62 +++--- swift/codegen/test/test_qlgen.py | 204 +++++++++--------- swift/codegen/test/test_schema.py | 1 - swift/codegen/test/test_trapgen.py | 5 +- .../cross-references/test.py | 1 - 9 files changed, 157 insertions(+), 146 deletions(-) create mode 100644 swift/codegen/.pep8 diff --git a/.github/workflows/swift-codegen.yml b/.github/workflows/swift-codegen.yml index d5d74af87c2..b853f10e23a 100644 --- a/.github/workflows/swift-codegen.yml +++ b/.github/workflows/swift-codegen.yml @@ -15,14 +15,18 @@ jobs: - uses: actions/checkout@v3 - uses: ./.github/actions/fetch-codeql - uses: bazelbuild/setup-bazelisk@v2 + - uses: actions/setup-python@v3 + - uses: pre-commit/action@v3.0.0 + name: Check that python code is properly formatted + with: + extra_args: autopep8 --all-files - name: Run unit tests run: | bazel test //swift/codegen/test --test_output=errors - - name: Check that QL generated code was checked in - run: | - bazel run //swift/codegen - git add swift - git diff --exit-code HEAD + - uses: pre-commit/action@v3.0.0 + name: Check that QL generated code was checked in + with: + extra_args: swift-codegen --all-files - name: Generate C++ files run: | bazel run //swift/codegen:codegen -- --generate=trap,cpp --cpp-output=$PWD/swift-generated-headers diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1b44de000fb..d51681aa65c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,6 +15,12 @@ repos: - id: clang-format files: ^swift/.*\.(h|c|cpp)$ + - repo: https://github.com/pre-commit/mirrors-autopep8 + rev: v1.6.0 + hooks: + - id: autopep8 + files: ^swift/codegen/.*\.py + - repo: local hooks: - id: codeql-format diff --git a/swift/codegen/.pep8 b/swift/codegen/.pep8 new file mode 100644 index 00000000000..0dfcfdf2d3c --- /dev/null +++ b/swift/codegen/.pep8 @@ -0,0 +1,2 @@ +[pep8] +max_line_length = 120 diff --git a/swift/codegen/generators/dbschemegen.py b/swift/codegen/generators/dbschemegen.py index 650c5a7e40b..3eb8298c16c 100755 --- a/swift/codegen/generators/dbschemegen.py +++ b/swift/codegen/generators/dbschemegen.py @@ -46,10 +46,10 @@ def cls_to_dbscheme(cls: schema.Class): keyset=keyset, name=inflection.tableize(cls.name), columns=[ - Column("id", type=dbtype(cls.name), binding=binding), - ] + [ - Column(f.name, dbtype(f.type)) for f in cls.properties if f.is_single - ] + Column("id", type=dbtype(cls.name), binding=binding), + ] + [ + Column(f.name, dbtype(f.type)) for f in cls.properties if f.is_single + ] ) # use property-specific tables for 1-to-many and 1-to-at-most-1 properties for f in cls.properties: diff --git a/swift/codegen/test/test_cppgen.py b/swift/codegen/test/test_cppgen.py index c1f205eaa3b..5577052ccfb 100644 --- a/swift/codegen/test/test_cppgen.py +++ b/swift/codegen/test/test_cppgen.py @@ -30,8 +30,8 @@ def test_empty_class(generate): assert generate([ schema.Class(name="MyClass"), ]) == [ - cpp.Class(name="MyClass", final=True, trap_name="MyClasses") - ] + cpp.Class(name="MyClass", final=True, trap_name="MyClasses") + ] def test_two_class_hierarchy(generate): @@ -40,9 +40,9 @@ def test_two_class_hierarchy(generate): schema.Class(name="A", derived={"B"}), schema.Class(name="B", bases={"A"}), ]) == [ - base, - cpp.Class(name="B", bases=[base], final=True, trap_name="Bs"), - ] + base, + cpp.Class(name="B", bases=[base], final=True, trap_name="Bs"), + ] def test_complex_hierarchy_topologically_ordered(generate): @@ -78,23 +78,23 @@ def test_class_with_field(generate, type, expected, property_cls, optional, repe assert generate([ schema.Class(name="MyClass", properties=[property_cls("prop", type)]), ]) == [ - cpp.Class(name="MyClass", - fields=[cpp.Field("prop", expected, is_optional=optional, - is_repeated=repeated, trap_name=trap_name)], - trap_name="MyClasses", - final=True) - ] + cpp.Class(name="MyClass", + fields=[cpp.Field("prop", expected, is_optional=optional, + is_repeated=repeated, trap_name=trap_name)], + trap_name="MyClasses", + final=True) + ] def test_class_with_predicate(generate): assert generate([ schema.Class(name="MyClass", properties=[schema.PredicateProperty("prop")]), ]) == [ - cpp.Class(name="MyClass", - fields=[cpp.Field("prop", "bool", trap_name="MyClassProp", is_predicate=True)], - trap_name="MyClasses", - final=True) - ] + cpp.Class(name="MyClass", + fields=[cpp.Field("prop", "bool", trap_name="MyClassProp", is_predicate=True)], + trap_name="MyClasses", + final=True) + ] @pytest.mark.parametrize("name", @@ -104,11 +104,11 @@ def test_class_with_overridden_unsigned_field(generate, name): schema.Class(name="MyClass", properties=[ schema.SingleProperty(name, "bar")]), ]) == [ - cpp.Class(name="MyClass", - fields=[cpp.Field(name, "unsigned")], - trap_name="MyClasses", - final=True) - ] + cpp.Class(name="MyClass", + fields=[cpp.Field(name, "unsigned")], + trap_name="MyClasses", + final=True) + ] def test_class_with_overridden_underscore_field(generate): @@ -116,11 +116,11 @@ def test_class_with_overridden_underscore_field(generate): schema.Class(name="MyClass", properties=[ schema.SingleProperty("something_", "bar")]), ]) == [ - cpp.Class(name="MyClass", - fields=[cpp.Field("something", "bar")], - trap_name="MyClasses", - final=True) - ] + cpp.Class(name="MyClass", + fields=[cpp.Field("something", "bar")], + trap_name="MyClasses", + final=True) + ] @pytest.mark.parametrize("name", cpp.cpp_keywords) @@ -129,11 +129,11 @@ def test_class_with_keyword_field(generate, name): schema.Class(name="MyClass", properties=[ schema.SingleProperty(name, "bar")]), ]) == [ - cpp.Class(name="MyClass", - fields=[cpp.Field(name + "_", "bar")], - trap_name="MyClasses", - final=True) - ] + cpp.Class(name="MyClass", + fields=[cpp.Field(name + "_", "bar")], + trap_name="MyClasses", + final=True) + ] if __name__ == '__main__': diff --git a/swift/codegen/test/test_qlgen.py b/swift/codegen/test/test_qlgen.py index a073499d60a..db6d0fbdf0e 100644 --- a/swift/codegen/test/test_qlgen.py +++ b/swift/codegen/test/test_qlgen.py @@ -128,9 +128,9 @@ def test_one_empty_class(generate_classes): assert generate_classes([ schema.Class("A") ]) == { - "A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "A"), - ql.Class(name="A", final=True)), - } + "A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "A"), + ql.Class(name="A", final=True)), + } def test_hierarchy(generate_classes): @@ -140,16 +140,16 @@ def test_hierarchy(generate_classes): schema.Class("B", bases={"A"}, derived={"D"}), schema.Class("A", derived={"B", "C"}), ]) == { - "A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "A"), - ql.Class(name="A")), - "B.qll": (ql.Stub(name="B", base_import=gen_import_prefix + "B"), - ql.Class(name="B", bases=["A"], imports=[stub_import_prefix + "A"])), - "C.qll": (ql.Stub(name="C", base_import=gen_import_prefix + "C"), - ql.Class(name="C", bases=["A"], imports=[stub_import_prefix + "A"])), - "D.qll": (ql.Stub(name="D", base_import=gen_import_prefix + "D"), - ql.Class(name="D", final=True, bases=["B", "C"], - imports=[stub_import_prefix + cls for cls in "BC"])), - } + "A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "A"), + ql.Class(name="A")), + "B.qll": (ql.Stub(name="B", base_import=gen_import_prefix + "B"), + ql.Class(name="B", bases=["A"], imports=[stub_import_prefix + "A"])), + "C.qll": (ql.Stub(name="C", base_import=gen_import_prefix + "C"), + ql.Class(name="C", bases=["A"], imports=[stub_import_prefix + "A"])), + "D.qll": (ql.Stub(name="D", base_import=gen_import_prefix + "D"), + ql.Class(name="D", final=True, bases=["B", "C"], + imports=[stub_import_prefix + cls for cls in "BC"])), + } def test_hierarchy_imports(generate_import_list): @@ -184,12 +184,12 @@ def test_single_property(generate_classes): schema.Class("MyObject", properties=[ schema.SingleProperty("foo", "bar")]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="Foo", type="bar", tablename="my_objects", - tableparams=["this", "result"]), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="Foo", type="bar", tablename="my_objects", + tableparams=["this", "result"]), + ])), + } def test_single_properties(generate_classes): @@ -200,16 +200,16 @@ def test_single_properties(generate_classes): schema.SingleProperty("three", "z"), ]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="One", type="x", tablename="my_objects", - tableparams=["this", "result", "_", "_"]), - ql.Property(singular="Two", type="y", tablename="my_objects", - tableparams=["this", "_", "result", "_"]), - ql.Property(singular="Three", type="z", tablename="my_objects", - tableparams=["this", "_", "_", "result"]), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="One", type="x", tablename="my_objects", + tableparams=["this", "result", "_", "_"]), + ql.Property(singular="Two", type="y", tablename="my_objects", + tableparams=["this", "_", "result", "_"]), + ql.Property(singular="Three", type="z", tablename="my_objects", + tableparams=["this", "_", "_", "result"]), + ])), + } @pytest.mark.parametrize("is_child", [False, True]) @@ -218,13 +218,13 @@ def test_optional_property(generate_classes, is_child): schema.Class("MyObject", properties=[ schema.OptionalProperty("foo", "bar", is_child=is_child)]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="Foo", type="bar", tablename="my_object_foos", - tableparams=["this", "result"], - is_optional=True, is_child=is_child), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="Foo", type="bar", tablename="my_object_foos", + tableparams=["this", "result"], + is_optional=True, is_child=is_child), + ])), + } @pytest.mark.parametrize("is_child", [False, True]) @@ -233,12 +233,12 @@ def test_repeated_property(generate_classes, is_child): schema.Class("MyObject", properties=[ schema.RepeatedProperty("foo", "bar", is_child=is_child)]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="Foo", plural="Foos", type="bar", tablename="my_object_foos", - tableparams=["this", "index", "result"], is_child=is_child), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="Foo", plural="Foos", type="bar", tablename="my_object_foos", + tableparams=["this", "index", "result"], is_child=is_child), + ])), + } @pytest.mark.parametrize("is_child", [False, True]) @@ -247,13 +247,13 @@ def test_repeated_optional_property(generate_classes, is_child): schema.Class("MyObject", properties=[ schema.RepeatedOptionalProperty("foo", "bar", is_child=is_child)]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="Foo", plural="Foos", type="bar", tablename="my_object_foos", - tableparams=["this", "index", "result"], is_optional=True, - is_child=is_child), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="Foo", plural="Foos", type="bar", tablename="my_object_foos", + tableparams=["this", "index", "result"], is_optional=True, + is_child=is_child), + ])), + } def test_predicate_property(generate_classes): @@ -261,13 +261,13 @@ def test_predicate_property(generate_classes): schema.Class("MyObject", properties=[ schema.PredicateProperty("is_foo")]), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class(name="MyObject", final=True, properties=[ - ql.Property(singular="isFoo", type="predicate", tablename="my_object_is_foo", - tableparams=["this"], - is_predicate=True), - ])), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class(name="MyObject", final=True, properties=[ + ql.Property(singular="isFoo", type="predicate", tablename="my_object_is_foo", + tableparams=["this"], + is_predicate=True), + ])), + } @pytest.mark.parametrize("is_child", [False, True]) @@ -277,18 +277,18 @@ def test_single_class_property(generate_classes, is_child): schema.SingleProperty("foo", "Bar", is_child=is_child)]), schema.Class("Bar"), ]) == { - "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), - ql.Class( - name="MyObject", final=True, imports=[stub_import_prefix + "Bar"], properties=[ - ql.Property(singular="Foo", type="Bar", tablename="my_objects", - tableparams=[ - "this", "result"], - is_child=is_child), - ], - )), - "Bar.qll": (ql.Stub(name="Bar", base_import=gen_import_prefix + "Bar"), - ql.Class(name="Bar", final=True)), - } + "MyObject.qll": (ql.Stub(name="MyObject", base_import=gen_import_prefix + "MyObject"), + ql.Class( + name="MyObject", final=True, imports=[stub_import_prefix + "Bar"], properties=[ + ql.Property(singular="Foo", type="Bar", tablename="my_objects", + tableparams=[ + "this", "result"], + is_child=is_child), + ], + )), + "Bar.qll": (ql.Stub(name="Bar", base_import=gen_import_prefix + "Bar"), + ql.Class(name="Bar", final=True)), + } def test_class_dir(generate_classes): @@ -297,12 +297,12 @@ def test_class_dir(generate_classes): schema.Class("A", derived={"B"}, dir=dir), schema.Class("B", bases={"A"}), ]) == { - f"{dir}/A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "another.rel.path.A"), - ql.Class(name="A", dir=dir)), - "B.qll": (ql.Stub(name="B", base_import=gen_import_prefix + "B"), - ql.Class(name="B", final=True, bases=["A"], - imports=[stub_import_prefix + "another.rel.path.A"])), - } + f"{dir}/A.qll": (ql.Stub(name="A", base_import=gen_import_prefix + "another.rel.path.A"), + ql.Class(name="A", dir=dir)), + "B.qll": (ql.Stub(name="B", base_import=gen_import_prefix + "B"), + ql.Class(name="B", final=True, bases=["A"], + imports=[stub_import_prefix + "another.rel.path.A"])), + } def test_class_dir_imports(generate_import_list): @@ -382,8 +382,8 @@ def test_test_source_present(opts, generate_tests): assert generate_tests([ schema.Class("A"), ]) == { - "A/A.ql": ql.ClassTester(class_name="A"), - } + "A/A.ql": ql.ClassTester(class_name="A"), + } def test_test_source_present_with_dir(opts, generate_tests): @@ -391,8 +391,8 @@ def test_test_source_present_with_dir(opts, generate_tests): assert generate_tests([ schema.Class("A", dir=pathlib.Path("foo")), ]) == { - "foo/A/A.ql": ql.ClassTester(class_name="A"), - } + "foo/A/A.ql": ql.ClassTester(class_name="A"), + } def test_test_total_properties(opts, generate_tests): @@ -405,13 +405,13 @@ def test_test_total_properties(opts, generate_tests): schema.PredicateProperty("y", "int"), ]), ]) == { - "B/B.ql": ql.ClassTester(class_name="B", properties=[ - ql.PropertyForTest( - getter="getX", is_single=True, type="string"), - ql.PropertyForTest( - getter="y", is_predicate=True, type="predicate"), - ]) - } + "B/B.ql": ql.ClassTester(class_name="B", properties=[ + ql.PropertyForTest( + getter="getX", is_single=True, type="string"), + ql.PropertyForTest( + getter="y", is_predicate=True, type="predicate"), + ]) + } def test_test_partial_properties(opts, generate_tests): @@ -424,13 +424,13 @@ def test_test_partial_properties(opts, generate_tests): schema.RepeatedProperty("y", "int"), ]), ]) == { - "B/B.ql": ql.ClassTester(class_name="B"), - "B/B_getX.ql": ql.PropertyTester(class_name="B", - property=ql.PropertyForTest(getter="getX", type="string")), - "B/B_getY.ql": ql.PropertyTester(class_name="B", - property=ql.PropertyForTest(getter="getY", is_repeated=True, + "B/B.ql": ql.ClassTester(class_name="B"), + "B/B_getX.ql": ql.PropertyTester(class_name="B", + property=ql.PropertyForTest(getter="getX", type="string")), + "B/B_getY.ql": ql.PropertyTester(class_name="B", + property=ql.PropertyForTest(getter="getY", is_repeated=True, type="int")), - } + } def test_test_properties_deduplicated(opts, generate_tests): @@ -444,14 +444,14 @@ def test_test_properties_deduplicated(opts, generate_tests): schema.Class("B", bases={"Base"}, derived={"Final"}), schema.Class("Final", bases={"A", "B"}), ]) == { - "Final/Final.ql": ql.ClassTester(class_name="Final", properties=[ - ql.PropertyForTest( - getter="getX", is_single=True, type="string"), - ]), - "Final/Final_getY.ql": ql.PropertyTester(class_name="Final", - property=ql.PropertyForTest(getter="getY", is_repeated=True, + "Final/Final.ql": ql.ClassTester(class_name="Final", properties=[ + ql.PropertyForTest( + getter="getX", is_single=True, type="string"), + ]), + "Final/Final_getY.ql": ql.PropertyTester(class_name="Final", + property=ql.PropertyForTest(getter="getY", is_repeated=True, type="int")), - } + } def test_test_properties_skipped(opts, generate_tests): @@ -467,8 +467,8 @@ def test_test_properties_skipped(opts, generate_tests): "b", "int", pragmas=["bar", "skip_qltest", "baz"]), ]), ]) == { - "Derived/Derived.ql": ql.ClassTester(class_name="Derived"), - } + "Derived/Derived.ql": ql.ClassTester(class_name="Derived"), + } def test_test_base_class_skipped(opts, generate_tests): @@ -480,8 +480,8 @@ def test_test_base_class_skipped(opts, generate_tests): ]), schema.Class("Derived", bases={"Base"}), ]) == { - "Derived/Derived.ql": ql.ClassTester(class_name="Derived"), - } + "Derived/Derived.ql": ql.ClassTester(class_name="Derived"), + } def test_test_final_class_skipped(opts, generate_tests): diff --git a/swift/codegen/test/test_schema.py b/swift/codegen/test/test_schema.py index 199b687dabe..59c58a16639 100644 --- a/swift/codegen/test/test_schema.py +++ b/swift/codegen/test/test_schema.py @@ -266,7 +266,6 @@ A: """) - def test_class_with_pragmas(load): ret = load(""" A: diff --git a/swift/codegen/test/test_trapgen.py b/swift/codegen/test/test_trapgen.py index 1241ce0b863..66491b66be1 100644 --- a/swift/codegen/test/test_trapgen.py +++ b/swift/codegen/test/test_trapgen.py @@ -122,8 +122,9 @@ def test_one_table_overridden_underscore_named_field(generate_traps): assert generate_traps([ dbscheme.Table(name="foos", columns=[dbscheme.Column("whatever_", "bar")]), ]) == [ - cpp.Trap("foos", name="Foos", fields=[cpp.Field("whatever", "bar")]), - ] + cpp.Trap("foos", name="Foos", fields=[cpp.Field("whatever", "bar")]), + ] + def test_one_table_no_tags(generate_tags): assert generate_tags([ diff --git a/swift/integration-tests/cross-references/test.py b/swift/integration-tests/cross-references/test.py index b4ddf3e66e0..19271030c19 100644 --- a/swift/integration-tests/cross-references/test.py +++ b/swift/integration-tests/cross-references/test.py @@ -4,4 +4,3 @@ run_codeql_database_create([ 'swift package clean', 'swift build' ], lang='swift') -