Repository: tefra/xsdata
Pull request #1043 (2024-05-19 12:44:57)

Run ruff once
## 📒 Description


Running ruff for each generated file is very slow,  let's address that. This unfortunately takes out the print mode, but I don't think anyone is using it anyway. If you do, I am sorry :)


Resolves #xxxx

## 🔗 What I've Done

> Write a description of the steps taken to resolve the issue

## 💬 Comments

> A place to write any comments to the reviewer.

## 🛫 Checklist

- [ ] Updated docs
- [ ] Added unit-tests
- [ ] [Sample tests](https://github.com/tefra/xsdata-samples) pass
- [ ] [W3C tests](https://github.com/tefra/xsdata-w3c-tests) pass



Patch:
diff --git a/xsdata/cli.py b/xsdata/cli.py
index 72025cba..a34a73d9 100644
--- a/xsdata/cli.py
+++ b/xsdata/cli.py
@@ -65,7 +65,6 @@ def cli(ctx: click.Context, **kwargs: Any):
 
 @cli.command("init-config")
 @click.argument("output", type=click.Path(), default=".xsdata.xml")
-@click.option("-pp", "--print", is_flag=True, default=False, help="Print output")
 def init_config(**kwargs: Any):
     """Create or update a configuration file."""
     file_path = Path(kwargs["output"])
@@ -76,11 +75,8 @@ def init_config(**kwargs: Any):
         logger.info("Initializing configuration file %s", kwargs["output"])
         config = GeneratorConfig.create()
 
-    if kwargs["print"]:
-        config.write(sys.stdout, config)
-    else:
-        with file_path.open("w") as fp:
-            config.write(fp, config)
+    with file_path.open("w") as fp:
+        config.write(fp, config)
 
     handler.emit_warnings()
 
@@ -112,7 +108,6 @@ def download(source: str, output: str):
     help="Search files recursively in the source directory",
 )
 @click.option("-c", "--config", default=".xsdata.xml", help="Project configuration")
-@click.option("-pp", "--print", is_flag=True, default=False, help="Print output")
 @click.option("--cache", is_flag=True, default=False, help="Cache sources loading")
 @click.option("--debug", is_flag=True, default=False, help="Show debug messages")
 @model_options(GeneratorOutput)
@@ -127,7 +122,6 @@ def generate(**kwargs: Any):
         logger.setLevel(logging.DEBUG)
 
     source = kwargs.pop("source")
-    stdout = kwargs.pop("print")
     cache = kwargs.pop("cache")
     recursive = kwargs.pop("recursive")
     config_file = Path(kwargs.pop("config")).resolve()
@@ -136,7 +130,7 @@ def generate(**kwargs: Any):
     config = GeneratorConfig.read(config_file)
     config.output.update(**params)
 
-    transformer = ResourceTransformer(config=config, print=stdout)
+    transformer = ResourceTransformer(config=config)
     uris = sorted(resolve_source(source, recursive=recursive))
     transformer.process(uris, cache=cache)
 
diff --git a/xsdata/codegen/transformer.py b/xsdata/codegen/transformer.py
index b01eabc5..fb1e5999 100644
--- a/xsdata/codegen/transformer.py
+++ b/xsdata/codegen/transformer.py
@@ -100,7 +100,6 @@ class ResourceTransformer:
     Supports xsd, wsdl, dtd, xml and json documents.
 
     Args:
-        print: Print to stdout the generated output
         config: Generator configuration
 
     Attributes:
@@ -109,10 +108,9 @@ class ResourceTransformer:
         preloaded: A uri/content map used as cache
     """
 
-    __slots__ = ("print", "config", "classes", "processed", "preloaded")
+    __slots__ = ("config", "classes", "processed", "preloaded")
 
-    def __init__(self, print: bool, config: GeneratorConfig):
-        self.print = print
+    def __init__(self, config: GeneratorConfig):
         self.config = config
         self.classes: List[Class] = []
         self.processed: List[str] = []
@@ -287,10 +285,7 @@ class ResourceTransformer:
 
             writer = CodeWriter.from_config(self.config)
             with stopwatch(CodeWriter.__name__):
-                if self.print:
-                    writer.print(classes)
-                else:
-                    writer.write(classes)
+                writer.write(classes)
 
     def convert_schema(self, schema: Schema):
         """Convert a schema instance to codegen classes.
diff --git a/xsdata/codegen/writer.py b/xsdata/codegen/writer.py
index df51e77f..198d5ed1 100644
--- a/xsdata/codegen/writer.py
+++ b/xsdata/codegen/writer.py
@@ -47,19 +47,6 @@ class CodeWriter:
                 result.path.parent.mkdir(parents=True, exist_ok=True)
                 result.path.write_text(src_code, encoding="utf-8")
 
-    def print(self, classes: List[Class]):
-        """Print the generated code for the given classes.
-
-        Args:
-            classes: A list of class instances
-        """
-        self.generator.normalize_packages(classes)
-        header = self.generator.render_header()
-        for result in self.generator.render(classes):
-            if result.source.strip():
-                src_code = header + result.source
-                print(src_code, end="")
-
     @classmethod
     def from_config(cls, config: GeneratorConfig) -> "CodeWriter":
         """Instance the code writer from the generator configuration instance.
diff --git a/xsdata/formats/dataclass/generator.py b/xsdata/formats/dataclass/generator.py
index 497e1399..1b45d3d1 100644
--- a/xsdata/formats/dataclass/generator.py
+++ b/xsdata/formats/dataclass/generator.py
@@ -61,12 +61,13 @@ class DataclassGenerator(AbstractGenerator):
         """
         packages = {obj.qname: obj.target_module for obj in classes}
         resolver = DependenciesResolver(registry=packages)
-
+        file_names = []
         # Generate packages
         for path, cluster in self.group_by_package(classes).items():
             module = ".".join(path.relative_to(Path.cwd()).parts)
             package_path = path.joinpath("__init__.py")
-            src_code = self.render_package(cluster, module, package_path)
+            src_code = self.render_package(cluster, module)
+            file_names.append(str(package_path))
 
             yield GeneratorResult(
                 path=package_path,
@@ -78,7 +79,8 @@ class DataclassGenerator(AbstractGenerator):
         # Generate modules
         for path, cluster in self.group_by_module(classes).items():
             module_path = path.with_suffix(".py")
-            src_code = self.render_module(resolver, cluster, module_path)
+            src_code = self.render_module(resolver, cluster)
+            file_names.append(str(module_path))
 
             yield GeneratorResult(
                 path=module_path,
@@ -86,6 +88,7 @@ class DataclassGenerator(AbstractGenerator):
                 source=src_code,
             )
 
+        self.ruff_code(file_names)
         self.validate_imports()
 
     def validate_imports(self):
@@ -109,13 +112,12 @@ class DataclassGenerator(AbstractGenerator):
         package = self.config.output.package
         import_package(self.package_name(package))
 
-    def render_package(self, classes: List[Class], module: str, filename: Path) -> str:
+    def render_package(self, classes: List[Class], module: str) -> str:
         """Render the package for the given classes.
 
         Args:
             classes: A list of class instances
             module: The target dot notation path
-            filename: The package path
 
         Returns:
             The rendered package output.
@@ -126,24 +128,21 @@ class DataclassGenerator(AbstractGenerator):
         ]
         DependenciesResolver.resolve_conflicts(imports, set())
 
-        output = self.env.get_template(self.package_template).render(
+        return self.env.get_template(self.package_template).render(
             imports=imports,
             module=module,
         )
-        return self.ruff_code(output, filename)
 
     def render_module(
         self,
         resolver: DependenciesResolver,
         classes: List[Class],
-        filename: Path,
     ) -> str:
         """Render the module for the given classes.
 
         Args:
             resolver: The dependencies resolver
             classes: A list of class instances
-            filename: The module path
 
         Returns:
             The rendered module output.
@@ -159,7 +158,7 @@ class DataclassGenerator(AbstractGenerator):
         output = self.render_classes(classes, module_namespace)
         module = classes[0].target_module
 
-        result = self.env.get_template(self.module_template).render(
+        return self.env.get_template(self.module_template).render(
             output=output,
             classes=classes,
             module=module,
@@ -167,8 +166,6 @@ class DataclassGenerator(AbstractGenerator):
             namespace=module_namespace,
         )
 
-        return self.ruff_code(result, filename)
-
     def render_classes(
         self,
         classes: List[Class],
@@ -236,30 +233,23 @@ class DataclassGenerator(AbstractGenerator):
         """Initialize the filters instance by the generator configuration."""
         return Filters(config)
 
-    def ruff_code(self, src_code: str, file_path: Path) -> str:
-        """Run ruff format on the src code.
+    def ruff_code(self, file_names: List[str]):
+        """Run ruff lint and format on a list of file names.
 
         Args:
-            src_code: The output source code
-            file_path: The file path the source code will be written to
-
-        Returns:
-            The formatted output source code
+            file_names: A list of files to format and check
         """
         commands = [
             [
                 "ruff",
                 "format",
-                "--stdin-filename",
-                str(file_path),
                 "--line-length",
                 str(self.config.output.max_line_length),
+                *file_names,
             ],
             [
                 "ruff",
                 "checks",
-                "--stdin-filename",
-                str(file_path),
                 "--line-length",
                 str(self.config.output.max_line_length),
                 "--config",
@@ -267,40 +257,16 @@ class DataclassGenerator(AbstractGenerator):
                 "--fix",
                 "--unsafe-fixes",
                 "--exit-zero",
+                *file_names,
             ],
         ]
         try:
-            src_code_encoded = src_code.encode()
             for command in commands:
-                result = subprocess.run(
+                subprocess.run(
                     command,
-                    input=src_code_encoded,
                     capture_output=True,
                     check=True,
                 )
-                src_code_encoded = result.stdout
-
-            return src_code_encoded.decode()
         except subprocess.CalledProcessError as e:
             details = e.stderr.decode().replace("error: ", "").strip()
-            source = self.code_excerpt(details, src_code_encoded.decode())
-            raise CodegenError("Ruff failed", details=details, source=source)
-
-    @classmethod
-    def code_excerpt(cls, details: str, src_code: str) -> str:
-        """Extract source code excerpt from the error details message."""
-        match = re.search(r"(\d+):(\d+)", details)
-        if match:
-            line_number = int(match.group(1)) - 1
-            lines = src_code.split("\n")
-            start = max(0, line_number - 4)
-            end = min(len(lines), line_number + 4)
-
-            excerpt = ["\n"]
-            for index in range(start, end):
-                prepend = "--->" if index == line_number else "   "
-                excerpt.append(f"{prepend}{lines[index]}")
-
-            return "\n".join(excerpt)
-
-        return "NA"
+            raise CodegenError(f"Ruff failed\n{details}")


Test patch:
diff --git a/tests/codegen/test_transformer.py b/tests/codegen/test_transformer.py
index 84f2b775..ff01c940 100644
--- a/tests/codegen/test_transformer.py
+++ b/tests/codegen/test_transformer.py
@@ -30,7 +30,7 @@ from xsdata.utils.testing import ClassFactory, DtdFactory, FactoryTestCase
 class ResourceTransformerTests(FactoryTestCase):
     def setUp(self):
         config = GeneratorConfig()
-        self.transformer = ResourceTransformer(print=True, config=config)
+        self.transformer = ResourceTransformer(config=config)
         super().setUp()
 
     @mock.patch.object(ResourceTransformer, "process_classes")
@@ -251,35 +251,10 @@ class ResourceTransformerTests(FactoryTestCase):
             ]
         )
 
-    @mock.patch("xsdata.codegen.transformer.logger.info")
-    @mock.patch.object(CodeWriter, "print")
-    @mock.patch.object(ResourceTransformer, "analyze_classes")
-    def test_process_classes_with_print_true(
-        self,
-        mock_analyze_classes,
-        mock_writer_print,
-        mock_logger_into,
-    ):
-        schema_classes = ClassFactory.list(3)
-        analyzer_classes = ClassFactory.list(2)
-        mock_analyze_classes.return_value = analyzer_classes
-
-        self.transformer.classes = schema_classes
-        self.transformer.process_classes()
-
-        mock_analyze_classes.assert_called_once_with(schema_classes)
-        mock_writer_print.assert_called_once_with(analyzer_classes)
-        mock_logger_into.assert_has_calls(
-            [
-                mock.call("Analyzer input: %d main and %d inner classes", 3, 0),
-                mock.call("Analyzer output: %d main and %d inner classes", 2, 0),
-            ]
-        )
-
     @mock.patch("xsdata.codegen.transformer.logger.info")
     @mock.patch.object(CodeWriter, "write")
     @mock.patch.object(ResourceTransformer, "analyze_classes")
-    def test_process_classes_with_print_false(
+    def test_process_classes(
         self,
         mock_analyze_classes,
         mock_writer_write,
@@ -289,7 +264,6 @@ class ResourceTransformerTests(FactoryTestCase):
         analyzer_classes = ClassFactory.list(2)
         mock_analyze_classes.return_value = analyzer_classes
 
-        self.transformer.print = False
         self.transformer.classes = schema_classes
         self.transformer.process_classes()
 
diff --git a/tests/codegen/test_writer.py b/tests/codegen/test_writer.py
index 950ea631..82ba694d 100644
--- a/tests/codegen/test_writer.py
+++ b/tests/codegen/test_writer.py
@@ -42,32 +42,6 @@ class CodeWriterTests(FactoryTestCase):
             self.assertFalse(Path(f"{tmpdir}/c.py").exists())
             mock_normalize_packages.assert_called_once_with(classes)
 
-    @mock.patch.object(NoneGenerator, "render_header")
-    @mock.patch.object(NoneGenerator, "render")
-    @mock.patch.object(NoneGenerator, "normalize_packages")
-    @mock.patch("builtins.print")
-    def test_print(
-        self,
-        mock_print,
-        mock_normalize_packages,
-        mock_render,
-        mock_render_header,
-    ):
-        classes = ClassFactory.list(2)
-        mock_render.return_value = [
-            GeneratorResult(Path("foo/a.py"), "file", "aAa"),
-            GeneratorResult(Path("bar/b.py"), "file", "bBb"),
-            GeneratorResult(Path("c.py"), "file", ""),
-        ]
-        mock_render_header.return_value = "# H\n"
-
-        self.writer.print(classes)
-
-        mock_normalize_packages.assert_called_once_with(classes)
-        mock_print.assert_has_calls(
-            [mock.call("# H\naAa", end=""), mock.call("# H\nbBb", end="")]
-        )
-
     def test_from_config(self):
         CodeWriter.unregister_generator("dataclasses")
         config = GeneratorConfig()
diff --git a/tests/formats/dataclass/test_generator.py b/tests/formats/dataclass/test_generator.py
index c2147113..0e161040 100644
--- a/tests/formats/dataclass/test_generator.py
+++ b/tests/formats/dataclass/test_generator.py
@@ -1,4 +1,5 @@
 import random
+import tempfile
 from pathlib import Path
 from unittest import mock
 
@@ -16,11 +17,16 @@ class DataclassGeneratorTests(FactoryTestCase):
         config = GeneratorConfig()
         self.generator = DataclassGenerator(config)
 
+    @mock.patch.object(DataclassGenerator, "ruff_code")
     @mock.patch.object(DataclassGenerator, "validate_imports")
     @mock.patch.object(DataclassGenerator, "render_package")
     @mock.patch.object(DataclassGenerator, "render_module")
     def test_render(
-        self, mock_render_module, mock_render_package, mock_validate_imports
+        self,
+        mock_render_module,
+        mock_render_package,
+        mock_validate_imports,
+        mock_ruff_code,
     ):
         classes = [
             ClassFactory.create(package="foo.bar", module="tests"),
@@ -49,16 +55,12 @@ class DataclassGeneratorTests(FactoryTestCase):
         self.assertEqual(expected, actual)
         mock_render_package.assert_has_calls(
             [
-                mock.call([classes[0]], "foo.bar", cwd.joinpath("foo/bar/__init__.py")),
-                mock.call([classes[1]], "bar.foo", cwd.joinpath("bar/foo/__init__.py")),
-                mock.call(
-                    [classes[2]], "thug.life", cwd.joinpath("thug/life/__init__.py")
-                ),
+                mock.call([classes[0]], "foo.bar"),
+                mock.call([classes[1]], "bar.foo"),
+                mock.call([classes[2]], "thug.life"),
             ]
         )
-        mock_render_module.assert_has_calls(
-            [mock.call(mock.ANY, [x], mock.ANY) for x in classes]
-        )
+        mock_render_module.assert_has_calls([mock.call(mock.ANY, [x]) for x in classes])
         mock_validate_imports.assert_called_once()
 
     def test_render_package(self):
@@ -71,13 +73,11 @@ class DataclassGeneratorTests(FactoryTestCase):
 
         random.shuffle(classes)
 
-        actual = self.generator.render_package(classes, "foo.tests", Path.cwd())
+        actual = self.generator.render_package(classes, "foo.tests")
         expected = (
             "from foo.bar import A as BarA\n"
             "from foo.tests import (\n"
             "    A as TestsA,\n"
-            ")\n"
-            "from foo.tests import (\n"
             "    B,\n"
             "    C,\n"
             ")\n"
@@ -87,7 +87,7 @@ class DataclassGeneratorTests(FactoryTestCase):
             '    "TestsA",\n'
             '    "B",\n'
             '    "C",\n'
-            "]\n"
+            "]"
         )
         self.assertEqual(expected, actual)
 
@@ -102,7 +102,7 @@ class DataclassGeneratorTests(FactoryTestCase):
 
         resolver = DependenciesResolver({})
 
-        actual = self.generator.render_module(resolver, classes, Path.cwd())
+        actual = self.generator.render_module(resolver, classes)
         expected = (
             "from dataclasses import dataclass, field\n"
             "from enum import Enum\n"
@@ -118,18 +118,14 @@ class DataclassGeneratorTests(FactoryTestCase):
             "    :cvar ATTR_B: I am a member\n"
             "    :cvar ATTR_C:\n"
             '    """\n'
-            "\n"
             "    ATTR_B = None\n"
             "    ATTR_C = None\n"
-            "\n"
-            "\n"
             "@dataclass\n"
             "class ClassC:\n"
             '    """\n'
             "    :ivar attr_d: I am a field\n"
             "    :ivar attr_e:\n"
             '    """\n'
-            "\n"
             "    class Meta:\n"
             '        name = "class_C"\n'
             "\n"
@@ -138,20 +134,18 @@ class DataclassGeneratorTests(FactoryTestCase):
             "        metadata={\n"
             '            "name": "attr_D",\n'
             '            "type": "Element",\n'
-            "        },\n"
+            "        }\n"
             "    )\n"
             "    attr_e: Optional[str] = field(\n"
             "        default=None,\n"
             "        metadata={\n"
             '            "name": "attr_E",\n'
             '            "type": "Element",\n'
-            "        },\n"
+            "        }\n"
             "    )\n"
-            "\n"
-            "\n"
             "class ClassD:\n"
             '    attr_f = "None"\n'
-            '    attr_g = "None"\n'
+            '    attr_g = "None"'
         )
 
         self.assertEqual(expected, actual)
@@ -163,7 +157,7 @@ class DataclassGeneratorTests(FactoryTestCase):
         ]
         resolver = DependenciesResolver({})
 
-        actual = self.generator.render_module(resolver, classes, Path.cwd())
+        actual = self.generator.render_module(resolver, classes)
         expected = (
             "from dataclasses import dataclass, field\n"
             "from typing import Optional\n"
@@ -180,10 +174,8 @@ class DataclassGeneratorTests(FactoryTestCase):
             "        metadata={\n"
             '            "name": "attr_C",\n'
             '            "type": "Element",\n'
-            "        },\n"
+            "        }\n"
             "    )\n"
-            "\n"
-            "\n"
             "@dataclass\n"
             "class Bar:\n"
             "    class Meta:\n"
@@ -195,8 +187,8 @@ class DataclassGeneratorTests(FactoryTestCase):
             "        metadata={\n"
             '            "name": "attr_B",\n'
             '            "type": "Element",\n'
-            "        },\n"
-            "    )\n"
+            "        }\n"
+            "    )"
         )
 
         self.assertEqual(expected, actual)
@@ -229,26 +221,10 @@ class DataclassGeneratorTests(FactoryTestCase):
             "    bar: str\n"
             "    thug: str"
         )
-        file_path = Path(__file__)
-
-        self.generator.config.output.max_line_length = 55
-        with self.assertRaises(CodegenError) as cm:
-            self.generator.ruff_code(src_code, file_path)
 
-        expected = (
-            "\n"
-            "\n"
-            "   class AlternativeText:\n"
-            "       class Meta:\n"
-            '           namespace = "xsdata"\n'
-            "   \n"
-            "--->    foo: Optional[Union[]] = field(\n"
-            "              init=False,\n"
-            '              metadata={"type": "Ignore"}\n'
-            "       )"
-        )
-        self.assertEqual(expected, cm.exception.meta.get("source"))
+        with tempfile.NamedTemporaryFile(delete=True, suffix=".py") as temp_file:
+            temp_file.write(src_code.encode())
+            temp_file.seek(0)
 
-    def test_code_excerpt_with_no_line_number(self):
-        actual = self.generator.code_excerpt("foobar", "")
-        self.assertEqual("NA", actual)
+            with self.assertRaises(CodegenError):
+                self.generator.ruff_code([temp_file.name])
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 9bfb269c..a8326e98 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -42,7 +42,6 @@ class CliTests(TestCase):
         config = mock_init.call_args[1]["config"]
 
         self.assertIsNone(result.exception)
-        self.assertFalse(mock_init.call_args[1]["print"])
         self.assertEqual("foo", config.output.package)
         self.assertEqual("dataclasses", config.output.format.value)
         self.assertFalse(config.output.relative_imports)
@@ -78,7 +77,6 @@ class CliTests(TestCase):
         config = mock_init.call_args[1]["config"]
 
         self.assertIsNone(result.exception)
-        self.assertFalse(mock_init.call_args[1]["print"])
         self.assertEqual("foo.bar", config.output.package)
         self.assertEqual("dataclasses", config.output.format.value)
         self.assertFalse(config.output.format.eq)
@@ -86,16 +84,6 @@ class CliTests(TestCase):
         self.assertEqual([source.as_uri()], mock_process.call_args[0][0])
         file_path.unlink()
 
-    @mock.patch.object(ResourceTransformer, "process")
-    @mock.patch.object(ResourceTransformer, "__init__", return_value=None)
-    def test_generate_with_print_mode(self, mock_init, mock_process):
-        source = fixtures_dir.joinpath("defxmlschema/chapter03.xsd")
-        result = self.runner.invoke(cli, [str(source), "--package", "foo", "--print"])
-
-        self.assertIsNone(result.exception)
-        self.assertEqual([source.as_uri()], mock_process.call_args[0][0])
-        self.assertTrue(mock_init.call_args[1]["print"])
-
     @mock.patch.object(ResourceTransformer, "process")
     @mock.patch.object(ResourceTransformer, "__init__", return_value=None)
     def test_generate_with_debug_mode(self, *args):
@@ -151,12 +139,6 @@ class CliTests(TestCase):
         )
         output_path.unlink()
 
-    def test_init_config_with_print_mode(self):
-        result = self.runner.invoke(cli, ["init-config", "--print"])
-
-        self.assertIsNone(result.exception)
-        self.assertIn('<Config xmlns="http://pypi.org/project/xsdata"', result.output)
-
     @mock.patch.object(Downloader, "wget")
     @mock.patch.object(Downloader, "__init__", return_value=None)
     def test_download(self, mock_init, mock_wget):