Vscode-intelephense: Intelephense considers union of DocBlock return type and function annotation

[swashata]

Describe the bug

When a function is written with return annotation, and also docblock, intelephense considers union of both.

To Reproduce

Consider the following snippet

namespace EForm\GraphQL;

/**
 * Registry for our EForm GraphQL types, inputs etc.
 */
class Registry {

    /**
     * All registry instances.
     *
     * @var array
     */
    private static $instances = [];

    /**
     * A dynamic binding to the types class for instantiating the types only
     * once.
     *
     * @param string $name Name of the class.
     * @param string $type Type of the class as present in namespace.
     *                     One of `input`, `type`, `enum` and `interface`.
     *
     * @return GraphQL\Type\Definition\Type A type from registry.
     */
    public static function get( $name, $type ) : \GraphQL\Type\Definition\Type {
        // redacted, not important
        return self::$instances[ $name ][ $type ];
    }
}

Notice, in function docblock, I have mistakenly written GraphQL\Type\Definition\Type instead of \GraphQL\Type\Definition\Type.

So now, intelephense thinks the return type of Registry::get is \GraphQL\Type\Definition\Type|\EForm\GraphQL\GraphQL\Type\Definition\Type.

Expected behavior

I understand why the above behavior is logical. But I think if a function is annotated with a return type public static function get( $name, $type ) : \GraphQL\Type\Definition\Type, intelephense should only consider the annotated return and not the docblock return.

Platform and version

• OS: MacOS Mojave: 10.14.6
• Intelephense: 1.3.1

[jfcherng]

But I think if a function is annotated with a return type public static function get( $name, $type ) : GraphQL\Type\Definition\Type, intelephense should only consider the annotated return and not the docblock return.

I think there should be a warning/error for incompatible types between type hinting and docblock rather than use an union of them. At least, this can be caught by most of static analyzers.

Sometimes you type hint it as array and the docblock has more details.

[swashata]

Sometimes you type hint it as array and the docblock has more details.

Yes, I understand that. But doesn't @return in such cases are accompanied by a description?

/**
 * Some function.
 *
 * @return array Returns an associative array with `id` and `value`.
 */
function something(): array {
  return [
    'id' => 'aa',
    'value' => 'Some value',
  ];
}

Can you think of any example where the @return in docblock and type hint in function would actually be different?

EDIT: Just to be clear, I wasn't saying that intelephense should ignore all the @return directive from DocBlock. Just the type. If there's some description, then obviously I would like to see it in intellisense. But yes, throwing an error/warning in place, instead of assuming union makes more sense.

[jfcherng]

Yes, I understand that. But doesn't @return in such cases are accompanied by a description?

You can "type hint" more details for an array such as int[] means an array of ints. int[]|bool[] means an array of ints or an array of bools so these checkings can be performed by intelephense (ideally).

[swashata]

Ah, correct 👍.

[bmewburn]

I think there should be a warning/error for incompatible types between type hinting and docblock rather than use an union of them. At least, this can be caught by most of static analyzers.

This is probably the better option. The fallback currently is to create a union if they are not equivalent/variants but in such cases it's almost certainly an error.

[bmewburn]

Tracking this one in #694