bmewburn / vscode-intelephense

PHP intellisense for Visual Studio Code
https://intelephense.com
Other
1.6k stars 94 forks source link

Make diagnostic severity configurable #930

Open bmewburn opened 4 years ago

bmewburn commented 4 years ago

Make each diagnostic accept boolean|number where boolean turns on/off and number sets the diagnostic severity to use.

To disable - false|0 To enable with default severity - true To enable with custom severity -

namespace DiagnosticSeverity {
    /**
     * Reports an error.
     */
    export const Error = 1;
    /**
     * Reports a warning.
     */
    export const Warning = 2;
    /**
     * Reports an information.
     */
    export const Information = 3;
    /**
     * Reports a hint.
     */
    export const Hint = 4;
}
nunoperalta commented 4 years ago

I would love to make unused variables a warning :) At the moment they are just hints, hard to track.

rusproject commented 9 months ago

There should be a way for unused variables to be rendered as warnings, not just as faded-out text. Although it can sometimes be a little annoying to write code like

foreach($arr as $k => $unused) {
  // ...
}
$unused; // suppress "not used" warning

but I've come to terms with it. It's a lot more annoying to manually check for unused namespace imports or to miss logic errors, which can be spotted in time when you have warnings about unused variables

str commented 9 months ago
foreach($arr as $k => $unused) {
  // ...
}
$unused; // suppress "not used" warning

What I've done is, if I'm not going to use the values of the array, just the keys, is to loop the keys:

foreach (array_keys($arr) as $k {
  // ...
}

That will allow you not to need to define the $unused variable. Your code will be a bit more readable as you explain you are focusing on the array keys, and that the value will never be used

rusproject commented 9 months ago

@str Thank you! Never thought about it this way. Probably need to update a couple of hundred places in the codebase 😀

However, this solves the problem for the foreach case, but there are still scenarios where you may have an unused variable, such as catching an error and not using the $e object and some other

str commented 9 months ago

such as catching an error and not using the $e object and some other

You don't need to define the variable when catching an Exception. This is completely valid:


        try {
            // something dangerous
        } catch (\Exception) { // not defining a variable
            // send email to admin
        }
rusproject commented 9 months ago

This is completely valid

Omg, you're gorgeous! Who would have known that one simple message in the issue comments would teach me two new things in PHP 😁 Thanks again!

So, regarding my initial remark on the topic, now we see that there are mostly no drawbacks in displaying unused vars as warnings. Therefore, I am eagerly endorsing this feature request once again. It could even become the default behavior, just like in the ESlint, for instance.

yanghoxom commented 1 month ago

@bmewburn do we have any new update for this?