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

Improved closure parameters #209

Open
UFOMelkor opened this issue Sep 23, 2022 · 3 comments
Open

Improved closure parameters #209

UFOMelkor opened this issue Sep 23, 2022 · 3 comments
Labels
status: help wanted Backlog type: improvement The issue is about some basic functionality delivered earlier to make it closer to perfect

Comments

@UFOMelkor
Copy link

I don't know whether this is out of scope for this plugin, but it is AFAIK currently the only one detecting the type of untyped closure parameters. I add a little script further below as an example.

To show what I am talking about at first, let me show a little screenshot.
image
The plugin automatically detects that the argument of the closure must be of type WordBuilder, provides autocompletion and allows Go To Declaration or Usages.

IMHO, four things are missing to complete this feature.

  1. Support Find usage. As you can see, only the usages of ok() are found but not for withLetter()
    image
  2. Support renaming. Currently, this refactoring is not provided on the autocompleted methods.
  3. Do not get distracted by outer variables. While autocompletion works in this case
    image
    it does not work in this one, because the name of the outer variable is the same as the argument one.
    image
  4. Detect closure arguments within closure arguments. While ->withWord() is detected, withLetter() is not.
    image

Without deeper knowledge, I would guess most of this would be possible with a TypeProvider.

<?php

declare(strict_types=1);

class WordBuilder
{
    private string $letters = "";

    public function withLetter(string $letter): self
    {
        $this->letters .= $letter;
        return $this;
    }

    public function ok(): string
    {
        return $this->letters;
    }
}

class SentenceBuilder
{
    /** @psalm-var list<string> */
    private array $words = [];

    /**
     * @psalm-param callable(WordBuilder): WordBuilder $mod
     */
    public function withWord(callable $mod): self
    {
        $this->words[] = $mod(new WordBuilder())->ok();
        return $this;
    }

    public function ok(): string
    {
        return implode(' ', $this->words) . '.';
    }
}

class Test
{

    public function test_with_and_without_nested_builders(): void
    {
        echo $this->givenAWord(fn($word) => $word->withLetter("T")->withLetter("e")->withLetter("s")->withLetter("t")) . "\n";
        echo $this->givenASentence(
            fn($aSentence) => $aSentence
                ->withWord(fn($word) => $word->withLetter("T")->withLetter("h")->withLetter("i")->withLetter("s"))
                ->withWord(fn($word) => $word->withLetter("i")->withLetter("s"))
                ->withWord(fn($word) => $word->withLetter("a"))
                ->withWord(fn($word) => $word->withLetter("T")->withLetter("e")->withLetter("s")->withLetter("t"))) . "\n";
    }

    public function test_with_equal_named_variable(): void
    {
        $word = $this->givenAWord(fn($word) => $word->withLetter("T")->withLetter("e")->withLetter("s")->withLetter("t")) . "\n";
        echo $word;
    }

    /**
     * @psalm-param callable(SentenceBuilder): SentenceBuilder $mod
     */
    private function givenASentence(callable $mod): string
    {
        return $mod(new SentenceBuilder())->ok();
    }

    /**
     * @psalm-param callable(WordBuilder): WordBuilder $mod
     */
    private function givenAWord(callable $mod): string
    {
        return $mod(new WordBuilder())->ok();
    }
}

(new Test())->test_with_and_without_nested_builders();
(new Test())->test_with_equal_named_variable();
@klesun
Copy link
Owner

klesun commented Sep 23, 2022

Like you correctly guessed, the 1 and 2 can be implemented with TypeProvider. Sadly, the way plugin is designed, using TypeProvider is not possible without introducing severe performance issues: #33 (comment)

The 3 and 4 would be good doable improvements though. The heuristic resolution of arrow functions was not polished much. I don't have time to implement those improvements, but I can provide guidance in filing an MR.

The arrow functions are handled somewhere around here:

@klesun klesun added status: help wanted Backlog type: improvement The issue is about some basic functionality delivered earlier to make it closer to perfect labels Sep 23, 2022
@UFOMelkor
Copy link
Author

I experimented a little with a TypeProvider. I'm neither a Java/Kotlin expert nor have I written a plugin yet; therefore, the result is definitely not ready for any production like usage and might include multiple errors or unwanted side effects. For my current project, the providers helps with all four problems. I'll try to look deeper into it in the next weeks, but in the event that I'm distracted, I put my current code here.

import com.intellij.openapi.project.IndexNotReadyException
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiElement
import com.jetbrains.php.PhpIndex
import com.jetbrains.php.lang.psi.elements.Function
import com.jetbrains.php.lang.psi.elements.Method
import com.jetbrains.php.lang.psi.elements.MethodReference
import com.jetbrains.php.lang.psi.elements.Parameter;
import com.jetbrains.php.lang.psi.elements.ParameterList
import com.jetbrains.php.lang.psi.elements.PhpExpression
import com.jetbrains.php.lang.psi.elements.PhpNamedElement;
import com.jetbrains.php.lang.psi.resolve.types.PhpType;
import com.jetbrains.php.lang.psi.resolve.types.PhpTypeProvider4;
import java.util.Collections

class ClosureArgumentsProvider : PhpTypeProvider4 {
    override fun getKey(): Char {
        return '';
    }

    override fun getType(element: PsiElement?): PhpType? {
        if (element !is Parameter) {
            return null;
        }
        val params = element.parent;
        val func = params.parent;
        val expression = func.parent;
        val methodReference = expression.parent.parent;

        if (params !is ParameterList || func !is Function || ! func.isClosure || expression !is PhpExpression || methodReference !is MethodReference) {
            return null;
        }

        if (element.typeDeclaration != null || element.docTag != null) {
            return null;
        }

        val signature = methodReference.signature;
        return PhpType().add("#" + this.key + signature);
    }

    private fun completeFromElement(method: Method): PhpType? {
        val type = PhpType().add(method.getParameter(0)?.type);
        val types = type.typesWithParametrisedParts.filter { name -> name != "\\callable" };
        val collectedTypes: MutableList<String> = ArrayList();
        val callableRegex = Regex("\\\\callable<#ᤓᤓ([^,]+),([^>]+)>");
        types.forEach { eachType ->
            val match = callableRegex.matchEntire(eachType);
            if (match != null) {
                collectedTypes.add(match.groupValues.elementAt(1));
            } else {
                return null;
            }
        }
        if (collectedTypes.isEmpty()) {
            return null;
        }
        val newType = PhpType();
        collectedTypes.forEach { each -> newType.add(each) }
        return newType;
    }

    override fun complete(signature: String?, project: Project?): PhpType? {
        if (project == null || signature == null) {
            return null;
        }
        if (signature.indexOf("#" + this.key) != 0) {
            return null;
        }
        val phpIndex = PhpIndex.getInstance(project);
        val sig = signature.substringAfter(this.key);

        try {
            val bySignature = phpIndex.getBySignature(sig);
            if (bySignature.isEmpty()) {
                val elements: List<Collection<PhpNamedElement>> = sig.substringAfter("#" + this.key).split("|").map { each -> phpIndex.getBySignature(each) };
                val flatElements: Collection<Method> = elements.flatten().filterIsInstance<Method>();
                val result = PhpType();
                flatElements.map { each -> this.completeFromElement(each) }.forEach { each -> result.add(each) };
                if (result.size() > 0) {
                    return result;
                }
                return null;
            }
            val firstElement = bySignature.elementAt(0);
            if (firstElement !is Method) {
                return null;
            }
            return this.completeFromElement(firstElement);
        } catch (e: IndexNotReadyException) {
            return null;
        }
    }

    override fun getBySignature(
        signature: String?,
        visited: MutableSet<String>?,
        debth: Int,
        project: Project?
    ): MutableCollection<out PhpNamedElement> {
        return Collections.emptyList();
    }
}

@klesun
Copy link
Owner

klesun commented Sep 25, 2022

Cool. Enabling TypeProvider can not be merged into the main branch due to reasons I mentioned above, but you can create a fork of the plugin with TypeProvider enabled if you think someone may benefit from it like your project did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted Backlog type: improvement The issue is about some basic functionality delivered earlier to make it closer to perfect
Projects
None yet
Development

No branches or pull requests

2 participants