php-language-server icon indicating copy to clipboard operation
php-language-server copied to clipboard

Support type hinting for variables

Open filliph opened this issue 8 years ago • 25 comments

If I have a variable that's type hinted like so:

screenshot 2017-04-11 at 8 41 54 pm

I expect to see the method definitions for the methods I call on the $emailChange variable. This does not happen:

screenshot 2017-04-11 at 8 43 42 pm

This type hinting syntax is allegedly supported by phpStorm, and it would be wonderful to have support for this in VSCode via your IntelliSense plugin as well.

Thank you in advance :)

filliph avatar Apr 11 '17 19:04 filliph

Definitely, there is already handling for @params tags, so it would be trivial to add @var tags handling if someone wants to take a shot at it.

felixfbecker avatar Apr 11 '17 20:04 felixfbecker

Would you be able to give me a hint as to where to implement this? I could probably handle it if I had an idea of where to look :)

filliph avatar Apr 11 '17 20:04 filliph

In this function: https://github.com/felixfbecker/php-language-server/blob/master/src/DefinitionResolver.php#L726

felixfbecker avatar Apr 11 '17 20:04 felixfbecker

I guess I'm not advanced enough a PHP coder, I can't work out how to actually test any changes I make. Trying to run the web server just produces a bunch of Undefined Index errors related to headers in the ProtocolStreamer.php file.

I'm seeing around line 798 that variables should have @var tags supported, unless I'm just horribly misreading the code? Does that make the fact that this isn't working a bug?

If it isn't a bug and I'm just failing at reading your code, would you please consider adding it yourself, as you're more intimately familiar with the code? I would really appreciate it!

Sorry I couldn't be of more help :(

filliph avatar Apr 11 '17 21:04 filliph

The language server has nothing to do with a web server or headers ;) See the README for how to run tests. @var tags are read for properties, not variables.

felixfbecker avatar Apr 11 '17 21:04 felixfbecker

I still don't understand, sorry.

The only mention of testing that I could find in the README file was a reference to unit testing. I don't have much experience with unit testing, so me running a command that says "149 / 149 (100%)" doesn't do much when I'm struggling with actually working out how to read the stubs file to find out if any changes are working.

I guess I just have to hope someone is kind enough to implement this for me :)

filliph avatar Apr 11 '17 21:04 filliph

Well, then I take a look and check wether I can do that. This is very important to me, so I'll do it first.

@felixfbecker Can you assign me to this issue in any way? So I don't get lost what I should do or not?

jens1o avatar Apr 12 '17 07:04 jens1o

ah... thanks. Didn't expected that.

jens1o avatar Apr 12 '17 07:04 jens1o

Is this still an issue in the latest version?

felixfbecker avatar Jun 17 '17 08:06 felixfbecker

@felixfbecker I have tested this in v1.4.1 and it is still an issue.

		/** @var \DBTech\Credits\Entity\Currency $currency */
		if (!$currency = DBTech::em()->findOne('DBTech\Credits:Currency', $cleanedInput['id']))
		{
			// Bad currency
			throw $this->errorException(DBTech::phrase('dbtech_credits_invalid_currency'));
		}

		if (!$currency->isActive())
		{
			// Bad currency
			throw $this->errorException(DBTech::phrase('dbtech_credits_invalid_currency'));
		}

I receive no type hinting for isActive.

That method of docblocking allegedly works in phpStorm (untested, but this information comes from the XenForo product developers who use phpStorm internally) but does not appear to work here.

filliph avatar Jun 21 '17 14:06 filliph

Does it work if you write it like

/** @var \DBTech\Credits\Entity\Currency $currency */
$currency = DBTech::em()->findOne('DBTech\Credits:Currency', $cleanedInput['id']);

felixfbecker avatar Jun 21 '17 14:06 felixfbecker

Small sidenote, as I was cutting the variable definition out of that line I appear to be triggering a bug.

https://i.imgur.com/wacZGc9.png

It is not recognising that I typed an expression, and also the php process is running rampant, using 100% CPU and causing my fans to go into overdrive. The only fix is to completely close VSCode and Force Quit the php process, then restarting VSCode.

This happens seemingly randomly, and has happened since I first installed this extension around the time I opened this issue.

Is this a known issue?

Regardless;

		/** @var \DBTech\Credits\Entity\Currency $currency */
		$currency = DBTech::em()->findOne('DBTech\Credits:Currency', $cleanedInput['id']);

		if (!$currency)
		{
			// Bad currency
			throw $this->errorException(DBTech::phrase('dbtech_credits_invalid_currency'));
		}

		if (!$currency->isActive())
		{
			// Bad currency
			throw $this->errorException(DBTech::phrase('dbtech_credits_invalid_currency'));
		}

Still produces "No definition found for isActive".

filliph avatar Jun 21 '17 14:06 filliph

Could you file that as another issue?

felixfbecker avatar Jun 21 '17 14:06 felixfbecker

No problem, I was just checking to see if it was a known issue so I didn't clog up the system with a dupe, I'll go ahead and do that right now :)

filliph avatar Jun 21 '17 14:06 filliph

This does not work completly. Indeed it will be shown, but there are no suggestions for a type hinted var. For example: image

While I have no suggestions: image

When I create a new instance of it and make it explicit, it works: image

I'm using https://docs.planetteamspeak.com/ts3/php/framework/index.html

jens1o avatar Jun 26 '17 13:06 jens1o

yes, that's why this issue is open

felixfbecker avatar Jun 26 '17 13:06 felixfbecker

Ah, I thought it's not even implemented. I was kinda suprised why this issue exists, when it exists, but there is no handling... Hmm.

jens1o avatar Jun 26 '17 13:06 jens1o

Hi! Now with the new language server version with the speed optimization, I think this is the most important feature, IMHO. I would like to know if @property should also be adedd to this type hinting issue, or if it should be in another issue.

Thank you.

str avatar Nov 11 '18 22:11 str

~~Oh, exactly what I am looking for, hopefully this will be implemented soon! Even this issue was here for over a year.~~

Sorry, it works if I add 1 more *, just a little bit difference from PHPStorm /* @var File $file */ => /** @var File $file */

nguyentamgm avatar Dec 07 '18 09:12 nguyentamgm

Thanks for adding this feature, any chance to also support Netbeans/Eclipse/Zend style? More info in this link

The reason behind this request is not having to convert hundreds of files of legacy code so they work with VSCode.

Example of Netbeans/Eclipse/Zend style: /* @var $varName Type_Name */

vlad88sv avatar Mar 11 '19 17:03 vlad88sv

Hi,

Making some traits here... $this type-hinting for traits will be a tremendous help.

Cheers!

khooz avatar Jun 30 '19 14:06 khooz

So are we just waiting for PR #701 to have a unit test or two added and then this feature is ready to go?

twinklebob avatar Oct 11 '19 22:10 twinklebob

Thanks for adding this feature, any chance to also support Netbeans/Eclipse/Zend style? More info in this link

The reason behind this request is not having to convert hundreds of files of legacy code so they work with VSCode.

Example of Netbeans/Eclipse/Zend style: /* @var $varName Type_Name */

need support :)

shushenghong avatar Dec 25 '20 00:12 shushenghong

so that PR #701 is still sitting there - 4 years now? Or did some other change make variable type hinting work (I can't get it to)

julesgilson avatar Nov 02 '23 08:11 julesgilson