Load custom transformer on top of default ones (#480)

Author: bhirsz

docs/releasenotes/3.6.0.rst

@@ -26,8 +26,25 @@ although in vast majority of cases one extra run should suffice (and only in cas
 
 Example usage::
 
-    robotidy --reruns 3 --diff test.robot
+    > robotidy --reruns 3 --diff test.robot
 
 Note that if you enable it, it can double the execution time of Robotidy (if the file was modified, it will be
 transformed again to check if next transformation does not further modify the file). It should be not a problem because
 Robotidy is fast enough but report any issues with this feature.
+
+Load custom transformers together with defaults
+------------------------------------------------
+
+Previously Robotidy only supported importing custom transformers with ``--transform`` option. This option disables
+any other transformer not listed with ``--transform``. That's why if user would run following::
+
+    > robotidy --transform MyCustomClass.py test.robot
+
+It would disable all default transformers and only run MyCustomClass.
+This release introduces new option ``--load-transformer`` which imports custom transformers on top of the default ones::
+
+    > robotidy --load-transformer MyCustomClass.py test.robot
+
+It is also possible to pass transformer configuration either using this option or through ``--configure``::
+
+    > robotidy -c ExtClass1.py:param=value --load-transformer ExtClass2.py:param2=value test.robot

docs/source/configuration/configuring_transformers.rst

@@ -3,16 +3,26 @@
 Configuring Transformers
 ========================
 
-Transformers can be configured through two different options: ``--transform`` (``-t``) and ``--configure`` (``-c``). They share the same
-syntax for parameter names and values. The main difference is that ``--transform`` is also used to select what
-transformers will be used. For example::
+Transformers can be configured through three different options: ``--transform`` (``-t``), ``--load-transformer`` and
+``--configure`` (``-c``). They share the same syntax for parameter names and values.
+
+- ``--configure`` simply provides the configuration to the transformer,
+- ``--transform`` is used to include only transformers that have ``--transform`` option,
+- ``--load-transformer`` is used to load user transformers. Read more at :ref:`external-transformers`.
+
+For example::
 
     robotidy --transform NormalizeNewLines:test_case_lines=2 src
     robotidy --configure NormalizeNewLines:test_case_lines=2 src
+    robotidy --configure NormalizeNewLines:test_case_lines=1 --load-transformer MyCustomTransformer.py:param=value src
 
 With first command robotidy will run only ``NormalizeNewLines`` transformer and it will configure it with ``test_case_lines = 2``.
+
 Second command robotidy will run all of the transformers and will configure ``NormalizeNewLines`` with ``test_case_lines = 2``.
 
+Third command will run all of the transformers, configure ``NormalizeNewLines`` with ``test_case_lines = 1`` and
+import user transformer ``MyCustomTransformer`` with `param=value` configuration.
+
 You can also run all transformers except selected ones. For that you need to configure transformer you want to exclude
 with ``enabled`` parameter::
 

docs/source/external_transformers.rst

@@ -7,9 +7,14 @@ file with class to run external transformers with *robotidy*::
 
     robotidy --transform MyTransformers.YourCustomTransformer src
     robotidy --transform C:\transformers\YourCustomTransformer2.py src
+    robotidy --load-transformer C:\transformers\YourCustomTransformer2.py src
 
 External transformers can be configured in the same way internal transformers are configured - see :ref:`configuring-transformers`.
 
+You can use both ``--transform`` and ``--load-transformer`` options to load custom user transformer. The main difference
+is that ``--transform`` works like include and will only run transformers listed with ``--transform``. While ``--load-transformer``
+will run default transformers first and then user transformers.
+
 You can use the same syntax ``robotidy`` is using for developing internal transformers. The name of the file should
 be the same as name of the class containing your transformer. Your transformer should inherit from ``robot.api.parsing.ModelTransformer``
 parent class.
@@ -60,10 +65,6 @@ class:
 
 
     class ExternalTransformer(Transformer):
-        def __init__(self):
-            super().__init__()
+        pass
 
 ``Transformer`` also inherits from ``ModelTransformer`` but provides more utility methods (and better lint support).
-However because of how we are dynamically loading class arguments from cli/config we need to make a call to
-``super().__init__()`` even if our class don't have any arguments to set. If you're unsure what to use - use
-``ModelTransformer``.

robotidy/api.py

@@ -54,13 +54,17 @@ def get_formatting_config(config, kwargs):
 
 
 def get_robotidy(src: str, output: Optional[str], **kwargs):
+    def convert_transformers_config(param_name, config):
+        return [converter.convert(tr, None, None) for tr in config.get(param_name, ())]
+
     # TODO Refactor - Config should be read in one place both for API and CLI
     # TODO Remove kwargs usage - other SDKs are not using this feature
     config = files.find_and_read_config((src,))
     config = {k: str(v) if not isinstance(v, (list, dict)) else v for k, v in config.items()}
     converter = transformers.TransformType()
-    transformer_list = [converter.convert(tr, None, None) for tr in config.get("transform", ())]
-    configurations = [converter.convert(c, None, None) for c in config.get("configure", ())]
+    transformer_list = convert_transformers_config("transform", config)
+    custom_transformers = convert_transformers_config("load-transformers", config)
+    configurations = convert_transformers_config("configure", config)
     formatting_config = get_formatting_config(config, kwargs)
     exclude = config.get("exclude", None)
     extend_exclude = config.get("extend_exclude", None)
@@ -71,6 +75,7 @@ def get_robotidy(src: str, output: Optional[str], **kwargs):
     language = config.get("language", None)
     configuration = Config(
         transformers=transformer_list,
+        custom_transformers=custom_transformers,
         transformers_config=configurations,
         skip=global_skip,
         src=(),

robotidy/cli.py

@@ -23,6 +23,10 @@
         "name": "Run only selected transformers",
         "options": ["--transform"],
     },
+    {
+        "name": "Load custom transformers",
+        "options": ["--load-transformers"],
+    },
     {
         "name": "Work modes",
         "options": ["--overwrite", "--diff", "--check", "--force-order"],
@@ -165,7 +169,8 @@ def print_transformer_docs(transformer):
 
 @decorators.optional_rich
 def print_description(name: str, target_version: int):
-    transformers = load_transformers(None, {}, allow_disabled=True, target_version=target_version)
+    # TODO: --desc works only for default transformers, it should also print custom transformer desc
+    transformers = load_transformers([], [], {}, allow_disabled=True, target_version=target_version)
     transformer_by_names = {transformer.name: transformer for transformer in transformers}
     if name == "all":
         for transformer in transformers:
@@ -182,6 +187,7 @@ def print_description(name: str, target_version: int):
 
 def _load_external_transformers(
     transformers: List,
+    custom_transformers: List[Tuple[str, List]],
     transformers_from_config: List[Tuple[str, List]],
     transformer_config: List[Tuple[str, List]],
     target_version: int,
@@ -190,7 +196,7 @@ def _load_external_transformers(
     transformers_names = {transformer.name for transformer in transformers}
     transformer_config_converted = Config.convert_configure(transformer_config)
     transformers_from_conf = load_transformers(
-        transformers_from_config, transformer_config_converted, target_version=target_version
+        transformers_from_config, custom_transformers, transformer_config_converted, target_version=target_version
     )
     for transformer in transformers_from_conf:
         if transformer.name not in transformers_names:
@@ -201,6 +207,7 @@ def _load_external_transformers(
 @decorators.optional_rich
 def print_transformers_list(
     transformers_from_config: List[Tuple[str, List]],
+    custom_transformers: List[Tuple[str, List]],
     transformer_config: List[Tuple[str, List]],
     config: Config,
     target_version: int,
@@ -211,9 +218,11 @@ def print_transformers_list(
     table = Table(title="Transformers", header_style="bold red")
     table.add_column("Name", justify="left", no_wrap=True)
     table.add_column("Enabled")
-    transformers = load_transformers(None, {}, allow_disabled=True, target_version=target_version)
+    transformers = load_transformers([], [], {}, allow_disabled=True, target_version=target_version)
     transformers.extend(
-        _load_external_transformers(transformers, transformers_from_config, transformer_config, target_version)
+        _load_external_transformers(
+            transformers, custom_transformers, transformers_from_config, transformer_config, target_version
+        )
     )
 
     for transformer in transformers:
@@ -248,7 +257,15 @@ def print_transformers_list(
     type=TransformType(),
     multiple=True,
     metavar="TRANSFORMER_NAME",
-    help="Transform files from \[PATH(S)] with given transformer",
+    help="Transform files from [PATH(S)] with given transformer",
+)
+@click.option(
+    "--load-transformers",
+    "custom_transformers",
+    type=TransformType(),
+    multiple=True,
+    metavar="TRANSFORMER_NAME",
+    help="Load custom transformer from the path and run them after default ones.",
 )
 @click.option(
     "--configure",
@@ -474,6 +491,7 @@ def print_transformers_list(
 def cli(
     ctx: click.Context,
     transform: List[Tuple[str, List]],
+    custom_transformers: List[Tuple[str, List]],
     configure: List[Tuple[str, List]],
     src: Tuple[str, ...],
     exclude: Optional[Pattern],
@@ -566,10 +584,13 @@ def cli(
         end_line=endline,
         line_length=line_length,
     )
+    # array of some class, that holds info if it's forced, configure or custom
+    # or just name:args TODO
     config = Config(
         formatting=formatting,
         skip=skip_config,
         transformers=transform,
+        custom_transformers=custom_transformers,
         transformers_config=configure,
         src=src,
         exclude=exclude,
@@ -588,7 +609,7 @@ def cli(
     )
 
     if list_transformers:
-        print_transformers_list(transform, configure, config, target_version, list_transformers)
+        print_transformers_list(transform, custom_transformers, configure, config, target_version, list_transformers)
         sys.exit(0)
     if desc is not None:
         return_code = print_description(desc, target_version)

robotidy/config.py

@@ -63,6 +63,7 @@ def __init__(
         formatting: FormattingConfig,
         skip,
         transformers: List[Tuple[str, List]],
+        custom_transformers: List[Tuple[str, List]],
         transformers_config: List[Tuple[str, List]],
         src: Tuple[str, ...],
         exclude: Optional[Pattern],
@@ -92,11 +93,20 @@ def __init__(
         transformers_config = self.convert_configure(transformers_config)
         self.transformers = []
         self.transformers_lookup = dict()
-        self.load_transformers(transformers, transformers_config, force_order, target_version, skip)
+        self.load_transformers(
+            transformers, custom_transformers, transformers_config, force_order, target_version, skip
+        )
 
-    def load_transformers(self, transformers, transformers_config, force_order, target_version, skip):
+    def load_transformers(
+        self, transformers, custom_transformers, transformers_config, force_order, target_version, skip
+    ):
         transformers = load_transformers(
-            transformers, transformers_config, force_order=force_order, target_version=target_version, skip=skip
+            transformers,
+            custom_transformers,
+            transformers_config,
+            force_order=force_order,
+            target_version=target_version,
+            skip=skip,
         )
         for transformer in transformers:
             # inject global settings TODO: handle it better

robotidy/transformers/__init__.py

@@ -325,42 +325,56 @@ def assert_not_duplicated_transform(allowed_mapped, allowed_transformers):
 
 
 def load_transformers(
-    allowed_transformers,
+    selected_transformers,
+    custom_transformers,
     config,
     target_version,
     skip=None,
     allow_disabled=False,
     force_order=False,
     allow_version_mismatch=True,
 ):
-    """Dynamically load all classes from this file with attribute `name` defined in allowed_transformers"""
+    """Dynamically load all classes from this file with attribute `name` defined in selected_transformers"""
     loaded_transformers = []
-    allowed_mapped = {name: args for name, args in allowed_transformers} if allowed_transformers else {}
-    assert_not_duplicated_transform(allowed_mapped, allowed_transformers)
+    allowed_mapped = {name: args for name, args in selected_transformers}
+    custom_mapped = {name: args for name, args in custom_transformers}
+    assert_not_duplicated_transform(allowed_mapped, selected_transformers)
     validate_config(config, allowed_mapped)
     if not force_order:
         for name in TRANSFORMERS:
+            # load all default ones if allowed_mapped is not defined, or only those listed in allowed_mapped
             if not allowed_mapped or name in allowed_mapped:
                 args = get_args(name, allowed_mapped, config)
                 container = load_transformer(name, args, skip)
                 if container is None:
                     continue
                 enabled = getattr(container.instance, "ENABLED", True) or args.get("enabled", False)
-                if allowed_mapped or allow_disabled or enabled:
-                    overwritten = name in allowed_mapped or args.get("enabled", False)
-                    if can_run_in_robot_version(
-                        container.instance, overwritten=overwritten, target_version=target_version
-                    ):
-                        loaded_transformers.append(container)
-                    elif allow_version_mismatch and allow_disabled:
-                        setattr(container.instance, "ENABLED", False)
-                        container.enabled_by_default = False
-                        loaded_transformers.append(container)
+                if not (allowed_mapped or allow_disabled or enabled):
+                    continue
+                overwritten = name in allowed_mapped or args.get("enabled", False)
+                if can_run_in_robot_version(container.instance, overwritten=overwritten, target_version=target_version):
+                    loaded_transformers.append(container)
+                elif allow_version_mismatch and allow_disabled:
+                    setattr(container.instance, "ENABLED", False)
+                    container.enabled_by_default = False
+                    loaded_transformers.append(container)
     for name in allowed_mapped:
         if force_order or name not in TRANSFORMERS:
             args = get_args(name, allowed_mapped, config)
             container = load_transformer(name, args, skip)
-            if container is not None:
-                if can_run_in_robot_version(container.instance, overwritten=True, target_version=target_version):
-                    loaded_transformers.append(container)
+            if container is None:
+                continue
+            if can_run_in_robot_version(container.instance, overwritten=True, target_version=target_version):
+                loaded_transformers.append(container)
+    for name in custom_mapped:
+        args = get_args(name, custom_mapped, config)
+        container = load_transformer(name, args, skip)
+        if container is None:
+            continue
+        enabled = getattr(container.instance, "ENABLED", True) or args.get("enabled", False)
+        if not (allow_disabled or enabled):
+            continue
+        overwritten = args.get("enabled", False)
+        if can_run_in_robot_version(container.instance, overwritten=overwritten, target_version=target_version):
+            loaded_transformers.append(container)
     return loaded_transformers

tests/atest/transformers/ExternalTransformer/ExternalDisabledTransformer.py

@@ -0,0 +1,15 @@
+from robotidy.transformers import Transformer
+
+
+class ExternalDisabledTransformer(Transformer):
+    """
+    This transformer is disabled by default. If it is enabled, it replaces setting names to lowercase.
+    """
+
+    ENABLED = False
+
+    def visit_SectionHeader(self, node):  # noqa
+        if not node.name:
+            return node
+        node.data_tokens[0].value = node.data_tokens[0].value.lower()
+        return node

tests/atest/transformers/ExternalTransformer/expected/tests_lowercase.robot

@@ -0,0 +1,7 @@
+*** settings ***
+Library    KeywordLibrary
+
+
+*** test cases
+Test
+    Pass

tests/atest/transformers/ExternalTransformer/expected/tests_only_defaults.robot

@@ -0,0 +1,7 @@
+*** Settings ***
+Library     KeywordLibrary
+
+
+*** Test Cases ***
+Test
+    Pass