Robotidy should always use the same order (#143)

Author: bhirsz

CHANGES.md

@@ -11,11 +11,13 @@
 - New option ``--output`` option for saving transformed file to provided path instead of overwriting source file [#108](https://github.com/MarketSquare/robotframework-tidy/issues/108)
 - ``--desc`` now accepts ``all`` for printing out description of all transformers [#105](https://github.com/MarketSquare/robotframework-tidy/issues/105)
 - Robotidy will now suggest similar names for invalid transformer names used with ``--transform`` or ``--desc`` options  [#107](https://github.com/MarketSquare/robotframework-tidy/issues/107)
+- ``--list`` now prints transformers in alphabetical order [#141](https://github.com/MarketSquare/robotframework-tidy/issues/141)
 
 ### Fixes
 - Renamed short version of ``--lineseparator`` to ``-ls`` to avoid collision with ``--list\-l``
 - Description for disabled transformers can be now displayed & disabled transformers are in ``--list`` output [#114](https://github.com/MarketSquare/robotframework-tidy/issues/114)
 - Robotidy should now correctly load configuration files from path when using ``--config`` [#138](https://github.com/MarketSquare/robotframework-tidy/issues/138)
+- Transformers should always use the same order. If you need to use custom order, provide --force-order flag [#142](https://github.com/MarketSquare/robotframework-tidy/issues/142)
 
 ### Other
 - Removed ``'--describe-transformer`` and ``--list-transformers`` aliases for ``--list`` and ``--desc``

docs/source/transformers/index.rst

@@ -32,6 +32,11 @@ It will transform files according to internal order (in this example ``ReplaceRu
    robotidy --transform SplitTooLongLine src
    robotidy --transform ReplaceRunKeywordIf src
 
+You can also add ``--force-order`` flag to use order provided in cli:
+   robotidy --force-order --transform SplitTooLongLine --transform ReplaceRunKeywordIf src
+
+External transformers are used last. If you want to change this behaviour (for example run your custom transformer
+before default ones) you need to use ``--force-order`` flag.
 
 .. rubric:: List of transformers
 

robotidy/api.py

@@ -33,7 +33,8 @@ def __init__(self, src: str, output: Optional[str], **kwargs):
             formatting_config=formatting_config,
             verbose=False,
             check=False,
-            output=output
+            output=output,
+            force_order=False
         )
 
 

robotidy/app.py

@@ -27,7 +27,8 @@ def __init__(self,
                  formatting_config: GlobalFormattingConfig,
                  verbose: bool,
                  check: bool,
-                 output: Optional[Path]
+                 output: Optional[Path],
+                 force_order: bool
                  ):
         self.sources = self.get_paths(src)
         self.overwrite = overwrite

robotidy/cli.py

@@ -192,9 +192,12 @@ def print_transformers_list():
     click.echo('To see detailed docs run --desc <transformer_name> or --desc all. '
                'Transformers with (disabled) tag \nare executed only when selected explictly with --transform. '
                'Available transformers:\n')
+    transformer_names = []
     for transformer in transformers:
         disabled = ' (disabled)' if not getattr(transformer, 'ENABLED', True) else ''
-        click.echo(transformer.__class__.__name__ + disabled)
+        transformer_names.append(transformer.__class__.__name__ + disabled)
+    for name in sorted(transformer_names):
+        click.echo(name)
 
 
 @click.command(cls=RawHelp, help=HELP_MSG, epilog=EPILOG, context_settings=CONTEXT_SETTINGS)
@@ -332,6 +335,11 @@ def print_transformers_list():
     '--describe-transformer',
     default=None
 )
+@click.option(
+    '--force-order',
+    is_flag=True,
+    help='Transform files using transformers in order provided in cli'
+)
 @click.version_option(version=__version__, prog_name='robotidy')
 @click.pass_context
 def cli(
@@ -352,7 +360,8 @@ def cli(
         desc: Optional[str],
         output: Optional[Path],
         list_transformers: bool,
-        describe_transformer: Optional[str]
+        describe_transformer: Optional[str],
+        force_order: bool
 ):
     if list_transformers:
         print('--list-transformers is deprecated in 1.3.0. Use --list instead')
@@ -388,7 +397,8 @@ def cli(
         formatting_config=formatting_config,
         verbose=verbose,
         check=check,
-        output=output
+        output=output,
+        force_order=force_order
     )
     status = tidy.transform_files()
     ctx.exit(status)

robotidy/transformers/__init__.py

@@ -51,28 +51,40 @@ def import_transformer(name, args):
                                      f"Verify if correct name or configuration was provided.{similar}") from None
 
 
-def load_transformers(allowed_transformers, config, allow_disabled=False):
+def load_transformer(name, args, config):
+    # if we configure the same parameter for both --transform and --configure we need to overwrite it
+    # it is done by converting to dict and back to list in format of key=value
+    temp_args = {}
+    for arg in chain(args, config.get(name, ())):
+        param, value = arg.split('=', maxsplit=1)
+        temp_args[param] = value
+    args = [f'{key}={value}' for key, value in temp_args.items()]
+    import_name = f'robotidy.transformers.{name}' if name in TRANSFORMERS else name
+    return import_transformer(import_name, args)
+
+
+def load_transformers(allowed_transformers, config, allow_disabled=False, force_order=False):
     """ Dynamically load all classes from this file with attribute `name` defined in allowed_transformers """
     loaded_transformers = []
-    if allowed_transformers:
-        for name, args in allowed_transformers:
-            # if we are configure the same param from both --transform and --configure we need to overwrite it
-            # it is done by converting to dict and back to list in format of key=value
-            temp_args = {}
-            for arg in chain(args, config.get(name, ())):
-                param, value = arg.split('=', maxsplit=1)
-                temp_args[param] = value
-            args = [f'{key}={value}' for key, value in temp_args.items()]
-            import_name = f'robotidy.transformers.{name}' if name in TRANSFORMERS else name
-            imported_class = import_transformer(import_name, args)
-            if imported_class is None:
-                return []
-            loaded_transformers.append(imported_class)
-    else:
+    allowed_mapped = {name: args for name, args in allowed_transformers} if allowed_transformers else {}
+    if not force_order:
         for name in TRANSFORMERS:
-            imported_class = import_transformer(f'robotidy.transformers.{name}', config.get(name, ()))
+            if allowed_mapped:
+                if name in allowed_mapped:
+                    imported_class = load_transformer(name, allowed_mapped[name], config)
+                    if imported_class is None:
+                        return []
+                    loaded_transformers.append(imported_class)
+            else:
+                imported_class = import_transformer(f'robotidy.transformers.{name}', config.get(name, ()))
+                if imported_class is None:
+                    return []
+                if allow_disabled or getattr(imported_class, 'ENABLED', True):
+                    loaded_transformers.append(imported_class)
+    for name in allowed_mapped:
+        if force_order or name not in TRANSFORMERS:
+            imported_class = load_transformer(name, allowed_mapped[name], config)
             if imported_class is None:
                 return []
-            if allow_disabled or getattr(imported_class, 'ENABLED', True):
-                loaded_transformers.append(imported_class)
+            loaded_transformers.append(imported_class)
     return loaded_transformers

tests/utest/test_cli.py

@@ -43,13 +43,26 @@ def test_not_existing_transformer(self, name, similar):
         result = run_tidy(args, exit_code=1)
         assert expected_output in str(result.exception)
 
+    def test_transformer_order(self):
+        order_1 = ['NormalizeSeparators', 'OrderSettings']
+        order_2 = ['OrderSettings', 'NormalizeSeparators']
+        transformers_1 = load_transformers([(transf, []) for transf in order_1], {})
+        transformers_2 = load_transformers([(transf, []) for transf in order_2], {})
+        assert all(t1.__class__.__name__ == t2.__class__.__name__ for t1, t2 in zip(transformers_1, transformers_2))
+
+    def test_transformer_force_order(self):
+        # default_order = ['NormalizeSeparators', 'OrderSettings']
+        custom_order = ['OrderSettings', 'NormalizeSeparators']
+        transformers = load_transformers([(transf, []) for transf in custom_order], {}, force_order=True)
+        assert all(t1.__class__.__name__ == t2 for t1, t2 in zip(transformers, custom_order))
+
     # TODO: raise exception if kwarg does not match
     # def test_not_existing_configurable(self):
     #     expected_output = "Usage: cli [OPTIONS] [PATH(S)]\n\n" \
     #                       "Error: Invalid configurable name: 'missing_configurable' for transformer: " \
     #                       "'DiscardEmptySections'\n"
     #
-    #     args = '--transform DiscardEmptySections:allow_only_commentss=True'.split()
+    #     args = '--transform DiscardEmptySections:allow_only_commentss=True -'.split()
     #     result = run_tidy(args, exit_code=2)
     #     assert expected_output == result.output
 
@@ -189,6 +202,7 @@ def test_list_transformers(self, flag):
                in result.output
         assert 'ReplaceRunKeywordIf\n' in result.output
         assert 'SmartSortKeywords (disabled)\n' in result.output  # this transformer is disabled by default
+        assert 'Available transformers:\n\nAlignSettingsSection\n' in result.output  # assert order
 
     @pytest.mark.parametrize('flag', ['--desc', '-d'])
     @pytest.mark.parametrize('name, expected_doc', [