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

Extbase's htmlResponse() is added wrongly #2764

Closed
brotkrueml opened this issue Dec 28, 2021 · 2 comments · Fixed by #2776 or #2812
Closed

Extbase's htmlResponse() is added wrongly #2764

brotkrueml opened this issue Dec 28, 2021 · 2 comments · Fixed by #2776 or #2812
Labels

Comments

@brotkrueml
Copy link

Bug Report

Subject Details
Rector version 6861ae9a4fe8e52dd4633101d80c9c55c1093b9e (0.12.9)

ExtbaseControllerActionsMustReturnResponseInterfaceRector inserts wrong htmlResponse().

Minimal PHP Code Causing Issue

Sorry, can't reproduce it on getrector.org/demo. But I have two issues in the mentioned rector rule in an Extbase controller:

htmlReponse() is wrapped around ForwardResponse

public function someAction(): ResponseInterface
{
    return new \TYPO3\CMS\Extbase\Http\ForwardResponse('another');
}

is changed to:

public function someAction(): ResponseInterface
{
    return $this->htmlResponse(new \TYPO3\CMS\Extbase\Http\ForwardResponse('another'));
}

htmlResponse() is added when PropagateResponseException is used

This is the case when providing a download of a file to circumvent page renderer.

 public function someAction(): ResponseInterface
{
    $response = $this->responseFactory->createResponse(200);
    throw new \TYPO3\CMS\Core\Http\PropagateResponseException($response, 200);
}

is changed to:

public function someAction(): ResponseInterface
{
    $response = $this->responseFactory->createResponse(200);
    throw new \TYPO3\CMS\Core\Http\PropagateResponseException($response, 200);
    return $this->htmlResponse();
}

Expected Behaviour

The code should be leaved untouched if a ResponseInterface is already returned. Also when a PropagateResponseException is thrown.

@brotkrueml brotkrueml added the Bug label Dec 28, 2021
sabbelasichon added a commit that referenced this issue Jan 10, 2022
sabbelasichon added a commit that referenced this issue Jan 10, 2022
@julianhofmann
Copy link

julianhofmann commented Jan 28, 2022

ExtbaseControllerActionsMustReturnResponseInterfaceRector changes also abstract methods...
(Not reproducablet on getrector.org/demo)

 -    abstract public function listAction(?Filter $filter = null, string $letter = ''): \Psr\Http\Message\ResponseInterface;
 +    public abstract function listAction(?Filter $filter = null, string $letter = ''): \Psr\Http\Message\ResponseInterface
 +    {
 +        return $this->htmlResponse();
 +    }

And violates PSR2 by changing the order of modifiers (abstract public to public abstract)

@sabbelasichon
Copy link
Owner

@julianhofmann: Sure, this is a bug. It is fixed by #2812

sabbelasichon added a commit that referenced this issue Feb 6, 2022
sabbelasichon added a commit that referenced this issue Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants