Skip to content
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

Eval did not return a valid python object, on signature popup #3870

Closed
12 tasks done
TuM0xA-S opened this issue Apr 12, 2021 · 8 comments
Closed
12 tasks done

Eval did not return a valid python object, on signature popup #3870

TuM0xA-S opened this issue Apr 12, 2021 · 8 comments
Labels
bug Upstream Issue is likely in upstream completion engine. Awaiting upstream patches.

Comments

@TuM0xA-S
Copy link

TuM0xA-S commented Apr 12, 2021

Issue Prelude

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your issue:

  • I have read and understood YCM's [CONTRIBUTING][cont] document.
  • I have read and understood YCM's [CODE_OF_CONDUCT][code] document.
  • I have read and understood YCM's [README][readme], especially the
    [Frequently Asked Questions][faq] section.
  • I have searched YCM's issue tracker to find issues similar to the one I'm
    about to report and couldn't find an answer to my problem. ([Example Google
    search.][search])
  • If filing a bug report, I have included the output of vim --version.
  • If filing a bug report, I have included the output of :YcmDebugInfo.
  • If filing a bug report, I have attached the contents of the logfiles using
    the :YcmToggleLogs command.
  • If filing a bug report, I have included which OS (including specific OS
    version) I am using.
  • If filing a bug report, I have included a minimal test case that reproduces
    my issue, using vim -Nu /path/to/YCM/vimrc_ycm_minimal, including what I
    expected to happen and what actually happened.
  • If filing a installation failure report, I have included the entire output
    of install.py (or cmake/make/ninja) including its invocation
  • I understand this is an open-source project staffed by volunteers and
    that any help I receive is a selfless, heartfelt gift of their free time. I
    know I am not entitled to anything and will be polite and courteous.
  • I understand my issue may be closed if it becomes obvious I didn't
    actually perform all of these steps.

Thank you for adhering to this process! It ensures your issue is resolved
quickly and that neither your nor our time is needlessly wasted.

Issue Details

Provide a clear description of the problem, including the following key
questions:

  • What did you do?

Include steps to reproduce here.

  1. vim -Nu /path/to/YCM/ycm_vimrc_minimal
  2. :edit index.php
  3. Enter insert mode and type array_slice(

where index.php looks like:
before editing

<?php
?>

after

<?php
array_slice(
?>

If you made changes to vimrc_ycm_minimal, paste them here:

" only adding config for custom language server (-vvv for logging)

let g:ycm_language_server = [
  \   {
  \     'name': 'php',
  \     'cmdline': ['/home/tum0xa/src/phpactor/bin/phpactor', 'language-server', '-vvv'],
  \     'filetypes': [ 'php' ],
  \   },
  \ ]

Include description of the observed behaviour, including actual output,
screenshots, etc.

Diagnostic data

Output of vim --version

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Feb 10 2021 18:06:52)
Included patches: 1-2489
Compiled by Brenton Horne
Huge version with GTK2 GUI. Features included (+) or not (-):
+acl -farsi +mouse_sgr +tag_binary
+arabic +file_in_path -mouse_sysmouse -tag_old_static
+autocmd +find_in_path +mouse_urxvt -tag_any_white
+autochdir +float +mouse_xterm -tcl
-autoservername +folding +multi_byte +termguicolors
+balloon_eval -footer +multi_lang +terminal
+balloon_eval_term +fork() -mzscheme +terminfo
+browse +gettext +netbeans_intg +termresponse
++builtin_terms -hangul_input +num64 +textobjects
+byte_offset +iconv +packages +textprop
+channel +insert_expand +path_extra +timers
+cindent +ipv6 +perl/dyn +title
+clientserver +job +persistent_undo +toolbar
+clipboard +jumplist +popupwin +user_commands
+cmdline_compl +keymap +postscript +vartabs
+cmdline_hist +lambda +printer +vertsplit
+cmdline_info +langmap +profile +virtualedit
+comments +libcall +python/dyn +visual
+conceal +linebreak +python3/dyn +visualextra
+cryptv +lispindent +quickfix +viminfo
+cscope +listcmds +reltime +vreplace
+cursorbind +localmap +rightleft +wildignore
+cursorshape -lua +ruby/dyn +wildmenu
+dialog_con_gui +menu +scrollbind +windows
+diff +mksession +signs +writebackup
+digraphs +modify_fname +smartindent +X11
+dnd +mouse +sound -xfontset
-ebcdic +mouseshape +spell +xim
+emacs_tags +mouse_dec +startuptime +xpm
+eval +mouse_gpm +statusline +xsmp_interact
+ex_extra -mouse_jsbterm -sun_workshop +xterm_clipboard
+extra_search +mouse_netterm +syntax -xterm_save
system vimrc file: "/etc/vimrc"
user vimrc file: "$HOME/.vimrc"
2nd user vimrc file: "/.vim/vimrc"
user exrc file: "$HOME/.exrc"
system gvimrc file: "/etc/gvimrc"
user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "
/.vim/gvimrc"
defaults file: "$VIMRUNTIME/defaults.vim"
system menu file: "$VIMRUNTIME/menu.vim"
fall-back for $VIM: "/usr/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi -I/usr/include/cairo -I/usr/include/lzo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/atk-1.0 -pthread -g -O2 -D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
Linking: gcc -L. -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -fstack-protector-strong -rdynamic -Wl,-export-dynamic -Wl,-E -Wl,-rpath,/usr/lib/perl5/5.32/core_perl/CORE -L/usr/local/lib -Wl,--as-needed -o vim -lgtk-x11-2.0 -lgdk-x11-2.0 -lpangocairo-1.0 -latk-1.0 -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lharfbuzz -lfontconfig -lfreetype -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE -lm -ltinfo -lelf -lcanberra -lacl -lattr -lgpm -ldl -Wl,-E -Wl,-rpath,/usr/lib/perl5/5.32/core_perl/CORE -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -fstack-protector-strong -L/usr/local/lib -L/usr/lib/perl5/5.32/core_perl/CORE -lperl -lpthread -ldl -lm -lcrypt -lutil -lc

Output of YcmDebugInfo

Printing YouCompleteMe debug information...
-- Resolve completions: Up front
-- Client logfile: /tmp/ycm_a8330ukj.log
-- Server Python interpreter: /usr/bin/python3
-- Server Python version: 3.9.2
-- Server has Clang support compiled in: False
-- Clang version: None
-- No extra configuration file found
-- GenericLSP completer debug information:
-- phpCompleter running
-- phpCompleter process ID: 4803
-- phpCompleter executable: ['/home/tum0xa/src/phpactor/bin/phpactor', 'language-server', '-vvv']
-- phpCompleter logfiles:
-- /tmp/phpcompleter_stderrim5ydxfr.log
-- phpCompleter Server State: Initialized
-- phpCompleter Project Directory: /home/tum0xa/src/phpsite
-- phpCompleter Settings: {}
-- Server running at: http://127.0.0.1:35485
-- Server process ID: 4790
-- Server logfiles:
-- /tmp/ycmd_35485_stdout_u4ihzteu.log
-- /tmp/ycmd_35485_stderr_c2r_0k5v.log

Output of YcmDiags

No warnings or errors detected.

Output of git rev-parse HEAD in YouCompleteMe installation directory

a3d0223

Vim error messages

https://gist.github.com/TuM0xA-S/2848781a7503eb0b48fe487227ae00b4

Contents of YCM, ycmd and completion engine logfiles

ycm:
https://gist.github.com/TuM0xA-S/01e1d584f313ed8a0a1d9af66420f32f
https://gist.github.com/TuM0xA-S/a02e0c5cb03607dc054887871a1bea19
https://gist.github.com/TuM0xA-S/0dc339a57805d67f4c981c411a429c84

language server:
https://gist.github.com/TuM0xA-S/b46fff7f0cd7279caa2d84412f1895b6

OS version, distribution, etc.

Linux 5.11.2-1-MANJARO GNU/Linux

Output of build/install commands

./install.py --go-completer --clangd-completer

@bstaletic
Copy link
Collaborator

So the relevant parts of the logs are:

  1. LSP
    1.1. This one is perfectly fine.
  2. ycmd
    2.1. This is showing the same thing as above. Again, valid response from phpactor received.
  3. YCM
    3.1. This one could be a problem. Let's see...
          {
            "label": [
              94,
              5
            ],
            "documentation": "array $array"
          },

That's the first parameter. The 94 should be the start column of the parameter and 5 should be the end. This is clearly nonsense.

The way to calculate this is here.

Something's off in this calculation when the above is the signature_help response...

@bstaletic bstaletic added the bug label Apr 14, 2021
@bstaletic
Copy link
Collaborator

Here's a quick and dirty fix:

diff --git a/ycmd/utils.py b/ycmd/utils.py
index 74c352b2..55937915 100644
--- a/ycmd/utils.py
+++ b/ycmd/utils.py
@@ -163,8 +163,11 @@ def ByteOffsetToCodepointOffset( line_value, byte_offset ):
   This method converts the |byte_offset|, which is a 1-based utf-8 byte offset,
   into a 1-based codepoint offset in the unicode string |line_value|."""

-  byte_line_value = ToBytes( line_value )
-  return len( ToUnicode( byte_line_value[ : byte_offset - 1 ] ) ) + 1
+  if byte_offset == 0:
+    return 0
+  else:
+    byte_line_value = ToBytes( line_value )
+    return len( ToUnicode( byte_line_value[ : byte_offset - 1 ] ) ) + 1


 def CodepointOffsetToByteOffset( unicode_line_value, codepoint_offset ):
@@ -179,8 +182,11 @@ def CodepointOffsetToByteOffset( unicode_line_value, codepoint_offset ):
   version of |unicode_line_value|."""

   # Should be a no-op, but in case someone passes a bytes instance.
-  unicode_line_value = ToUnicode( unicode_line_value )
-  return len( ToBytes( unicode_line_value[ : codepoint_offset - 1 ] ) ) + 1
+  if codepoint_offset == 0:
+    return 0
+  else:
+    unicode_line_value = ToUnicode( unicode_line_value )
+    return len( ToBytes( unicode_line_value[ : codepoint_offset - 1 ] ) ) + 1


 def GetUnusedLocalhostPort():

The backtrace was caused by miscalculating offset 0 when converting from codepoint offset to byte offset (and vice versa). However, since phpactor returns array as the first argument label, it's impossible, in generic way, to calculate correct offsets.

  1. array can be found at offset 0 in the word array_slice. This is clearly wrong and the immediate question is "why not fast forward to (?" Well...
    1.1. Some languages have infix function calls, like (a foo b) doing foo(a, b).
    1.2. Other languages have prefix function calls, but still like that parenthesis shenanigans, like (foo a b).
    1.3. Swift (or was it objective-c) has square bracket function calls in addition to what is usual.
  2. In theory, we could special-case PHP and do 1, but then array parameter label would be found in array type, when the intention of phpactor was to highlight the third occurence in $array.
    2.1. Still, it's possible to make a special case for PHP. Fast-forward to $.

On the other hand, phpactor should really send array $array as the label. Or, you know, name the variable differently! I do consider this to also show an upstream bug, thus labeled appropriately.

@bstaletic bstaletic added the Upstream Issue is likely in upstream completion engine. Awaiting upstream patches. label Apr 14, 2021
@puremourning
Copy link
Member

Yeah, if the php lang server used the newer signature help response, then it would work better. As it uses the older one without the offsets we just have to guess, and we guess wrong.

But yeah, are we saying that our whole calculation is off ? CodepointOffsetToByteOffset is supposed to only work with 1-based offsets..... right ?!

  This method converts the |codepoint_offset| which is a 1-based unicode
  codepoint offset into a 1-based byte offset into the utf-8 encoded bytes
  version of |unicode_line_value|."""

0 is not a valid input. We should probably correct that?

@bstaletic
Copy link
Collaborator

From the /signature_help API documentation:

 label: integer[]

    The array of offsets representing an inclusive start and exclusive end within the parameter's containing signature label.

    Offsets are byte offsets into the UTF8 encoded string 'label'

So this specifically says the output is 0-based. Changing it to 1-based is an API break, but one we might want to make.

Here's a patch I think is enough to solve this:

diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py
index cdbafa18c..14b61cd5c 100644
--- a/ycmd/completers/language_server/language_server_completer.py
+++ b/ycmd/completers/language_server/language_server_completer.py
@@ -1524,7 +1524,7 @@ class LanguageServerCompleter( Completer ):
       for arg in sig[ 'parameters' ]:
         arg_label = arg[ 'label' ]
         if not isinstance( arg_label, list ):
-          begin = sig[ 'label' ].find( arg_label, end )
+          begin = sig[ 'label' ].find( arg_label, end ) + 1 # 1-based
           end = begin + len( arg_label )
         else:
           begin, end = arg_label

Alternatively:

diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py
index cdbafa18c..151bc8307 100644
--- a/ycmd/completers/language_server/language_server_completer.py
+++ b/ycmd/completers/language_server/language_server_completer.py
@@ -1529,8 +1529,8 @@ class LanguageServerCompleter( Completer ):
         else:
           begin, end = arg_label
         arg[ 'label' ] = [
-          utils.CodepointOffsetToByteOffset( sig_label, begin ),
-          utils.CodepointOffsetToByteOffset( sig_label, end ) ]
+          utils.CodepointOffsetToByteOffset( sig_label, begin + 1 ) - 1,
+          utils.CodepointOffsetToByteOffset( sig_label, end + 1 ) - 1 ]
     result.setdefault( 'activeParameter', 0 )
     result.setdefault( 'activeSignature', 0 )
     return result

@TuM0xA-S
Copy link
Author

now it works like this - no errors but something wrong with offsets
https://terminalizer.com/view/8adcc22c4811

@bstaletic
Copy link
Collaborator

The first offset is definitely wrong. That's what I've been complaining about, with phpactor making us guess. I've created phpactor/phpactor#1235, which should resolve the rest of the issue.

@bstaletic
Copy link
Collaborator

Fixed a while ago. The rest is up to phpactor.

@TuM0xA-S
Copy link
Author

TuM0xA-S commented Jun 1, 2021

btw i just stopped use php

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Upstream Issue is likely in upstream completion engine. Awaiting upstream patches.
Projects
None yet
Development

No branches or pull requests

3 participants