Skip to content

Conversation

@dereuromark
Copy link
Collaborator

@dereuromark dereuromark commented Oct 12, 2025

Had to fix CS too.. also fixed some phpstan deprecations.

Summary

Implements a new PHPStan rule that enforces explicit return statements for controller methods like render() and redirect() to prevent
unreachable code and make control flow more explicit.

Fixes #49

Motivation

In CakePHP controllers, methods like render() and redirect() terminate the response and prevent further code execution. However, without
an explicit return statement, it's not immediately obvious that subsequent code is unreachable, which can lead to:

  • Confusion about code execution flow
  • Wasted time debugging code that never runs
  • Maintenance issues when developers add code after these methods

Examples

Before (Problems Detected)

class ArticlesController extends Controller
{
    public function view($id)
    {
        $article = $this->Articles->get($id);

        if (!$article->published) {
            $this->redirect(['action' => 'index']);
            // This code is unreachable but not obvious!
            $this->Flash->error('Article not found');
        }

        $this->set(compact('article'));
    }

    public function edit($id)
    {
        // ...

        if ($this->request->is('ajax')) {
            $this->render('ajax_form');
            // Unreachable code that might confuse developers
            $this->set('debug', true);
        }
    }
}

PHPStan Output:

  ------ ---------------------------------------------------------------------------------------------
  Line   ArticlesController.php
  ------ ---------------------------------------------------------------------------------------------
  8      Method redirect() must be returned to prevent unreachable code.
         Use "return $this->redirect()" instead.
  18     Method render() must be returned to prevent unreachable code.
         Use "return $this->render()" instead.
  ------ ---------------------------------------------------------------------------------------------

After (Correct Usage)

  class ArticlesController extends Controller
  {
      public function view($id)
      {
          $article = $this->Articles->get($id);

          if (!$article->published) {
              // Explicit return makes it clear execution stops here
              return $this->redirect(['action' => 'index']);
          }

          $this->set(compact('article'));
      }

      public function edit($id)
      {
          // ...

          if ($this->request->is('ajax')) {
              // Clear that this terminates the action
              return $this->render('ajax_form');
          }

          // Regular flow continues here
          $this->set('title', 'Edit Article');
      }
  }

Implementation Details

  • Rule Location: src/Rule/Controller/ControllerMethodMustReturnRule.php
  • Detected Methods: render(), redirect()
  • Detection: Analyzes AST Expression nodes to find method calls that aren't wrapped in return statements
  • Scope: Only applies to classes that extend Cake\Controller\Controller
  • Default State: Enabled by default

Configuration

The rule is enabled by default. To disable it:

  # phpstan.neon
  parameters:
      cakeDC:
          controllerMethodMustReturnRule: false

@dereuromark dereuromark marked this pull request as draft October 12, 2025 10:45
@dereuromark dereuromark changed the title Rule to enforce that certain controller methods must be returned Rule to enforce that certain controller methods must "used" Oct 12, 2025
@dereuromark dereuromark marked this pull request as ready for review October 12, 2025 10:54
@dereuromark dereuromark marked this pull request as draft October 12, 2025 11:07
@rochamarcelo rochamarcelo merged commit 2c77907 into CakeDC:4.next-cake5 Oct 12, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stricter response returns validated/checked

2 participants