Merge pull request robotframework#3433 from robotframework/tidy

yanne · web-flow · commit 1e131fcbc3c3 · 2020-01-10T15:00:18.000+02:00
Tidy
diff --git a/atest/robot/tidy/format_documentation.robot b/atest/robot/tidy/format_documentation.robot
@@ -5,9 +5,4 @@ Suite Teardown    Remove Directory     ${TEMP}    recursive=True
 
 *** Test Cases ***
 Documentation in text file
-    Verify documentation formatting    robot
-
-*** Keywords ***
-Verify documentation formatting
-    [Arguments]    ${format}
-    Run tidy and check result    --format=${format}    documentation.robot    expected=documentation_expected.${format}
+    Run tidy and check result    ${EMPTY}  documentation.robot    expected=documentation_expected.robot
diff --git a/atest/robot/tidy/tidy.robot b/atest/robot/tidy/tidy.robot
@@ -8,23 +8,24 @@ Tidying single test case file
     [Documentation]   Test tidying to different formats
     [Template]    Run tidy with golden file and check result
     ${EMPTY}            golden.robot
-    --usepipes -f txt   golden_pipes.robot
-    --for robot         golden.robot
+    --usepipes          golden_pipes.robot
+    # TODO: fix the commented test, split this test into smaller pieces
+    #  ${EMPTY}          golden.robot    input=pipes-input.robot
+    --usepipes          golden_pipes.robot    input=pipes-input.robot
     --spacecount 2      golden_two_spaces.robot
 
 Tidying single resource file
     [Template]    Run tidy with golden resource file and check result
     ${EMPTY}            golden_resource.robot
     -p                  golden_pipes_resource.robot
-    --FOR ROBOT         golden_resource.robot
 
 Tidying single init file
     Run tidy and check result    input=__init__.robot
     File Should Exist    ${OUTFILE}
 
 Tidying single file without output file prints output to console
     [Documentation]    Depending on console encoding, non-ASCII characters may not be shown correctly.
-    ${stdout} =    Run tidy    input=golden.robot    output=None
+    ${stdout} =    Run tidy    input=golden-input.robot    output=None
     Compare tidy results    ${stdout}    golden.robot    \\s+Log Many\\s+Non-ASCII:.*\\s+\\$\\{CURDIR\\}
     File Should Not Exist    ${OUTFILE}
 
@@ -35,7 +36,7 @@ Default format is got from output file
 Tidying directory
     [Setup]    Copy Directory    ${DATA}/tests    ${TEMP}/tests
     ${result_before}=    Run Tests    sources=${DATA}/tests
-    Run Tidy    --recursive --format robot    ${TEMP}/tests
+    Run Tidy    --recursive    ${TEMP}/tests
     Check file count    ${TEMP}/tests    *.robot    2
     Check file count    ${TEMP}/tests/sub    *.robot    1
     Check file count    ${TEMP}/tests    *.txt    0
diff --git a/atest/robot/tidy/tidy_resource.robot b/atest/robot/tidy/tidy_resource.robot
@@ -9,8 +9,8 @@ ${OUTFILE}        ${TEMP}${/}tidy-test-file.robot
 
 *** Keywords ***
 Run tidy with golden file and check result
-    [Arguments]    ${options}    ${expected}
-    ${output} =    Run tidy and check result    ${options}    golden-input.robot    expected=${expected}
+    [Arguments]    ${options}    ${expected}    ${input}=golden-input.robot
+    ${output} =    Run tidy and check result    ${options}    ${input}    expected=${expected}
     [Return]    ${output}
 
 Run tidy with golden resource file and check result
diff --git a/atest/testdata/tidy/for_loops_expected.robot b/atest/testdata/tidy/for_loops_expected.robot
@@ -4,6 +4,12 @@ For loop in test
         Log    ${x}
     END
 
+Missing END
+    FOR    ${x}    IN    foo    bar
+        Log    ${x}
+        Keyword
+    END
+
 Old for loop in test
     FOR    ${x}    IN    foo    bar
         Log    ${x}
diff --git a/atest/testdata/tidy/for_loops_input.robot b/atest/testdata/tidy/for_loops_input.robot
@@ -4,6 +4,11 @@ For loop in test
         Log    ${x}
     END
 
+Missing END
+    FOR    ${x}    IN    foo    bar
+        Log    ${x}
+    Keyword
+
 Old for loop in test
     :FOR    ${x}    IN    foo    bar
     \    Log    ${x}
diff --git a/atest/testdata/tidy/pipes-input.robot b/atest/testdata/tidy/pipes-input.robot
@@ -0,0 +1,44 @@
+| This data is ignored at runtime but should be preserved by Tidy. |
+
+| *** Settings *** |
+| Library        | MyLibrary | argument | WITH NAME | My Alias | # My library comment |
+| Variables      | MyVariables | args | args 2 | args 3 | args 4 | args 5 | args 6 |
+| ...            | args 7 | args 8 | args 9 | args 10 | args 11 | args 12 |
+| Resource       | resource.robot |
+|
+| *** Variables *** |
+| # standalone   | comment |
+| ${VALID}       | Value |
+| MyVar          | val1 | val2 | val3 | val4 | val5 | val6 | val7 |
+| ...            | val8 | val9 | val10 | # var comment |
+| # standalone   |
+
+| *** Test Cases *** |
+| My Test Case |
+|    | [Documentation] | This is a documentation |
+|    | ... | in two lines |
+|    | My TC Step 1 | my step arg | # step 1 comment |
+|    | My TC Step 2 | my step 2 arg | second \ arg | # step 2 comment |
+|    | ... | third arg split to own row |
+|    | ... | fourth and | fifth as well | # comment |
+|    | [Teardown] | 1 minute | args |
+
+| Another Test |
+|    | Log Many | Non-ASCII: ääöö§§ | ${CURDIR} |
+
+| *** Keyword *** |
+| My Keyword |
+|    | [Documentation] | Documentation | # Comment for doc |
+|    | [Tags] | keyword | tags |
+|    | # Comment row |
+|    | # Comment row 2 |
+|    | My Step 1 | args | args 2 | args 3 | args 4 | args 5 | args 6 |
+|    | ... | args 7 | args 8 | args 9 | # step 1 comment |
+|    | FOR | ${param1} | ${param2} | IN | ${data 1} | ${data 2} | ${data 3} |
+|    | ... | ${data 4} | ${data 5} | ${data 6} | # FOR comment |
+|    |    | Loop Step | args | args 2 | args 3 | args 4 | args 5 |
+|    |    | ... | args 6 | args 7 | args 8 | args 9 | # loop step comment |
+|    |    | Loop Step 2 |
+|    | END |
+|    | My Step 2 | my step 2 arg | second arg | # step 2 comment |
+|    | [Return] | args 1 | args 2 |
diff --git a/src/robot/parsing/__init__.py b/src/robot/parsing/__init__.py
@@ -51,5 +51,5 @@ def print_suite(suite):
 
 from .builders import get_model, get_resource_model
 from .lexer import get_tokens, get_resource_tokens, Token
-from .model import ModelVisitor
+from .model import ModelTransformer
 from .suitestructure import SuiteStructureBuilder, SuiteStructureVisitor
diff --git a/src/robot/parsing/builders.py b/src/robot/parsing/builders.py
@@ -23,16 +23,16 @@
 
 def get_model(source, data_only=False, curdir=None):
     tokens = get_tokens(source, data_only)
-    return _build_model(get_statements(tokens, curdir))
+    return _build_model(source, get_statements(tokens, curdir))
 
 
 def get_resource_model(source, data_only=False, curdir=None):
     tokens = get_resource_tokens(source, data_only)
-    return _build_model(get_statements(tokens, curdir))
+    return _build_model(source, get_statements(tokens, curdir))
 
 
-def _build_model(statements):
-    builder = FileBuilder()
+def _build_model(source, statements):
+    builder = FileBuilder(source)
     stack = [builder]
     for statement in statements:
         while not stack[-1].handles(statement):
@@ -57,8 +57,8 @@ def statement(self, statement):
 
 class FileBuilder(Builder):
 
-    def __init__(self, model=None):
-        Builder.__init__(self, model or File())
+    def __init__(self, source=None):
+        Builder.__init__(self, File(source))
 
     def statement(self, statement):
         try:
@@ -90,6 +90,8 @@ class TestCaseSectionBuilder(SectionBuilder):
 
     def statement(self, statement):
         if statement.type == Token.EOL:
+            if self.model.body.items:
+                self.model.body.add(statement)
             return self
         model = TestCase(statement)
         self.model.body.add(model)
@@ -100,6 +102,8 @@ class KeywordSectionBuilder(SectionBuilder):
 
     def statement(self, statement):
         if statement.type == Token.EOL:
+            if self.model.body.items:
+                self.model.body.add(statement)
             return self
         model = Keyword(statement)
         self.model.body.add(model)
diff --git a/src/robot/parsing/lexer/readers.py b/src/robot/parsing/lexer/readers.py
@@ -125,7 +125,7 @@ def get_tokens(self):
                 yield EOS.from_token(prev_token)
 
     def _handle_old_for(self, statements):
-        end_statement = [Token(Token.END)]
+        end_statement = [Token(Token.SEPARATOR), Token(Token.END)]
         old_for = False
         for statement in statements:
             marker = self._get_first_data_token(statement)
diff --git a/src/robot/parsing/model/__init__.py b/src/robot/parsing/model/__init__.py
@@ -16,4 +16,4 @@
 from .blocks import (File, SettingSection, VariableSection, TestCaseSection,
                      KeywordSection, CommentSection, TestCase, Keyword, ForLoop)
 from .statements import get_statements
-from .visitor import ModelVisitor
+from .visitor import ModelTransformer
diff --git a/src/robot/parsing/model/blocks.py b/src/robot/parsing/model/blocks.py
@@ -15,25 +15,45 @@
 
 import ast
 
-from robot.utils import normalize_whitespace
+from robot.utils import normalize_whitespace, file_writer
 
+from .visitor import ModelVisitor
 from ..lexer import Token
 
 
+class FileWriter(ModelVisitor):
+
+    def __init__(self, output):
+        self.output = output
+
+    def visit_Statement(self, statement):
+        for line in statement.lines:
+            for token in line:
+                self.output.write(token.value)
+
+
 class Block(ast.AST):
     _fields = ()
 
 
 class File(Block):
     _fields = ('sections',)
 
-    def __init__(self):
+    def __init__(self, source=None):
+        self.source = source
         self.sections = []
 
     @property
     def has_tests(self):
         return any(isinstance(s, TestCaseSection) for s in self.sections)
 
+    def save(self, given_output):
+        output = given_output if given_output else \
+            file_writer(self.source)
+        FileWriter(output).visit(self)
+        if given_output is None:
+            output.close()
+
 
 class Section(Block):
     _fields = ('header', 'body')
diff --git a/src/robot/parsing/model/statements.py b/src/robot/parsing/model/statements.py
@@ -48,16 +48,15 @@ def register(cls, subcls):
         cls._statement_handlers[subcls.type] = subcls
         if subcls.type == Token.KEYWORD:
             cls._statement_handlers[Token.ASSIGN] = subcls
+        return cls
 
     @classmethod
     def from_tokens(cls, tokens):
-        if len(tokens) == 1 and tokens[0].type == Token.EOL:
-            return EmptyLine(tokens)
         handlers = cls._statement_handlers
         for token in tokens:
             if token.type in handlers:
                 return handlers[token.type](tokens)
-        raise TypeError('Invalid statement: %r' % (tokens,))
+        return EmptyLine(tokens)
 
     @property
     def data_tokens(self):
@@ -77,15 +76,12 @@ def _tokens(self, *types):
 
     @property
     def lines(self):
-        if self.type in Token.SETTING_TOKENS and len(self.tokens) == 1:
-            return
         line = []
         for token in self.tokens:
-            if token.type != Token.CONTINUATION:
-                line.append(token)
-            else:
+            line.append(token)
+            if token.type == Token.EOL:
                 yield line
-                line = [token]
+                line = []
         if line:
             yield line
 
@@ -424,4 +420,4 @@ def error(self):
 
 
 class EmptyLine(Statement):
-    pass
+    type = Token.EOL
diff --git a/src/robot/parsing/model/visitor.py b/src/robot/parsing/model/visitor.py
@@ -16,11 +16,7 @@
 import ast
 
 
-class ModelVisitor(ast.NodeVisitor):
-
-    def visit(self, node):
-        visitor = self._find_visitor(type(node)) or self.generic_visit
-        return visitor(node)
+class VisitorFinder(object):
 
     def _find_visitor(self, cls):
         if cls is ast.AST:
@@ -32,3 +28,17 @@ def _find_visitor(self, cls):
             visitor = self._find_visitor(base)
             if visitor:
                 return visitor
+
+
+class ModelVisitor(ast.NodeVisitor, VisitorFinder):
+
+    def visit(self, node):
+        visitor = self._find_visitor(type(node)) or self.generic_visit
+        visitor(node)
+
+
+class ModelTransformer(ast.NodeTransformer, VisitorFinder):
+
+    def visit(self, node):
+        visitor = self._find_visitor(type(node)) or self.generic_visit
+        return visitor(node)
diff --git a/src/robot/tidy.py b/src/robot/tidy.py
diff --git a/src/robot/tidypkg/__init__.py b/src/robot/tidypkg/__init__.py
diff --git a/src/robot/tidypkg/datafilewriter.py b/src/robot/tidypkg/datafilewriter.py
diff --git a/src/robot/tidypkg/transformers.py b/src/robot/tidypkg/transformers.py
diff --git a/utest/tidy/test_argument_validation.py b/utest/tidy/test_argument_validation.py
