sandhje / vscode-phpmd

VSCode PHP Mess Detector extension
MIT License
15 stars 3 forks source link

Problem Missing After Saving/Reloading #12

Closed rezashamdani closed 7 years ago

rezashamdani commented 7 years ago

Hi,

I leave a comment on gitter, since it's empty there so i write in here as well.

The phpmd succefully detect 11 problems on a file, yet when i try to fix only 1 problem, and then saving it, the phpmd failed to detect the rest of the problems. it stated 0 problem on the output tab.

screen shot 2017-06-17 at 1 16 47 pm screen shot 2017-06-17 at 1 18 33 pm

When i try to run the command on the console, the output works.

screen shot 2017-06-17 at 1 20 04 pm

Also when i try to close the file, and the reopen again, the result is still the same;

screen shot 2017-06-17 at 1 20 55 pm

Please help. Thanks

neerolyte commented 7 years ago

I'm seeing the same behaviour.

sandhje commented 7 years ago

Sorry I didn't get back to you sooner. I'm on vacation in the Alpes and don't have good access to internet right now. Not really the best time to launch an extension I guess :(.

If you could send me the php file at info@ecodes.io I'll try and have a look at it asap.

neerolyte commented 7 years ago

I would but I can't actually find a file that's showing problems in the extension at all now even though phpmd reports warnings directly.

The issue was that any file that had any issue was losing the display of issues whenever it was saved.

sandhje commented 7 years ago

@neerolyte: At the moment I can't reproduce this with any of my own projects (phpmd reporting problems on cli but nothing showing in vscode). So files displaying that behaviour would be fine to, please send one to info@ecodes.io. Perhaps you can also post the output of your logs. Maybe something else is going wrong if you don't get problems for any php file.

sandhje commented 7 years ago

Closing this issue since I received no more feedback on it and can't reproduce it in any way.

neerolyte commented 7 years ago

So the main problem I had supplying a reproduction case was not finding a file (every file does it) it was actually getting the plugin to function again at all.

If I create a brand new file and type/paste this in to it:

<?php
class Foo {
    public function foo() {
        $foo = 1;
    }
}

I get one warning about the constructor name and one about the unused variable.

If I put a comment somewhere (e.g just add a line after $foo = 1; with only // on it) then the warnings disappear.

If I create a new problem, e.g writing a line in the function with $bar = 2 I get a warning that I should avoid the unused variable $bar but the previous warnings are still missing. Any other change and save at this point hides all the warnings again.

neerolyte commented 7 years ago

It actually seems like even that doesn't work reliably, if I paste that contents in to another new file I don't get those warnings again. I only get warnings that are new.

neerolyte commented 7 years ago

Here's a more visual example:

jul-13-2017 13-42-39

sandhje commented 7 years ago

Thanks for the feedback @neerolyte. This shows clearly that it has nothing to do with anything in the file's code. I would really like to have a look at the verbose logs the extension gives you while this happens (the logs posted by @rezashamdani didn't show anything odd, maybe yours do give a hint). I still have no way to reproduce this and as long as I can't reliably reproduce it on any of my systems I'll be unable to find the root cause and fix it. Hopefully your logs will help with that. Thanks in advance.

neerolyte commented 7 years ago

@rezashamdani Sorry I'm kind of new to VSCode still - do you mind pointing me at some doco that explains how to enable the logging you'll need (or giving me some steps)?

For what it's worth I've tried Version 1.15.0-insider and Version 1.14.0 (current stable) and they both show this behaviour.

I have php installed via brew on OS X:

$ php -v
PHP 7.1.5 (cli) (built: May 13 2017 13:30:32) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Xdebug v2.5.4, Copyright (c) 2002-2017, by Derick Rethans
neerolyte commented 7 years ago

Assuming you just mean the output window (under the view menu) I just get:

[Info  - 8:00:53 am] Language server connection initialized.
[Info  - 8:00:53 am] Configuration change triggerd, validating all open documents.
[Info  - 8:00:53 am] New document opened, starting validation.
[Info  - 8:01:05 am] Document saved, starting validation.

The last log entry was triggered by saving. Grepping the code I can't see any calls to the logger other than info/error so I'm assuming I can't turn on more verbose logging here?

neerolyte commented 7 years ago

I noticed I'm not getting log entries generate from controller/PhpmdController.js.

Sticking a console.log in the block that gets the problems back from phpmd:

this.getLogger().info("PHP Mess Detector validation completed for " + document.uri + ". " + diagnostics.length + " problems found", true);
console.log("PHP Mess Detector validation completed for " + document.uri + ". " + diagnostics.length + " problems found");

I get 0 problems back on the second round (when I trigger a save):

[Info  - 8:08:31 am] New document opened, starting validation.
PHP Mess Detector validation completed for file:///Users/.../foo.php. 2 problems found
[Info  - 8:08:40 am] Document saved, starting validation.
PHP Mess Detector validation completed for file:///Users/.../foo.php. 0 problems found

So whatever's going on is upstream from that point.

neerolyte commented 7 years ago

console.loging the payload and output variables (not on exactly the linked lines, but when they're actually filled out).

For the first load (where the warnings display in the editor) I get:

===================payload
PipelinePayloadModel {
  uri: 'file:///.../foo.php',
  raw: '',
  diagnostics: [] }
=================output
PipelinePayloadModel {
  uri: 'file:///.../foo.php',
  raw: '<?xml version="1.0" encoding="UTF-8" ?>\n<pmd version="@project.version@" timestamp="2017-07-13T22:14:03+00:00">\n  <file name="/.../foo.php">\n    <violation beginline="3" endline="6" rule="ConstructorWithNameAsEnclosingClass" ruleset="Naming Rules" package="+global" externalInfoUrl="http://phpmd.org/rules/naming.html#constructorwithnameasenclosingclass" class="Foo" method="foo" priority="3">\n      Classes should not have a constructor method with the same name as the class\n    </violation>\n    <violation beginline="4" endline="4" rule="UnusedLocalVariable" ruleset="Unused Code Rules" externalInfoUrl="http://phpmd.org/rules/unusedcode.html#unusedlocalvariable" priority="3">\n      Avoid unused local variables such as \'$bar\'.\n    </violation>\n  </file>\n</pmd>\n',
  diagnostics: 
   [ { message: '\n      Classes should not have a constructor method with the same name as the class\n    ',
       range: [Object],
       severity: 3,
       source: 'PHP Mess Detector' },
     { message: '\n      Avoid unused local variables such as \'$bar\'.\n    ',
       range: [Object],
       severity: 3,
       source: 'PHP Mess Detector' } ],
  pmd: 
   { '$': 
      { version: '@project.version@',
        timestamp: '2017-07-13T22:14:03+00:00' },
     file: [ [Object] ] } }
[Info  - 8:14:11 am] Document saved, starting validation.

For the second save (where no warnings display in the editor) I get:

===================payload
PipelinePayloadModel {
  uri: 'file:///.../foo.php',
  raw: '',
  diagnostics: [] }
=================output
PipelinePayloadModel {
  uri: 'file:///.../foo.php',
  raw: '<?xml version="1.0" encoding="UTF-8" ?>\n<pmd version="@project.version@" timestamp="2017-07-13T22:14:12+00:00">\n  <file name="/.../foo.php">\n    <violation beginline="3" endline="6" rule="ConstructorWithNameAsEnclosingClass" ruleset="Naming Rules" package="+global" externalInfoUrl="http://phpmd.org/rules/naming.html#constructorwithnameasenclosingclass" class="Foo" method="foo" priority="3">\n      Classes should not have a constructor method with the same name as the class\n    </violation>\n    <violation beginline="4" endline="4" rule="UnusedLocalVariable" ruleset="Unused Code Rules" externalInfoUrl="http://phpmd.org/rules/unusedcode.html#unusedlocalvariable" priority="3">\n      Avoid unused local variables such as \'$bar\'.\n    </violation>\n  </file>\n</pmd>\n',
  diagnostics: [],
  pmd: 
   { '$': 
      { version: '@project.version@',
        timestamp: '2017-07-13T22:14:12+00:00' },
     file: [ [Object] ] } }

The issues are there in the raw XML payload both times, but the diagnostics key is empty when issues fail to display.

neerolyte commented 7 years ago

So I tracked it a bit further in to service/pipeline/BuildDiagnosticsStrategy.js.

If I no-op alreadyReported to be:

    alreadyReported(hash) {
        //return this.reported.indexOf(hash) >= 0;
        return false;
    }

the plugin works ok for my purposes.

I'm not clear why it makes sense to deduplicate the reports here, so for the moment I've reached the limit I can progress this to. Hopefully the debug spam ^ helps :)

sandhje commented 7 years ago

Hi @neerolyte, Thanks for the "digging", this really helps a lot. The verbose logging is not a function of vscode but of my extension. You can enable it in the settings, see the readme file :).

Anyway you've already provided me with much more detail than the verbose logs would have so don't bother posting them here. I guess I need to "revisit the de-duplication" logic (reason for that logic is that PHPMD would give me duplicate errors at times). I'll get this fixed en submit an update for the extension. Thanks a lot for your help.

sandhje commented 7 years ago

Resolved in version 1.0.1

neerolyte commented 7 years ago

Initial testing - everything seems to work "as expected" now, cheers :)