Skip to content
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

doc-comments are not used for autocomplete and navigation #363

Closed
TitanNano opened this issue Jan 23, 2018 · 11 comments
Closed

doc-comments are not used for autocomplete and navigation #363

TitanNano opened this issue Jan 23, 2018 · 11 comments
Labels

Comments

@TitanNano
Copy link

When I add a doc comment like this:

/**
 * @var MyClass
 *
 */
$variable = $instance->method();

Atom doesn't know what's the type of $variable. Therefore I can't get auto completion for it's properties or navigate to the class definition.

In a second step it would also be great if the return type could be determined based on @return or the functions code it self.

@Gert-dev
Copy link
Owner

You were close, but the syntax that currently works should be:

/** @var MyClass $variable */
$variable = $instance->method();

The reason is that this comment can basically appear anywhere, for example:

/** @var MyClass $variable */
some_call();

$variable = $instance->method();

What also currently still works is /** @var $variable MyClass */, but I advise against using it because it is inconsistent with the syntax used in docblocks.

For your second question: do you mean that the return type of the method is used? This should already be the case, i.e.:

class Foo
{
    /**
     * @return Foo
     */
    public function bar()
    {
        return $this;
    }
}

/** @var A $a */
$a->bar()-> // Should still show "bar".

@TitanNano
Copy link
Author

I will give /** @var Type $var **/ a try tomorrow, but i want to note here that phpdoc.org states the following:

The @var tag MUST contain the name of the element it documents. An exception to this is when property declarations only refer to a single property.

$a->bar()-> // Should still show "bar".

Yes this works, I will get all the properties of the return type of bar()

What I mean is:

$var = $a->bar();

Now the type of $var is unknown.

@Gert-dev
Copy link
Owner

Gert-dev commented Jan 25, 2018

You are right. Strictly speaking, @var tags may omit the name of the property if it refers to a single property.

What's strange is that I'm certain that the documentation of phpdoc at some point mentioned explicitly that it did not concern itself with so-called "inline docblocks", but they have recently renewed their documentation and now it explicitly does seem to mention them and even includes the example you gave here.

However, later on, on the page for the var tag, the documentation conflicts a bit in the sense that it indeed says

The @var tag MUST contain the name of the element it documents. An exception to this is when property declarations only refer to a single property. In this case the name of the property MAY be omitted.

... though it does mention "a single property", to which inline docblocks can never refer - but I admit that is being a bit pedantic.

It sounds possible to support this in the core, but I'm wondering if it is worth it to shave off a few characters in a scenario where there is no ambiguity, for example:

<?php
// Impossible to omit name here, could refer to $foo or $bar otherwise
/** @var Type */
foreach ($foo as $bar) {

}

// Same
/** @var Type */
$a = $b->test();

// Same
/** @var Type */
$a = 5; $b = 3;

Regarding your last example, it should work as well, but there is a case in which it doesn't - due to a php-parser technical limitation IIRC:

$var = $a->bar();

$var-> // Proper autocompletion.

$someOtherCode = 5;

$var-> // Oops! Doesn't work.

if (true) {

}

Is this the case you are referring to, by any chance?

@TitanNano
Copy link
Author

I don't really mind writing my doc comments the verbose way.
The actual problem I'm facing is that coworkers with other IDEs (e.g. PHPStorm) are mostly fine with the short /** @var Type */ comments, and when I'm viewing that code I'm left in the dark about the types.

@Gert-dev
Copy link
Owner

Gert-dev commented Jan 26, 2018

Argh, I understand your pain. I have the fortune of coworkers with PhpStorm that default to the verbose way 😄.

@Patrick-R
Copy link

A bit similar: is there support for autocompletion on method chaining or are there any plans to add this as a feature?

@Gert-dev
Copy link
Owner

@Patrick-R Yes. That should already be supported:

class Foo
{
    public function bar(): self {}
}

$foo = new Foo();
$foo->bar()->bar()->bar()-> // Autocompletion.

@Patrick-R
Copy link

@Gert-dev Seems to work fine in simple cases like that, but I can't get it to work reliably in a Drupal 8 project no matter what I try. :-( Autocompletion on first level works most of the time, but no luck at all when trying to chain calls. Not even using a temporary variable for the return value from the first really helps.

@tortuetorche
Copy link

tortuetorche commented Mar 16, 2018

Hi,

Since the version 3.2.0, navigation in DocBlocks comments doesn't work anymore for me with the last versions of Atom (1.23, 1.24, 1.25) on both Windows 10 and Ubuntu 16.04

Cheers,
Tortue Torche

@Gert-dev
Copy link
Owner

Gert-dev commented Mar 16, 2018

@tortuetorche Actually that already happened in core 3.1.0. I wanted to get that release out the door ASAP because the Atom package was in a completely botched state. As such, I did not add it back yet, but there is a ticket to track that.

In short: it's in the backlog. Pull requests are welcome, as always, though.

@Gert-dev
Copy link
Owner

Not sure why I left this open, I'm closing this since there is a ticket in the server to support navigation in docblocks (see my previous comment). There isn't really anything this client package can do, especially now that #460 is around the corner.

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

No branches or pull requests

4 participants