xdebug / vscode-php-debug

PHP Debug Adapter for Visual Studio Code 🐞⛔
MIT License
775 stars 176 forks source link

The same expression is evaluated differently depending on `context` #742

Open przepompownia opened 2 years ago

przepompownia commented 2 years ago

PHP version: 8.1.2 (cli) Xdebug version: v3.1.2 VS Code extension version: 1.23.0

Your launch.json:

dap.configurations.php = {
  {
    log = true,
    type = 'php',
    request = 'launch',
    name = 'Listen for XDebug',
    port = 9003,
    stopOnEntry = false,
    xdebugSettings = {
      max_children = 512,
      max_data = 1024,
      max_depth = 4,
      extended_properties = 1,
    },
    breakpoints = {
      exception = {
        Notice = false,
        Warning = false,
        Error = false,
        Exception = false,
        ['*'] = false,
      },
    },
  }
}

Xdebug php.ini config:

zend_extension=xdebug.so
[debug]
xdebug.show_local_vars=on
xdebug.discover_client_host = false
xdebug.client_host = localhost
xdebug.client_port = 9003
xdebug.file_link_format=xdebug://%f@%l
xdebug.cli_color=1
xdebug.log=/tmp/xdebug.log
xdebug.show_exception_trace=off
xdebug.var_display_max_data=512
xdebug.var_display_max_depth=3
xdebug.max_nesting_level=250
xdebug.mode=debug
xdebug.start_with_request=trigger

Code snippet to reproduce:

<?php

$x = 1;
echo ($x + 1);

xdebug_break();

echo '';

Problem: I try to evaluate $x + 1 with two different contexts: hover and repl. I can get the correct result only with repl.

VS Code extension logfile (from setting "log": true in launch.json): Where to find the log given set log to true? Currently I have nvim's dap log instead, but it seems to give enough information:

[ DEBUG ] 2022-02-12T17:46:46Z+0100 ] ...dap-ui-test/pack/bundle/opt/nvim-dap/lua/dap/session.lua:839 ] "request"     {  arguments = {    
    context = "hover",    
    expression = "$x + 1",
    frameId = 1  
  },
  command = "evaluate",
  seq = 14,
  type = "request"
}
[ DEBUG ] 2022-02-12T17:46:46Z+0100 ] ...dap-ui-test/pack/bundle/opt/nvim-dap/lua/dap/session.lua:560 ] {
  command = "evaluate",  message = "can not get property",
  request_seq = 14,
  seq = 18,
  success = false,  type = "response"
}
[ DEBUG ] 2022-02-12T17:54:01Z+0100 ] ...dap-ui-test/pack/bundle/opt/nvim-dap/lua/dap/session.lua:839 ]        "request"       {                                                                                        
  arguments = {                                                                                                                                                                                                                                 
    context = "repl",                                                                                                                                                                                                                           
    expression = "$x + 1",                                                                                                                                                                                                                      
    frameId = 1                                                                                                                                                                                                                                 
  },                                                                                                                                                                                                                                            
  command = "evaluate",                                                                                                                                                                                                                         
  seq = 15,                                                                                                                                                                                                                                     
  type = "request"                                                                                                                                                                                                                              
}                                                                                                                                                                                                                                               
[ DEBUG ] 2022-02-12T17:54:01Z+0100 ] ...dap-ui-test/pack/bundle/opt/nvim-dap/lua/dap/session.lua:560 ] {                                                                                                                                       
  body = {                                                                                                                                                                                                                                      
    result = "2",                                                                                                                                                                                                                               
    variablesReference = 0                                                                                                                                                                                                                      
  },                                                                                                                                                                                                                                            
  command = "evaluate",                                                                                                                                                                                                                         
  request_seq = 15,                                                                                                                                                                                                                             
  seq = 19,                                                                                                                                                                                                                                     
  success = true,                                                                                                                                                                                                                               
  type = "response"                                                                                                                                                                                                                             
}

Xdebug logfile: I see different requests With context = hover (the adapter seems to try to evaluate nonexisting property?):

[155268] [Step Debug] <- property_get -i 18 -d 0 -c 0 -n "$x + 1"
[155268] [Step Debug] -> <response xmlns="urn:debugger_protocol_v1" xmlns:xdebug="https://xdebug.org/dbgp/xdebug" command="property_get" transaction_id="18" status="break" reason="ok"><error code="300"><message><![CDATA[can not get property]]></message></error></response>

With context = repl

[155268] [Step Debug] <- eval -i 33 -- JHggKyAx
[155268] [Step Debug] -> <response xmlns="urn:debugger_protocol_v1" xmlns:xdebug="https://xdebug.org/dbgp/xdebug" command="eval" transaction_id="33"><property type="int"><![CDATA[2]]></property></response>

See also the initial problem https://github.com/rcarriga/nvim-dap-ui/issues/80 For reproduction I created a portable environment (make and neovim need to be installed additionally): https://github.com/przepompownia/dap-ui-test

zobo commented 2 years ago

Hi @przepompownia ! I'm not sure where nvim logs the Output messages, but they are probably in the dap log as you described it. I do more vscode centric development, so I don't test the debug adapter with nvim or other clients.

However, you are correct, the evaluate command behaves differently depending on context. If you look at the spec the hover operation should be a side effect free operation. This is why I changed the underlying DBGp call from eval to property_get. I changed it because I was implementing a better hover support in vscode (via extensions logic) and wanted to make sure, accidental hovers don't produce side effects. Other contexts (watch, repl, clipboard) still use the eval DBGp command.

Although I think your case is valid, I'm not sure how to handle this in the best way... Honesty, I miss this feature in vscode (select + hover), I would perhaps expect it to send a different context with it...

Let me know your thoughts.

przepompownia commented 2 years ago

Hi @zobo! Thanks for explaining the reason for which you replaced eval with property_get. Indeed, eval-uating visual selection can be an unsafe action or at least can have a side effect (e.g. for $x++).

Maybe error 300 (can not get property) from XDebug - should be converted by the adapter to something more meaningful for the user if property_get was used by evaluate. Probably the user should know that the adapter attempts to evaluate the property with the name specified as expression value, not the value.

Being aware of the side effects I would like to have this unsafe but very useful action in DAP client UI-s (by default, currently nvim-dap uses repl, nvim-dap-ui uses hover). This is obviously beyond the scope of this project.

In practice, on Linux, I use the defaut path to nvim-dap log: ~/.cache/nvim/dap.log. Do you know where i should expect the log file written by the adapter?

I do not any knowledge what requests are sent by other DAP adapters to evaluate visual selection content.

przepompownia commented 2 years ago

I tried to check what does expr (using it like eval) do but I got

(cmd) expr -i 1 -- JHggKyAx
1 | expr
1 | Error(4): unimplemented command
zobo commented 2 years ago

Hi! Yes, expr is not implemented in Xdebug. I talked to Derick about it a while ago and there is basically no way to achieve a side effect free code execution in PHP VM.

Here is a related issue about this feature in vscode https://github.com/microsoft/vscode/issues/18058

Regarding your problem. The only way I see to solve this is by either agreeing on a different context - that would require nvim-dap-ui to change and maybe even behave in a non standard DAP way - or me putting this "feature" behind a flag.

I would like to avoid using launch.json for that as it really feels out of place. Perhaps we could use env to pass such a system option? I looked at nvim-dap source and you should be able to add a env key to your dap.adapters.php dict...

Just an idea.

przepompownia commented 2 years ago

From the practical perspective it good to know for me about reasons mentioned there and that there is a way to strict evaluation at one's own risk. Thanks to https://github.com/rcarriga/nvim-dap-ui/commit/2ae01a8b0b12217cc5c750addd108c4ef15df10a dap-ui users can now choose the context manually.

Thanks @zobo for explanation and insight into the problem.