Skip to content

Support type hinting for variables #350

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

Open
filliph opened this issue Apr 11, 2017 · 25 comments
Open

Support type hinting for variables #350

filliph opened this issue Apr 11, 2017 · 25 comments

Comments

@filliph
Copy link

filliph commented Apr 11, 2017

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 :)

@felixfbecker
Copy link
Owner

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.

@filliph
Copy link
Author

filliph commented Apr 11, 2017

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
Copy link
Author

filliph commented Apr 11, 2017

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 :(

@felixfbecker
Copy link
Owner

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.

@filliph
Copy link
Author

filliph commented Apr 11, 2017

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 :)

@jens1o
Copy link
Contributor

jens1o commented Apr 12, 2017

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
Copy link
Contributor

jens1o commented Apr 12, 2017

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

@jens1o jens1o self-assigned this Apr 12, 2017
@felixfbecker
Copy link
Owner

Is this still an issue in the latest version?

@filliph
Copy link
Author

filliph commented Jun 21, 2017

@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.

@felixfbecker
Copy link
Owner

Does it work if you write it like

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

@filliph

This comment has been minimized.

@felixfbecker

This comment has been minimized.

@filliph

This comment has been minimized.

@jens1o
Copy link
Contributor

jens1o commented Jun 26, 2017

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

@felixfbecker
Copy link
Owner

yes, that's why this issue is open

@jens1o
Copy link
Contributor

jens1o commented Jun 26, 2017

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.

@str
Copy link

str commented Nov 11, 2018

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.

@nguyentamgm
Copy link

nguyentamgm commented Dec 7, 2018

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 */

@vlad88sv
Copy link

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 */

@khooz
Copy link

khooz commented Jun 30, 2019

Hi,

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

Cheers!

@twinklebob
Copy link

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

@shushenghong
Copy link

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 :)

@julesgilson
Copy link

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

11 participants