From 1ab3b69c78648e704536f605ed99b738b1650e44 Mon Sep 17 00:00:00 2001 From: Andreas Freimuth Date: Sat, 17 Mar 2018 01:55:11 +0100 Subject: [PATCH 1/5] Add testcases for tab indentation --- .../test/functional/bad_continuation_tabs.py | 45 +++++++++++++++++++ .../test/functional/bad_continuation_tabs.rc | 3 ++ .../test/functional/bad_continuation_tabs.txt | 6 +++ 3 files changed, 54 insertions(+) create mode 100644 pylint/test/functional/bad_continuation_tabs.py create mode 100644 pylint/test/functional/bad_continuation_tabs.rc create mode 100644 pylint/test/functional/bad_continuation_tabs.txt diff --git a/pylint/test/functional/bad_continuation_tabs.py b/pylint/test/functional/bad_continuation_tabs.py new file mode 100644 index 0000000000..f10d3c6e26 --- /dev/null +++ b/pylint/test/functional/bad_continuation_tabs.py @@ -0,0 +1,45 @@ +"""Regression test case for bad-continuation with tabs""" +# pylint: disable=too-few-public-methods,missing-docstring,invalid-name,unused-variable +# Various alignment for brackets + +# Issue 638 +TEST1 = ["foo", + "bar", + "baz" + ] + +MY_LIST = [ + 1, 2, 3, + 4, 5, 6 + ] + +# Issue 1148 +class Abc(object): + def b(self, c): + self.d( + c) + + def d(self, e): + self.b( + e) # [bad-continuation] + + def f(self): + return [ + self.b, + self.d + ] + + def g(self): + return [self.b, + self.d + ] # [bad-continuation] + + def h(self): + return [self.b, + self.d + ] + + def i(self): + return [self.b, + self.d + ] diff --git a/pylint/test/functional/bad_continuation_tabs.rc b/pylint/test/functional/bad_continuation_tabs.rc new file mode 100644 index 0000000000..5372a21bf4 --- /dev/null +++ b/pylint/test/functional/bad_continuation_tabs.rc @@ -0,0 +1,3 @@ +[FORMAT] +indent-string='\t' +indent-after-paren=1 \ No newline at end of file diff --git a/pylint/test/functional/bad_continuation_tabs.txt b/pylint/test/functional/bad_continuation_tabs.txt new file mode 100644 index 0000000000..7458b7a353 --- /dev/null +++ b/pylint/test/functional/bad_continuation_tabs.txt @@ -0,0 +1,6 @@ +bad-continuation:24::"Wrong hanging indentation (add 1 space). + e) # [bad-continuation] + ^|" +bad-continuation:35::"Wrong continued indentation. + ] # [bad-continuation] + ^ ||" From 8311f24a7790c6fc9085a085b833431ca06c03a6 Mon Sep 17 00:00:00 2001 From: Andreas Freimuth Date: Sat, 17 Mar 2018 23:56:39 +0100 Subject: [PATCH 2/5] Fix indentation handling with tabs (Issue: #1148) A Tab is not equal to 8 spaces. So just counting tabs as 8 spaces is wrong. Use the whole 'tabs' and/or 'spaces' string for indentation checks instead of some imaginary number of whitespaces. --- pylint/checkers/format.py | 127 +++++++++++++++++++++++--------------- 1 file changed, 77 insertions(+), 50 deletions(-) diff --git a/pylint/checkers/format.py b/pylint/checkers/format.py index 013ed47e3e..a1074d588f 100644 --- a/pylint/checkers/format.py +++ b/pylint/checkers/format.py @@ -168,6 +168,17 @@ def _token_followed_by_eol(tokens, position): tokens.type(position+2) == tokenize.NL) +def _get_indent_string(line): + """Return the indention string of the given line.""" + result = '' + for char in line: + if char == ' ' or char == '\t': + result += char + else: + break + return result + + def _get_indent_length(line): """Return the length of the indentation on the given token's line.""" result = 0 @@ -185,6 +196,9 @@ def _get_indent_hint_line(bar_positions, bad_position): """Return a line with |s for each of the positions in the given lists.""" if not bar_positions: return ('', '') + # TODO tabs should not be replaced by some random (8) numebr of spaces + bar_positions = [_get_indent_length(indent) for indent in bar_positions] + bad_position = _get_indent_length(bad_position) delta_message = '' markers = [(pos, '|') for pos in bar_positions] if len(markers) == 1: @@ -203,8 +217,8 @@ def _get_indent_hint_line(bar_positions, bad_position): class _ContinuedIndent(object): - __slots__ = ('valid_outdent_offsets', - 'valid_continuation_offsets', + __slots__ = ('valid_outdent_strings', + 'valid_continuation_strings', 'context_type', 'token', 'position') @@ -213,10 +227,10 @@ def __init__(self, context_type, token, position, - valid_outdent_offsets, - valid_continuation_offsets): - self.valid_outdent_offsets = valid_outdent_offsets - self.valid_continuation_offsets = valid_continuation_offsets + valid_outdent_strings, + valid_continuation_strings): + self.valid_outdent_strings = valid_outdent_strings + self.valid_continuation_strings = valid_continuation_strings self.context_type = context_type self.position = position self.token = token @@ -247,16 +261,16 @@ def __init__(self, _CONTINUATION_HINT_MESSAGE = ' (%s %d space%s)' # Ex: (remove 2 spaces) -def _Offsets(*args): - """Valid indentation offsets for a continued line.""" +def _Indentations(*args): + """Valid indentation strings for a continued line.""" return dict((a, None) for a in args) -def _BeforeBlockOffsets(single, with_body): - """Valid alternative indent offsets for continued lines before blocks. +def _BeforeBlockIndentations(single, with_body): + """Valid alternative indentation strings for continued lines before blocks. - :param int single: Valid offset for statements on a single logical line. - :param int with_body: Valid offset for statements on several lines. + :param int single: Valid indentation string for statements on a single logical line. + :param int with_body: Valid indentation string for statements on several lines. :returns: A dictionary mapping indent offsets to a string representing whether the indent if for a line or block. @@ -286,6 +300,13 @@ def start_col(self, idx): def line(self, idx): return self._tokens[idx][4] + def line_indent(self, idx): + return _get_indent_string(self.line(idx)) + + def token_indent(self, idx): + line_indent = self.line_indent(idx) + return line_indent + ' ' * (self.start_col(idx) - len(line_indent)) + class ContinuedLineState(object): """Tracker for continued indentation inside a logical line.""" @@ -303,8 +324,12 @@ def has_content(self): return bool(self._cont_stack) @property - def _block_indent_size(self): - return len(self._config.indent_string.replace('\t', ' ' * _TAB_LENGTH)) + def _block_indent_string(self): + return self._config.indent_string.replace('\\t', '\t') + + @property + def _continuation_string(self): + return self._block_indent_string[0] * self._config.indent_after_paren @property def _continuation_size(self): @@ -337,10 +362,10 @@ def next_logical_line(self): self.retained_warnings = [] self._cont_stack = [] - def add_block_warning(self, token_position, state, valid_offsets): - self.retained_warnings.append((token_position, state, valid_offsets)) + def add_block_warning(self, token_position, state, valid_indentations): + self.retained_warnings.append((token_position, state, valid_indentations)) - def get_valid_offsets(self, idx): + def get_valid_indentations(self, idx): """Returns the valid offsets for the token at the given position.""" # The closing brace on a dict or the 'for' in a dict comprehension may # reset two indent levels because the dict value is ended implicitly @@ -349,30 +374,30 @@ def get_valid_offsets(self, idx): stack_top = -2 indent = self._cont_stack[stack_top] if self._tokens.token(idx) in _CLOSING_BRACKETS: - valid_offsets = indent.valid_outdent_offsets + valid_indentations = indent.valid_outdent_strings else: - valid_offsets = indent.valid_continuation_offsets - return indent, valid_offsets.copy() + valid_indentations = indent.valid_continuation_strings + return indent, valid_indentations.copy() def _hanging_indent_after_bracket(self, bracket, position): """Extracts indentation information for a hanging indent.""" - indentation = _get_indent_length(self._tokens.line(position)) - if self._is_block_opener and self._continuation_size == self._block_indent_size: + indentation = self._tokens.line_indent(position) + if self._is_block_opener and self._continuation_string == self._block_indent_string: return _ContinuedIndent( HANGING_BLOCK, bracket, position, - _Offsets(indentation + self._continuation_size, indentation), - _BeforeBlockOffsets(indentation + self._continuation_size, - indentation + self._continuation_size * 2)) + _Indentations(indentation + self._continuation_string, indentation), + _BeforeBlockIndentations(indentation + self._continuation_string, + indentation + self._continuation_string * 2)) if bracket == ':': # If the dict key was on the same line as the open brace, the new # correct indent should be relative to the key instead of the # current indent level - paren_align = self._cont_stack[-1].valid_outdent_offsets - next_align = self._cont_stack[-1].valid_continuation_offsets.copy() + paren_align = self._cont_stack[-1].valid_outdent_strings + next_align = self._cont_stack[-1].valid_continuation_strings.copy() next_align_keys = list(next_align.keys()) - next_align[next_align_keys[0] + self._continuation_size] = True + next_align[next_align_keys[0] + self._continuation_string] = True # Note that the continuation of # d = { # 'a': 'b' @@ -384,27 +409,29 @@ def _hanging_indent_after_bracket(self, bracket, position): HANGING, bracket, position, - _Offsets(indentation, indentation + self._continuation_size), - _Offsets(indentation + self._continuation_size)) + _Indentations(indentation, indentation + self._continuation_string), + _Indentations(indentation + self._continuation_string)) def _continuation_inside_bracket(self, bracket, pos): """Extracts indentation information for a continued indent.""" - indentation = _get_indent_length(self._tokens.line(pos)) - token_start = self._tokens.start_col(pos) - next_token_start = self._tokens.start_col(pos + 1) - if self._is_block_opener and next_token_start - indentation == self._block_indent_size: + indentation = self._tokens.line_indent(pos) + token_indent = self._tokens.token_indent(pos) + next_token_indent = self._tokens.token_indent(pos + 1) + if self._is_block_opener and next_token_indent == indentation + self._block_indent_string: return _ContinuedIndent( CONTINUED_BLOCK, bracket, pos, - _Offsets(token_start), - _BeforeBlockOffsets(next_token_start, next_token_start + self._continuation_size)) + _Indentations(token_indent), + _BeforeBlockIndentations( + next_token_indent, + next_token_indent + self._continuation_string)) return _ContinuedIndent( CONTINUED, bracket, pos, - _Offsets(token_start), - _Offsets(next_token_start)) + _Indentations(token_indent), + _Indentations(next_token_indent)) def pop_token(self): self._cont_stack.pop() @@ -889,9 +916,9 @@ def _check_line_ending(self, line_ending, line_num): def _process_retained_warnings(self, tokens, current_pos): single_line_block_stmt = not _last_token_on_line_is(tokens, current_pos, ':') - for indent_pos, state, offsets in self._current_line.retained_warnings: - block_type = offsets[tokens.start_col(indent_pos)] - hints = {k: v for k, v in offsets.items() if v != block_type} + for indent_pos, state, indentations in self._current_line.retained_warnings: + block_type = indentations[tokens.token_indent(indent_pos)] + hints = {k: v for k, v in indentations.items() if v != block_type} if single_line_block_stmt and block_type == WITH_BODY: self._add_continuation_message(state, hints, tokens, indent_pos) elif not single_line_block_stmt and block_type == SINGLE_LINE: @@ -906,27 +933,27 @@ def same_token_around_nl(token_type): if not self._current_line.has_content or tokens.type(next_idx) == tokenize.NL: return - state, valid_offsets = self._current_line.get_valid_offsets(next_idx) + state, valid_indentations = self._current_line.get_valid_indentations(next_idx) # Special handling for hanging comments and strings. If the last line ended # with a comment (string) and the new line contains only a comment, the line # may also be indented to the start of the previous token. if same_token_around_nl(tokenize.COMMENT) or same_token_around_nl(tokenize.STRING): - valid_offsets[tokens.start_col(next_idx-2)] = True + valid_indentations[tokens.token_indent(next_idx-2)] = True # We can only decide if the indentation of a continued line before opening # a new block is valid once we know of the body of the block is on the # same line as the block opener. Since the token processing is single-pass, # emitting those warnings is delayed until the block opener is processed. if (state.context_type in (HANGING_BLOCK, CONTINUED_BLOCK) - and tokens.start_col(next_idx) in valid_offsets): - self._current_line.add_block_warning(next_idx, state, valid_offsets) - elif tokens.start_col(next_idx) not in valid_offsets: - - self._add_continuation_message(state, valid_offsets, tokens, next_idx) + and tokens.token_indent(next_idx) in valid_indentations): + self._current_line.add_block_warning(next_idx, state, valid_indentations) + elif tokens.token_indent(next_idx) not in valid_indentations: + self._add_continuation_message(state, valid_indentations, tokens, next_idx) - def _add_continuation_message(self, state, offsets, tokens, position): + def _add_continuation_message(self, state, indentations, tokens, position): readable_type, readable_position = _CONTINUATION_MSG_PARTS[state.context_type] - hint_line, delta_message = _get_indent_hint_line(offsets, tokens.start_col(position)) + hint_line, delta_message = _get_indent_hint_line(indentations, + tokens.token_indent(position)) self.add_message( 'bad-continuation', line=tokens.start_line(position), From 1d4beb2136a059fe46779bd996ba881296dde25a Mon Sep 17 00:00:00 2001 From: Andreas Freimuth Date: Sat, 17 Mar 2018 11:33:14 +0100 Subject: [PATCH 3/5] Fix closing brace/bracket/parenthesis on multi-line constructs (Issue: #638) --- pylint/checkers/format.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/format.py b/pylint/checkers/format.py index a1074d588f..dfcf373bd5 100644 --- a/pylint/checkers/format.py +++ b/pylint/checkers/format.py @@ -430,7 +430,7 @@ def _continuation_inside_bracket(self, bracket, pos): CONTINUED, bracket, pos, - _Indentations(token_indent), + _Indentations(token_indent, next_token_indent), _Indentations(next_token_indent)) def pop_token(self): From 5d4708a7aa31b9650558072024268cfb3b7fba3c Mon Sep 17 00:00:00 2001 From: Andreas Freimuth Date: Sat, 24 Mar 2018 10:20:37 +0100 Subject: [PATCH 4/5] Add some documentation --- pylint/checkers/format.py | 12 ++++++++-- .../test/functional/bad_continuation_tabs.py | 23 ++++++++++++------- .../test/functional/bad_continuation_tabs.rc | 2 +- .../test/functional/bad_continuation_tabs.txt | 8 +++---- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/pylint/checkers/format.py b/pylint/checkers/format.py index dfcf373bd5..e069d3a124 100644 --- a/pylint/checkers/format.py +++ b/pylint/checkers/format.py @@ -172,7 +172,7 @@ def _get_indent_string(line): """Return the indention string of the given line.""" result = '' for char in line: - if char == ' ' or char == '\t': + if char in ' \t': result += char else: break @@ -196,7 +196,7 @@ def _get_indent_hint_line(bar_positions, bad_position): """Return a line with |s for each of the positions in the given lists.""" if not bar_positions: return ('', '') - # TODO tabs should not be replaced by some random (8) numebr of spaces + # TODO tabs should not be replaced by some random (8) number of spaces bar_positions = [_get_indent_length(indent) for indent in bar_positions] bad_position = _get_indent_length(bad_position) delta_message = '' @@ -301,9 +301,17 @@ def line(self, idx): return self._tokens[idx][4] def line_indent(self, idx): + """Get the string of TABs and Spaces used for indentation of the line of this token""" return _get_indent_string(self.line(idx)) def token_indent(self, idx): + """Get an indentation string for hanging indentation, consisting of the line-indent plus + a number of spaces to fill up to the column of this token. + + e.g. the token indent for foo + in "print(foo)" + is " " + """ line_indent = self.line_indent(idx) return line_indent + ' ' * (self.start_col(idx) - len(line_indent)) diff --git a/pylint/test/functional/bad_continuation_tabs.py b/pylint/test/functional/bad_continuation_tabs.py index f10d3c6e26..4c42b424cc 100644 --- a/pylint/test/functional/bad_continuation_tabs.py +++ b/pylint/test/functional/bad_continuation_tabs.py @@ -16,30 +16,37 @@ # Issue 1148 class Abc(object): def b(self, c): + # bock indentation self.d( - c) + c) # (is: tabs only) def d(self, e): + # bad hanging indentation self.b( - e) # [bad-continuation] + e) # [bad-continuation] (is: 2 tabs + 7 spaces) def f(self): + # block indentiation (is: all tabs only) return [ self.b, self.d ] def g(self): + # bad hanging indentation + # the closing ] requires 7 - 8 more spcaes; see h(), i() return [self.b, - self.d - ] # [bad-continuation] + self.d # (is: 2 tabs + 8 spaces) + ] # [bad-continuation] (is: tabs only) def h(self): + # hanging indentation: all lined up with first token 'self.b' return [self.b, - self.d - ] + self.d # (is: 2 tabs + 8 spaces) + ] # (is: 2 tabs + 8 spaces) def i(self): + # hangin identation: closing ] lined up with opening [ return [self.b, - self.d - ] + self.d # (2 tabs + 8 spaces) + ] # (2 tabs + 7 spaces) diff --git a/pylint/test/functional/bad_continuation_tabs.rc b/pylint/test/functional/bad_continuation_tabs.rc index 5372a21bf4..bebda38099 100644 --- a/pylint/test/functional/bad_continuation_tabs.rc +++ b/pylint/test/functional/bad_continuation_tabs.rc @@ -1,3 +1,3 @@ [FORMAT] indent-string='\t' -indent-after-paren=1 \ No newline at end of file +indent-after-paren=1 diff --git a/pylint/test/functional/bad_continuation_tabs.txt b/pylint/test/functional/bad_continuation_tabs.txt index 7458b7a353..8fff7bbfb1 100644 --- a/pylint/test/functional/bad_continuation_tabs.txt +++ b/pylint/test/functional/bad_continuation_tabs.txt @@ -1,6 +1,6 @@ -bad-continuation:24::"Wrong hanging indentation (add 1 space). - e) # [bad-continuation] +bad-continuation:26::"Wrong hanging indentation (add 1 space). + e) # [bad-continuation] (is: 2 tabs + 7 spaces) ^|" -bad-continuation:35::"Wrong continued indentation. - ] # [bad-continuation] +bad-continuation:40::"Wrong continued indentation. + ] # [bad-continuation] (is: tabs only) ^ ||" From deaae6181601498d7bf7444b10472bc473f69808 Mon Sep 17 00:00:00 2001 From: Andreas Freimuth Date: Sat, 24 Mar 2018 10:21:48 +0100 Subject: [PATCH 5/5] Changelog entry for indentation handling --- CONTRIBUTORS.txt | 4 +++- ChangeLog | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index ac47fe8793..f27a23edc5 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -10,7 +10,7 @@ Order doesn't matter (not that much, at least ;) * Claudiu Popa: maintainer, contributor * Ashley Whetter: committer - + check_docs extension can find constructor parameters in __init__, added check_raise_docs extension, various patches. @@ -161,3 +161,5 @@ Order doesn't matter (not that much, at least ;) * Tobias Hernstig: contributor * Konstantin Manna: contributor + +* Andreas Freimuth: fix indentation checking with tabs diff --git a/ChangeLog b/ChangeLog index dea0184c23..dccdc2c14e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -110,6 +110,14 @@ What's New in Pylint 2.0? Close #1699 + * Fix indentation handling with tabs + + Close #1148 + + * Fix false-positive ``bad-continuation`` error + + Close #638 + What's New in Pylint 1.8.1? =========================