-
Couldn't load subscription status.
- Fork 20
Improve multiline statement handling #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1bd98e1
4786d31
118ef0e
431e8cd
40922ba
3346e05
8928232
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| import logging | ||
| import pathlib | ||
| import re | ||
| from collections import defaultdict | ||
|
|
||
| from ni_python_styleguide._acknowledge_existing_errors import _lint_errors_parser | ||
|
|
@@ -25,32 +24,34 @@ def in_multiline_string(self, lineno): | |
|
|
||
| @staticmethod | ||
| def _count_multiline_string_endings_in_line(line): | ||
| return line.count('"""') + line.count("'''") | ||
| return line.count('"""'), line.count("'''") | ||
|
|
||
| def _load_lines(self): | ||
| in_file = self._error_file.read_text().splitlines() | ||
| current_count = 0 | ||
| current_count = [0, 0] | ||
| for line in in_file: | ||
| line_value = ( | ||
| current_count | ||
| + _InMultiLineStringChecker._count_multiline_string_endings_in_line(line) | ||
| type1, type2 = _InMultiLineStringChecker._count_multiline_string_endings_in_line(line) | ||
| current_count[0] += type1 | ||
| current_count[1] += type2 | ||
|
|
||
| code_part_of_line = line | ||
| if "#" in line: | ||
| code_part_of_line = line.split("#", maxsplit=1)[0] | ||
|
|
||
| # if occurrences of multiline string markers is odd, this must be in a multiline | ||
| # or, if line continuation token is on the ending, assume in a multiline statement | ||
| self._values.append( | ||
| any([part % 2 == 1 for part in current_count]) | ||
| or code_part_of_line.strip().endswith("\\") | ||
| ) | ||
| # if occurances of multiline string markers is odd, this must be in a multiline | ||
| self._values.append(line_value % 2 == 1) | ||
| current_count = line_value | ||
|
|
||
|
|
||
| def _add_noqa_to_line(lineno, code_lines, error_code, explanation): | ||
| line = code_lines[lineno] | ||
| old_line_ending = "\n" if line.endswith("\n") else "" | ||
| line = line.rstrip("\n") | ||
|
|
||
| existing_suppression = re.search(r"noqa (?P<existing_suppresions>[\w\d]+\: [\w\W]+?) -", line) | ||
| if existing_suppression: | ||
| before = existing_suppression.groupdict()["existing_suppresions"] | ||
| if error_code not in before: | ||
| line = line.replace(before, before + f", {error_code}: {explanation}") | ||
| else: | ||
| if f"noqa {error_code}" not in line: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I experimented with changing the multi-output to be actually be like: But when I was looking at the output, I realized that this created distance between the second code - both making it harder to parse, and increasing the size of the diff when fixing one of the codes. Since this wasn't working as intended, and I've realized that this slightly more verbose output is better anyways, simplifying to just append (snapshots show now change to output) |
||
| line += f" # noqa {error_code}: {explanation} (auto-generated noqa)" | ||
|
|
||
| code_lines[lineno] = line + old_line_ending | ||
|
|
@@ -64,7 +65,7 @@ def acknowledge_lint_errors(lint_errors): | |
| """ | ||
| parsed_errors = map(_lint_errors_parser.parse, lint_errors) | ||
| parsed_errors = filter(None, parsed_errors) | ||
| lint_errors_to_process = [error for error in parsed_errors if error not in EXCLUDED_ERRORS] | ||
| lint_errors_to_process = [error for error in parsed_errors if error.code not in EXCLUDED_ERRORS] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initial tests had BLK100 show up, not sure how this was missed previously. |
||
|
|
||
| lint_errors_by_file = defaultdict(list) | ||
| for error in lint_errors_to_process: | ||
|
|
@@ -93,7 +94,7 @@ def acknowledge_lint_errors(lint_errors): | |
| cached_key = f"{error.file}:{error.line + skip}" | ||
| if error.code in handled_lines[cached_key]: | ||
| logging.warning( | ||
| "Multiple occurances of error %s code were logged for %s:%s, only suprressing first", | ||
| "Multiple occurrences of error %s code were logged for %s:%s, only suprressing first", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you spell checker |
||
| error.code, | ||
| error.file, | ||
| error.line + skip, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| some_string_with_line_continuation = f"{installPath}\\foo\\bar\\baz.exe \ | ||
| --check --this-is-cool --bacon --ipsum" | ||
|
|
||
| # black, please leave these ridiculous line continuations in place for testing. | ||
| # fmt: off | ||
| if 1 < 2 == True and \ | ||
| 3 < 4 and \ | ||
| 5 < 6: | ||
| print("There is order in this world") | ||
| else: | ||
| raise Exception("There's funny going on here.") | ||
| # fmt: on | ||
|
|
||
|
|
||
| someStringWithProblems = """ | ||
| problems, I have # even if | ||
| putting the other quote in would be weird | ||
| but like I said: '''I got problems :| | ||
| # even some like this ''' | ||
| many they be, | ||
| suppresion should be after | ||
| """ | ||
|
|
||
| loremIpsum = "dolor" # comments however are not affected \ | ||
| # even if they have backslashes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| some_string_with_line_continuation = f"{installPath}\\foo\\bar\\baz.exe \ | ||
| --check --this-is-cool --bacon --ipsum" # noqa D100: Missing docstring in public module (auto-generated noqa) # noqa F821: undefined name 'installPath' (auto-generated noqa) | ||
|
|
||
| # black, please leave these ridiculous line continuations in place for testing. | ||
| # fmt: off | ||
| if 1 < 2 == True and \ | ||
| 3 < 4 and \ | ||
| 5 < 6: # noqa E712: comparison to True should be 'if cond is True:' or 'if cond:' (auto-generated noqa) | ||
| print("There is order in this world") | ||
| else: | ||
| raise Exception("There's funny going on here.") | ||
| # fmt: on | ||
|
|
||
|
|
||
| someStringWithProblems = """ | ||
| problems, I have # even if | ||
| putting the other quote in would be weird | ||
| but like I said: '''I got problems :| | ||
| # even some like this ''' | ||
| many they be, | ||
| suppresion should be after | ||
| """ # noqa N816: variable 'someStringWithProblems' in global scope should not be mixedCase (auto-generated noqa) | ||
|
|
||
| loremIpsum = "dolor" # comments however are not affected \ # noqa N816: variable 'loremIpsum' in global scope should not be mixedCase (auto-generated noqa) | ||
| # even if they have backslashes |
Uh oh!
There was an error while loading. Please reload this page.