Merge pull request neomake#1854 from neomake/flake8-fstrings

python: flake8: fix length processing in f-strings

autoload/neomake/makers/ft/python.vim

@@ -130,13 +130,14 @@ function! neomake#makers#ft#python#Flake8EntryProcess(entry) abort
         let type = ''
     endif
 
-    let l:token = matchstr(a:entry.text, "'.*'")
-    if strlen(l:token)
-        " remove quotes
-        let l:token = substitute(l:token, "'", '', 'g')
-        if a:entry.type ==# 'F' && (a:entry.nr == 401 ||  a:entry.nr == 811)
-            " The unused column is incorrect for import errors and redefinition
-            " errors.
+    let token_pattern = '\v''\zs[^'']+\ze'
+    if a:entry.type ==# 'F' && (a:entry.nr == 401 || a:entry.nr == 811)
+        " Special handling for F401 (``module`` imported but unused) and
+        " F811 (redefinition of unused ``name`` from line ``N``).
+        " The unused column is incorrect for import errors and redefinition
+        " errors.
+        let token = matchstr(a:entry.text, token_pattern)
+        if !empty(token)
             let l:view = winsaveview()
             call cursor(a:entry.lnum, a:entry.col)
             " The number of lines to give up searching afterwards
@@ -172,9 +173,43 @@ function! neomake#makers#ft#python#Flake8EntryProcess(entry) abort
             endif
 
             call winrestview(l:view)
+
+            let a:entry.length = strlen(l:token)
         endif
+    else
+        call neomake#postprocess#generic_length_with_pattern(a:entry, token_pattern)
 
-        let a:entry.length = strlen(l:token) " subtract the quotes
+        " Special processing for F821 (undefined name) in f-strings.
+        if !has_key(a:entry, 'length') && a:entry.type ==# 'F' && a:entry.nr == 821
+            let token = matchstr(a:entry.text, token_pattern)
+            if !empty(token)
+                " Search for '{token}' in reported and following lines.
+                " It seems like for the first line it is correct already (i.e.
+                " flake8 reports the column therein), but we still test there
+                " to be sure.
+                " https://gitlab.com/pycqa/flake8/issues/407
+                let line = get(getbufline(a:entry.bufnr, a:entry.lnum), 0, '')
+                " NOTE: uses byte offset, starting at col means to start after
+                " the opening quote.
+                let pos = match(line, '\C\V{'.token, a:entry.col)
+                if pos == -1
+                    let line_offset = 0
+                    while line_offset < 10
+                        let line_offset += 1
+                        let line = get(getbufline(a:entry.bufnr, a:entry.lnum + line_offset), 0, '')
+                        let pos = match(line, '\C\V{'.token)
+                        if pos != -1
+                            let a:entry.lnum = a:entry.lnum + line_offset
+                            break
+                        endif
+                    endwhile
+                endif
+                if pos > 0
+                    let a:entry.col = pos + 2
+                    let a:entry.length = strlen(token)
+                endif
+            endif
+        endif
     endif
 
     let a:entry.text = a:entry.type . a:entry.nr . ' ' . a:entry.text

autoload/neomake/postprocess.vim

@@ -1,12 +1,13 @@
 " Generic postprocessor to add `length` to `a:entry`.
-" The pattern can be overridden on `self`, and should adhere to this:
+" The pattern can be overridden on `self` and should adhere to this:
 "  - the matched word should be returned as the whole match (you can use \zs
 "    and \ze).
 "  - enclosing patterns should be returned as \1 and \2, where \1 is used as
 "    offset when the first entry did not match.
 " See tests/postprocess.vader for tests/examples.
+" See neomake#postprocess#generic_length_with_pattern for a non-dict variant.
 function! neomake#postprocess#generic_length(entry) abort dict
-    if a:entry.bufnr == bufnr('%') && a:entry.lnum > 0 && a:entry.col
+    if a:entry.lnum > 0 && a:entry.col
         let pattern = get(self, 'pattern', '\v(["''`])\zs[^\1]{-}\ze(\1)')
         let start = 0
         let best = 0
@@ -18,7 +19,8 @@ function! neomake#postprocess#generic_length(entry) abort dict
             let l = len(m[0])
             if l > best
                 " Ensure that the text is there.
-                if getline(a:entry.lnum)[a:entry.col-1 : a:entry.col-2+l] == m[0]
+                let line = get(getbufline(a:entry.bufnr, a:entry.lnum), 0, '')
+                if line[a:entry.col-1 : a:entry.col-2+l] == m[0]
                     let best = l
                 endif
             endif
@@ -38,6 +40,12 @@ function! neomake#postprocess#generic_length(entry) abort dict
     endif
 endfunction
 
+" Wrapper to call neomake#process#generic_length (a dict function).
+function! neomake#postprocess#generic_length_with_pattern(entry, pattern) abort
+    let this = {'pattern': a:pattern}
+    return call('neomake#postprocess#generic_length', [a:entry], this)
+endfunction
+
 " Deprecated: renamed to neomake#postprocess#generic_length.
 function! neomake#postprocess#GenericLengthPostprocess(entry) abort dict
     return neomake#postprocess#generic_length(a:entry)

tests/ft_python.vader

@@ -112,20 +112,64 @@ Execute (python: flake8: errorformat/postprocess: F811):
   \ 'text': "F811 redefinition of unused 'os' from line 2"}, e
   bwipe!
 
+Execute (flake8: postprocess for F821 in continuous f-strings):
+  new
+  let bufnr = bufnr('%')
+  file t-undefined-in-fstring.py
+  call append(0, [
+  \ 'bar',
+  \ 'BAZ = 1',
+  \ 'foo = (f"prefix {foo}"',
+  \ '       f"prefix {BAZ} {baz} {obj.attr}")',
+  \ ])
+
+  " Output from flake8:
+  " t-undefined-in-fstring.py:1:1: F821 undefined name 'bar'
+  " t-undefined-in-fstring.py:3:9: F821 undefined name 'baz'
+  " t-undefined-in-fstring.py:3:9: F821 undefined name 'obj'
+  " t-undefined-in-fstring.py:3:18: F821 undefined name 'foo'
+
+  let e = {'lnum': 1, 'bufnr': bufnr, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': 821, 'type': 'F', 'pattern': '', 'text': 'F821 undefined name ''bar'''}
+  call neomake#makers#ft#python#Flake8EntryProcess(e)
+  AssertEqual [e.lnum, e.col, e.length], [1, 1, 3]
+
+  " Correct for first line in f-string.
+  let e = {'lnum': 3, 'bufnr': bufnr, 'col': 18, 'valid': 1, 'vcol': 0, 'nr': 821, 'type': 'F', 'pattern': '', 'text': 'F821 undefined name ''foo'''}
+  call neomake#makers#ft#python#Flake8EntryProcess(e)
+  AssertEqual [e.lnum, e.col, e.length], [3, 18, 3]
+
+  " Needs adjustment for second line in f-string.
+  let e = {'lnum': 3, 'bufnr': bufnr, 'col': 9, 'valid': 1, 'vcol': 0, 'nr': 821, 'type': 'F', 'pattern': '', 'text': 'F821 undefined name ''baz'''}
+  call neomake#makers#ft#python#Flake8EntryProcess(e)
+  AssertEqual [e.lnum, e.col, e.length], [4, 24, 3]
+
+  " Needs adjustment for second line in f-string, handling objects.
+  let e = {'lnum': 3, 'bufnr': bufnr, 'col': 9, 'valid': 1, 'vcol': 0, 'nr': 821, 'type': 'F', 'pattern': '', 'text': 'F821 undefined name ''obj'''}
+  call neomake#makers#ft#python#Flake8EntryProcess(e)
+  AssertEqual [e.lnum, e.col, e.length], [4, 30, 3]
+
+  " Something that cannot be found.
+  let e = {'lnum': 3, 'bufnr': bufnr, 'col': 9, 'valid': 1, 'vcol': 0, 'nr': 821, 'type': 'F', 'pattern': '', 'text': 'F821 undefined name ''cannotbefound'''}
+  call neomake#makers#ft#python#Flake8EntryProcess(e)
+  AssertEqual [e.lnum, e.col], [3, 9]
+  Assert !has_key(e, 'length')
+  bwipe!
+
 Execute (neomake#makers#ft#python#Flake8EntryProcess):
-  let entry = {'type': 'F', 'nr': 841, 'text': "local variable 'foo' is assigned to but never used"}
+  let bufnr = bufnr('%')
+  let entry = {'type': 'F', 'nr': 841, 'text': "local variable 'foo' is assigned to but never used", 'lnum': 1, 'col': 1, 'bufnr': bufnr}
   call neomake#makers#ft#python#Flake8EntryProcess(entry)
   AssertEqual entry.type, 'W'
 
-  let entry = {'type': 'F', 'nr': 999, 'text': "something"}
+  let entry = {'type': 'F', 'nr': 999, 'text': "something", 'lnum': 1, 'col': 1, 'bufnr': bufnr}
   call neomake#makers#ft#python#Flake8EntryProcess(entry)
   AssertEqual entry.type, 'E'
 
-  let entry = {'type': 'F', 'nr': 404, 'text': "not found"}
+  let entry = {'type': 'F', 'nr': 404, 'text': "not found", 'lnum': 1, 'col': 1, 'bufnr': bufnr}
   call neomake#makers#ft#python#Flake8EntryProcess(entry)
   AssertEqual entry.type, 'W'
 
-  let entry = {'type': 'F', 'nr': 407, 'text': "no future"}
+  let entry = {'type': 'F', 'nr': 407, 'text': "no future", 'lnum': 1, 'col': 1, 'bufnr': bufnr}
   call neomake#makers#ft#python#Flake8EntryProcess(entry)
   AssertEqual entry.type, 'E'
 

tests/postprocess.vader

@@ -15,7 +15,7 @@ Execute (neomake#postprocess#generic_length):
   call call('neomake#postprocess#generic_length', [new], this)
   AssertEqual new.length, 7
 
-  " Skips handling of unexpected buffer.
+  " Handles non-existing buffer.
   let new = deepcopy(entry)
   let new.bufnr = entry.bufnr + 1
   call call('neomake#postprocess#generic_length', [new], this)
@@ -45,6 +45,15 @@ Execute (neomake#postprocess#generic_length):
   let new = deepcopy(entry)
   call call('neomake#postprocess#generic_length', [new], this)
   AssertEqual new.length, 3
+
+  " Handles entry for another buffer.
+  new
+  let this = {'postprocess': {}}
+  let new = deepcopy(entry)
+  call call('neomake#postprocess#generic_length', [new], this)
+  AssertEqual new.length, 7
+  bwipe!
+
   bwipe!
 
 Execute (Postprocess: called with dict+maker as self for list):