From 7bd7e7b4153e61b9e03a820c6685954f12bcb310 Mon Sep 17 00:00:00 2001 From: Mateusz Nojek Date: Sun, 28 Jun 2020 22:08:41 +0200 Subject: [PATCH 1/5] Add ignoring for lines containing comment 'noqa' --- rflint/parser/parser.py | 7 ++++++- rflint/rflint.py | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/rflint/parser/parser.py b/rflint/parser/parser.py index eadc936..fd0e685 100644 --- a/rflint/parser/parser.py +++ b/rflint/parser/parser.py @@ -124,6 +124,7 @@ def __init__(self, path, parent=None): self.path = os.path.abspath(path) self.tables = [] self.rows = [] + self.skip_lines = [] try: self._load(path) @@ -172,10 +173,11 @@ def _load(self, path): # N.B. the caller should be catching errors self.raw_text = f.read() f.file.seek(0) + skip_pattern = r'# *noqa\b' matcher = Matcher(re.IGNORECASE) for linenumber, raw_text in enumerate(f.readlines()): - linenumber += 1; # start counting at 1 rather than zero + linenumber += 1 # start counting at 1 rather than zero # this mimics what the robot TSV reader does -- # it replaces non-breaking spaces with regular spaces, @@ -183,6 +185,9 @@ def _load(self, path): raw_text = raw_text.replace(u'\xA0', ' ') raw_text = raw_text.rstrip() + if re.search(skip_pattern, raw_text): + self.skip_lines.append(linenumber) + # FIXME: I'm keeping line numbers but throwing away # where each cell starts. I should be preserving that # (though to be fair, robot is throwing that away so diff --git a/rflint/rflint.py b/rflint/rflint.py index 8355fd9..966b2f5 100644 --- a/rflint/rflint.py +++ b/rflint/rflint.py @@ -47,6 +47,7 @@ def __init__(self): # mapping of class names to instances, to enable us to # instantiate each rule exactly once self._rules = {} + self.noqa = [] for path in (builtin_rules, site_rules): for filename in glob.glob(path+"/*.py"): @@ -155,6 +156,10 @@ def _process_file(self, filename): self._print_filename = filename if self.args.print_filenames else None robot_file = RobotFactory(filename) + + for skip_line in robot_file.skip_lines: + self.noqa.append({"path": robot_file.path, "line": skip_line}) + for rule in self.general_rules: if rule.severity != IGNORE: rule.apply(robot_file) @@ -195,6 +200,11 @@ def report(self, linenumber, filename, severity, message, rulename, char): print("+ " + self._print_filename) self._print_filename = None + # do not report nor add error count if line was indicated as 'noqa' + for noqa in self.noqa: + if noqa["path"] == filename and noqa["line"] == linenumber: + return + if severity in (WARNING, ERROR): self.counts[severity] += 1 else: From 403b35e0d4da5ef984afe69b9508b8bf438e232b Mon Sep 17 00:00:00 2001 From: Mateusz Nojek Date: Sun, 28 Jun 2020 22:10:22 +0200 Subject: [PATCH 2/5] Fix reporting correct line number for TagWithSpaces rule --- rflint/rules/testcaseRules.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/rflint/rules/testcaseRules.py b/rflint/rules/testcaseRules.py index 6a7d9df..bbb0153 100644 --- a/rflint/rules/testcaseRules.py +++ b/rflint/rules/testcaseRules.py @@ -20,12 +20,12 @@ class PeriodInTestName(TestRule): '''Warn about periods in the testcase name - + Since robot uses "." as a path separator, using a "." in a testcase - name can lead to ambiguity. + name can lead to ambiguity. ''' severity = WARNING - + def apply(self,testcase): if "." in testcase.name: self.report(testcase, "'.' in testcase name '%s'" % testcase.name, testcase.linenumber) @@ -35,9 +35,16 @@ class TagWithSpaces(TestRule): severity=ERROR def apply(self, testcase): + tags_with_spaces = [] for tag in testcase.tags: if ((" " in tag) or ("\t" in tag)): - self.report(testcase, "space not allowed in tag name: '%s'" % tag, testcase.linenumber) + tags_with_spaces.append(tag) + # second loop is to discover line number of tag appearance + for setting in testcase.settings: + for cell in setting: + for tag_with_spaces in tags_with_spaces: + if tag_with_spaces in cell: + self.report(testcase, "space not allowed in tag name: '%s'" % tag_with_spaces, setting.startline) class RequireTestDocumentation(TestRule): '''Verify that a test suite has documentation From bcac5ebb9e7116948dbfdc628f11ed4dd2020801 Mon Sep 17 00:00:00 2001 From: Mateusz Nojek Date: Wed, 1 Jul 2020 22:05:57 +0200 Subject: [PATCH 3/5] Add tests to cover noqa feature --- test_data/acceptance/noqa.robot | 40 +++++++++++++++++++++++++++++++++ tests/acceptance/noqa.robot | 29 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 test_data/acceptance/noqa.robot create mode 100644 tests/acceptance/noqa.robot diff --git a/test_data/acceptance/noqa.robot b/test_data/acceptance/noqa.robot new file mode 100644 index 0000000..ecca514 --- /dev/null +++ b/test_data/acceptance/noqa.robot @@ -0,0 +1,40 @@ +*** Settings *** +Documentation Test cases with skip line indicator (# noqa) + + +*** Test Cases *** +Skip Tag With Spaces + [Documentation] Skip next line containing tag with spaces + [Tags] Tag With Spaces TagWithNoSpace # noqa + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Do Not Skip Tag With Spaces + [Documentation] Report tag with spaces + [Tags] Tag With Spaces TagWithNoSpace + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Skip This Test Case With Dot. #noqa + [Documentation] Do not report improper test case name + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Do Not Skip This Test Case With Dot. + [Documentation] Report dot presence in test case name + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Skip Very Long Line + [Documentation] Do not report very long line (+100 chars) + ${long_variable} Set Variable This is a very long sentence with too many characters that should not be reported because of the 'noqa' comment at the end of this line # noqa and something else + ${short_variable} Set Variable This is a short variable + +Do Not Skip Very Long Line + [Documentation] Report very long line (+100 chars) + ${long_variable} Set Variable This is a very long sentence with too many characters that will be reported because of lack of 'noqa' comment at the end of this line + ${short_variable} Set Variable This is a short variable + +Do Not Skip Line Because Of Improper Comment # noqaa + [Documentation] Improper comment should not make rflint omit the line + ${var} Set Variable This test case contains only one test step diff --git a/tests/acceptance/noqa.robot b/tests/acceptance/noqa.robot new file mode 100644 index 0000000..9df21f4 --- /dev/null +++ b/tests/acceptance/noqa.robot @@ -0,0 +1,29 @@ +*** Settings *** +| Documentation +| ... | Runs rflint against the rflint test suites and resource files +| +| Library | OperatingSystem +| Library | Process +| Resource | SharedKeywords.robot + +*** Test Cases *** +| Run rflint and verify there are no errors or warnings +| | Run rf-lint with the following options: +| | ... | test_data/acceptance/noqa.robot +| | +| | @{messages}= | Split to lines | ${result.stdout} +| | ${warnings}= | Get match count | ${messages} | regexp=^W: +| | ${errors}= | Get match count | ${messages} | regexp=^E: +| | +| | Run keyword if | "${result.rc}" != "${1}" or ${warnings} != 3 or ${errors} != 1 +| | ... | Fail | unexpectected errors or warnings: \n${result.stdout}\n${result.stderr} +| | +| | Output should contain +| | ... | W: 35, 100: Line is too long (exceeds 100 characters) (LineTooLong) +| | ... | E: 14, 0: space not allowed in tag name: 'Tag With Spaces' (TagWithSpaces) +| | ... | W: 23, 0: '.' in testcase name 'Do Not Skip This Test Case With Dot.' (PeriodInTestName) +| | ... | W: 38, 0: Too few steps (1) in test case (TooFewTestSteps) +| | Output should not contain +| | ... | W: 30, 100: Line is too long (exceeds 100 characters) (LineTooLong) +| | ... | E: 8, 0: space not allowed in tag name: 'Tag With Spaces' (TagWithSpaces) +| | ... | W: 18, 0: '.' in testcase name 'Skip This Test Case With Dot.' (PeriodInTestName) From 03befe8e063ef26d204e27a1b6d9f7c5a40de948 Mon Sep 17 00:00:00 2001 From: Mateusz Nojek Date: Wed, 1 Jul 2020 22:28:28 +0200 Subject: [PATCH 4/5] Add compiled regex, change test name --- rflint/parser/parser.py | 4 ++-- tests/acceptance/noqa.robot | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rflint/parser/parser.py b/rflint/parser/parser.py index fd0e685..f08e487 100644 --- a/rflint/parser/parser.py +++ b/rflint/parser/parser.py @@ -173,7 +173,7 @@ def _load(self, path): # N.B. the caller should be catching errors self.raw_text = f.read() f.file.seek(0) - skip_pattern = r'# *noqa\b' + skip_pattern = re.compile(r'# *noqa\b') matcher = Matcher(re.IGNORECASE) for linenumber, raw_text in enumerate(f.readlines()): @@ -185,7 +185,7 @@ def _load(self, path): raw_text = raw_text.replace(u'\xA0', ' ') raw_text = raw_text.rstrip() - if re.search(skip_pattern, raw_text): + if skip_pattern.search(raw_text): self.skip_lines.append(linenumber) # FIXME: I'm keeping line numbers but throwing away diff --git a/tests/acceptance/noqa.robot b/tests/acceptance/noqa.robot index 9df21f4..480d3ea 100644 --- a/tests/acceptance/noqa.robot +++ b/tests/acceptance/noqa.robot @@ -7,7 +7,7 @@ | Resource | SharedKeywords.robot *** Test Cases *** -| Run rflint and verify there are no errors or warnings +| Run rflint and check if output contains given warnings and errors | | Run rf-lint with the following options: | | ... | test_data/acceptance/noqa.robot | | From 5116fbe403e00a8b35fb83ae74516c86a9d22687 Mon Sep 17 00:00:00 2001 From: Mateusz Nojek Date: Sat, 4 Jul 2020 14:03:49 +0200 Subject: [PATCH 5/5] Change noqa to dict type to speed up the lookup --- rflint/rflint.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rflint/rflint.py b/rflint/rflint.py index 966b2f5..3794e2a 100644 --- a/rflint/rflint.py +++ b/rflint/rflint.py @@ -47,7 +47,7 @@ def __init__(self): # mapping of class names to instances, to enable us to # instantiate each rule exactly once self._rules = {} - self.noqa = [] + self.noqa = {} for path in (builtin_rules, site_rules): for filename in glob.glob(path+"/*.py"): @@ -158,7 +158,7 @@ def _process_file(self, filename): robot_file = RobotFactory(filename) for skip_line in robot_file.skip_lines: - self.noqa.append({"path": robot_file.path, "line": skip_line}) + self.noqa.setdefault(robot_file.path, []).append(skip_line) for rule in self.general_rules: if rule.severity != IGNORE: @@ -201,9 +201,8 @@ def report(self, linenumber, filename, severity, message, rulename, char): self._print_filename = None # do not report nor add error count if line was indicated as 'noqa' - for noqa in self.noqa: - if noqa["path"] == filename and noqa["line"] == linenumber: - return + if filename in self.noqa and linenumber in self.noqa[filename]: + return if severity in (WARNING, ERROR): self.counts[severity] += 1