Repository navigation
Issue/471 middleware hook refactor - #476
Conversation
…el and remove temp email files
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
📝 WalkthroughWalkthroughMiddleware return contracts and controller/middleware templates are changed to explicitly return Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (HTTP)
participant Adapter as WebAppAdapter
participant MiddlewareMgr as MiddlewareManager
participant Middleware as QtMiddleware*
participant Router as Router/Dispatcher
participant Response as Response
participant Request as Request
rect rgba(200,200,255,0.5)
Client->>Adapter: HTTP request
Adapter->>Request: create/request() (DI/global)
Adapter->>MiddlewareMgr: applyMiddlewares(Request, Response)
end
rect rgba(200,255,200,0.5)
MiddlewareMgr->>Middleware: invoke current middleware (pointer advanced)
Middleware->>MiddlewareMgr: return Response (may short-circuit)
MiddlewareMgr->>MiddlewareMgr: continue chain (callback returns Response)
MiddlewareMgr-->>Adapter: Response
end
rect rgba(255,200,200,0.5)
Adapter->>Request: refresh via request() helper
Adapter->>Router: dispatch(Request, Response)
Router-->>Client: Response (returned)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #476 +/- ##
============================================
+ Coverage 82.97% 83.20% +0.23%
Complexity 2905 2905
============================================
Files 251 251
Lines 7663 7663
============================================
+ Hits 6358 6376 +18
+ Misses 1305 1287 -18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/softberg/quantum-php-core/blob/484f32195c031d1c39c64d7cb104214f1d01d1f4/src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl#L85
signin() now declares : Response, but the GET branch only calls $response->html(...) and falls through without returning anything. In a generated DemoWeb app, a normal GET /signin request will therefore end with a runtime TypeError (“must be of type Response, none returned”) instead of rendering the page. This same refactor pattern appears in multiple updated templates, so all non-redirect branches need to explicitly return the Response object.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
src/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tpl (2)
17-20:⚠️ Potential issue | 🔴 CriticalMissing
Responseimport.The
createanddeletemethods declare a: Responsereturn type (Lines 52, 67), butQuantum\Http\Responseis not imported in this file. As written, the template will reference an undefinedResponseclass in the{{MODULE_NAMESPACE}}\Controllersnamespace, producing a generated module that fails to load.🔧 Proposed fix
use {{MODULE_NAMESPACE}}\Services\CommentService; use {{MODULE_NAMESPACE}}\DTOs\CommentDTO; +use Quantum\Http\Response; use Quantum\Http\Request;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tpl` around lines 17 - 20, The template for CommentController is missing the import for Quantum\Http\Response causing the declared return types on create() and delete() to resolve to an undefined class; add a "use Quantum\Http\Response;" import at the top of the file (alongside the existing use statements) so the create and delete methods in the CommentController template resolve correctly.
52-73:⚠️ Potential issue | 🔴 CriticalReturn type
Responseis declared but noResponseis actually returned.Both
create()(lines 52–60) anddelete()(lines 67–73) declare: Response, yet each ends withredirect(get_referrer())and no explicit return statement. Theredirect()helper returnsvoid, and although it callsstop()which throwsStopExecutionException, the declared return type contracts of these methods are not satisfied. They either need to return aResponseobject, declare avoidorneverreturn type, or the: Responsedeclaration should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tpl` around lines 52 - 73, The declared return type Response on CommentController::create and CommentController::delete is incorrect because neither returns a Response; update each method to either return a proper Response object (e.g., convert the redirect call into/return a Response instance created by your framework) or change the signature to a compatible type (void or never) and remove : Response; locate the methods named create(Request $request, ?string $lang, string $uuid): Response and delete(?string $lang, string $uuid): Response and make the change consistently (return a Response from redirect(get_referrer()) or adjust the method return types).src/Module/Templates/DemoApi/src/Controllers/AccountController.php.tpl (1)
49-100:⚠️ Potential issue | 🔴 CriticalCatch blocks fail the declared
: Responsecontract — will throwTypeErroron everyAuthException.Both
update()andupdatePassword()declare: Response, but thecatch (AuthException $e)branches call$response->json(...)withoutreturn. Whenever anAuthExceptionis thrown (e.g., auth/update failure), PHP will raiseTypeError: AccountController::update(): Return value must be of type Response, none returned, masking the original error and producing a 500 for what should be a handled 400-level response.Also, the
} catch (...)lines are indented inconsistently with the openingtry {.🐛 Proposed fix
return $response->json([ 'status' => self::STATUS_SUCCESS, 'message' => t('common.updated_successfully') ]); - } catch (AuthException $e) { - $response->json([ + } catch (AuthException $e) { + return $response->json([ 'status' => self::STATUS_ERROR, 'message' => $e->getMessage() ]); } }And the same change inside
updatePassword()’s catch block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/AccountController.php.tpl` around lines 49 - 100, The catch blocks in AccountController::update and AccountController::updatePassword call $response->json(...) but do not return it, violating the declared ": Response" return type and causing a TypeError when an AuthException occurs; fix both catch blocks to return the $response->json(...) value (i.e., add "return" before $response->json(...)) and correct the catch indentation to align with the corresponding try blocks so the method returns a Response in all paths.src/Module/Templates/Toolkit/src/Controllers/DatabaseController.php.tpl (1)
87-123:⚠️ Potential issue | 🟡 Minor
create(),update(), anddelete()declare: Responsebut never return one.All three redirect-only actions rely on
redirect(...)to terminate execution. This is only safe if the helper always throws; otherwise PHP will raiseTypeError: ... Return value must be of type Response, none returned. For consistency across the refactor, either remove the: Responseannotation from redirect-only actions or have them return theResponseproduced by the redirect helper.(Same pattern as
EmailsController::delete()andDemoWeb\AccountController::{update,updatePassword}()in this PR — pick one convention and apply uniformly across templates.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/Toolkit/src/Controllers/DatabaseController.php.tpl` around lines 87 - 123, The create(), update(), and delete() methods in DatabaseController.php.tpl declare a Response return type but never return one; update these methods (create, update, delete) to return the Response produced by the redirect(...) helper (or remove the ": Response" annotation if you prefer the other convention used elsewhere) so the signature matches behavior—specifically change each method to return the redirect(...) call instead of just calling redirect(...), ensuring the returned value is the expected Response from the redirect helper.src/Module/Templates/DemoWeb/src/Controllers/PostController.php.tpl (1)
88-113:⚠️ Potential issue | 🔴 Critical
post()never returns on the success path and has unreachablestop()after the 404 return.Two problems here:
- Line 96 — unreachable code.
stop();immediately afterreturn $response->html(..., StatusCode::NOT_FOUND);can never execute. It should be removed.- Line 112 — missing
return. The success branch calls$response->html($this->view->render('post/single'));without returning it. Since the method is declared: Response, every successful request will throwTypeError: PostController::post(): Return value must be of type Response, none returned.Also note the
}on line 97 is dedented relative to the openingifblock.🐛 Proposed fix
if ($post->isEmpty()) { return $response->html(partial('errors/404'), StatusCode::NOT_FOUND); - stop(); - } + } $commentService = service(CommentService::class); @@ - $response->html($this->view->render('post/single')); + return $response->html($this->view->render('post/single')); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/PostController.php.tpl` around lines 88 - 113, The post() method in PostController has an unreachable stop() after the 404 return and fails to return a Response on the success path; remove the dead call to stop(), fix the mis-indented closing brace of the if-block, and ensure the success branch returns the Response by changing the final line to return $response->html($this->view->render('post/single')); so PostController::post() always returns a Response object.src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl (1)
62-170:⚠️ Potential issue | 🔴 Critical
create(),amend(),delete(), anddeleteImage()never return — guaranteedTypeErroron every successful request.All four methods declare
: Responsebut end with$response->json([...]);without areturn. There is no exception on the success path, so PHP will raiseTypeError: PostManagementController::<method>(): Return value must be of type Response, none returnedfor every successful call. OnlymyPosts()was updated correctly.The
}dedents at lines 72, 101, 133, and 155 are also inconsistent with their openingifblocks.🐛 Proposed fix
- $response->json([ + return $response->json([ 'status' => 'success', 'message' => t('common.created_successfully'), 'data' => current($this->postService->transformData([$post])) ]); }Apply the same
returnprefix to the final$response->json([...])statements inamend()(line 114),delete()(line 137), anddeleteImage()(line 166), and fix the}indentation on lines 72, 101, 133, and 155.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl` around lines 62 - 170, The four controller actions create(), amend(), delete(), and deleteImage() declare a Response return but never return one (they call $response->json(...) without returning it) and several closing braces are mis-indented; fix by prepending return to the final $response->json(...) calls in create(), amend(), delete(), and deleteImage() so each method returns the Response, and correct the misplaced/dedented closing braces for the if blocks (the hasFile('image') checks in create() and amend(), and the if ($post->image) checks in amend(), delete(), and deleteImage()) so each if/brace pair is properly matched and indented.src/Module/Templates/DemoApi/src/Controllers/PostController.php.tpl (1)
84-111:⚠️ Potential issue | 🔴 Critical
post()success branch never returns, andstop()after the 404 return is unreachable.Same issue as the DemoWeb template:
- Line 94 — unreachable code.
stop();afterreturn $response->json(...)can’t be reached and should be removed.- Line 107 — missing
return. The success path calls$response->json([...])without returning it; with: Responsedeclared, every successful request will raiseTypeError: PostController::post(): Return value must be of type Response, none returned.Also the closing
}on line 95 is dedented relative to the openingif ($post->isEmpty()) {.🐛 Proposed fix
if ($post->isEmpty()) { return $response->json([ 'status' => 'error', 'message' => t('common.post_not_found') ], StatusCode::NOT_FOUND); - - stop(); - } + } $postData = current($this->postService->transformData([$post])); @@ - $response->json([ + return $response->json([ 'status' => 'success', 'data' => $postData, ]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/PostController.php.tpl` around lines 84 - 111, In PostController::post(), remove the unreachable stop() call after the 404 return and fix the misplaced closing brace so the if-block is properly closed; then ensure the success branch returns the Response by adding a return before the final $response->json([...]) call (so post() always returns a Response and no unreachable code remains).src/Module/Templates/Toolkit/src/Controllers/EmailsController.php.tpl (1)
80-85:⚠️ Potential issue | 🟡 Minor
delete()declares: Responsebut calls a method that throws instead of returning.
delete()declares: Responsebut only callsredirect(...), which returnsvoidand unconditionally throwsStopExecutionException. This means the method never returns normally and will never produce aResponseobject. While this avoids a runtimeTypeError(because the exception terminates execution), the return-type annotation is misleading to readers and static analyzers.Either refactor the method to return
void, or consider using: never(PHP 8.1+) if the intent is to document that this method terminates execution rather than returning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/Toolkit/src/Controllers/EmailsController.php.tpl` around lines 80 - 85, The delete() method declares a Response return type but calls redirect(...) which throws StopExecutionException and never returns; update the method signature to reflect that it does not return (either change public function delete(string $emailId): Response to public function delete(string $emailId): void if you want no return, or to public function delete(string $emailId): never if you want to document that it always terminates via redirect/StopExecutionException) and ensure any callers or docblocks are adjusted accordingly; locate the method by the delete(string $emailId) declaration and the redirect(...) call to apply the change.src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl (1)
36-54:⚠️ Potential issue | 🔴 CriticalCritical: missing
returnoncatch/elsebranches — exception paths will throwTypeError.Success paths correctly use
return $response->json(...), but several error/alternate branches still call$response->json(...)as a statement:
signinLine 45 (insidetrybefore thecatch) and Line 49 (catchbranch) — neither returns.signoutLine 83 (elsebranch) — no return.verifyLine 165 (catchbranch) — no return.resendLine 184 (catchbranch) — no return.With
: Responsedeclared, whenever one of these branches is hit the method falls through to the end and PHP throws:TypeError: ...::signin(): Return value must be of type Quantum\Http\Response, none returnedEffectively, every API error response (auth failure, unauthorized signout, OTP failure, resend failure) is replaced with a 500 TypeError. Please add
returnto each of these$response->json(...)calls.🛠️ Proposed fix (applies identically to the other flagged branches)
public function signin(Request $request, Response $response): Response { try { $code = auth()->signin($request->get('email'), $request->get('password')); if (filter_var(config()->get('auth.two_fa'), FILTER_VALIDATE_BOOLEAN)) { $response->set('code', $code); } - $response->json([ + return $response->json([ 'status' => self::STATUS_SUCCESS ]); } catch (AuthException $e) { - $response->json([ + return $response->json([ 'status' => self::STATUS_ERROR, 'message' => $e->getMessage() ], StatusCode::UNPROCESSABLE_ENTITY); } }Also applies to: 76-88, 156-170, 176-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl` around lines 36 - 54, Several branches in AuthController methods (signin, signout, verify, resend) call $response->json(...) without returning it, which breaks the declared ": Response" return type and causes TypeError; update each non-returning call inside signin (try before catch and the catch block), signout (the else branch), verify (catch block), and resend (catch block) to "return $response->json(...)" so the method always returns a Response instance — search for the methods signin(), signout(), verify(), resend() and replace those standalone $response->json(...) statements with returned values.src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl (1)
84-101:⚠️ Potential issue | 🔴 CriticalCritical:
create,amend,delete,deleteImagedeclare: Responsebut never return one.The view-rendering actions (
myPosts,createFrom,amendForm) correctlyreturn $response->html(...), but the four side-effect actions end with only aredirect(...)call and noreturn. Theredirect()helper returnsvoid, so these methods violate their: Responsereturn type declaration, throwing at runtime:TypeError: ...::create(): Return value must be of type Quantum\Http\Response, none returnedEvery scaffolded project will fail on post-create, amend, delete, and delete-image flows. After the
redirect()call, explicitlyreturn response();to satisfy the return type on each of these four methods.Proposed fix for `create` (apply same pattern to `amend`, `delete`, `deleteImage`)
public function create(Request $request): Response { ... $this->postService->addPost($postDto); redirect(base_url(true) . '/' . current_lang() . '/my-posts'); + return response(); }Also applies to: 131-154, 161-172, 179-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl` around lines 84 - 101, The create, amend, delete, and deleteImage methods declare a Response return type but only call the redirect helper (which returns void), causing a TypeError; after the redirect(...) call in each of PostManagementController::create, ::amend, ::delete, and ::deleteImage, explicitly return the framework response object (call response() and return it) so the method satisfies its : Response signature.src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl (1)
65-208:⚠️ Potential issue | 🔴 CriticalCritical:
: Responsereturn type declared but methods never return anything — will causeTypeErrorat runtime.All eight action methods (
signin,signout,signup,activate,forget,reset,verify,resend) are declared: Response, but none actually return aResponseobject. Theredirect()helper returnsvoid(it only causes a side effect), and$response->html(...)is called without being returned. This causes the methods to implicitly returnnull, violating the return type contract:TypeError: ...::signin(): Return value must be of type Quantum\Http\Response, null returnedFix: Add
return $response;after eachredirect()call, and change$response->html(...)toreturn $response->html(...)Example fix for `signin` (apply same pattern to all eight methods)
public function signin(Request $request, Response $response): Response { if ($request->isMethod('post')) { try { $code = auth()->signin($request->get('email'), $request->get('password'), !!$request->get('remember')); if (filter_var(config()->get('auth.two_fa'), FILTER_VALIDATE_BOOLEAN)) { redirect(base_url(true) . '/' . current_lang() . '/verify/' . $code); } else { redirect(base_url(true) . '/' . current_lang()); } } catch (AuthException $e) { session()->setFlash('error', $e->getMessage()); redirect(base_url(true) . '/' . current_lang() . '/signin'); } + return $response; } else { $this->view->setParams([ 'title' => t('common.signin') . ' | ' . config()->get('app.name'), ]); - $response->html($this->view->render(self::VIEW_SIGNIN)); + return $response->html($this->view->render(self::VIEW_SIGNIN)); } }Minor: Indentation of
} else {and} catch {blocks (lines 73, 112, 143, 162, 183, 205) is broken. These closing braces are at 4-space indent instead of 8-space, misaligning with method body structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl` around lines 65 - 208, All action methods (signin, signout, signup, activate, forget, reset, verify, resend) declare ": Response" but currently don't return a Response; change every redirect(...) call to be followed by "return $response;" and change calls that render HTML like "$response->html(...)" to "return $response->html(...)" (apply consistently inside signin, signout, signup, activate, forget, reset, verify, resend), and also fix the misaligned indentation of the "} else {" and "} catch {" closing braces so they align with their corresponding if/catch blocks in those same methods.
🧹 Nitpick comments (11)
src/App/Adapters/WebAppAdapter.php (1)
85-92: Use the local$requestconsistently.Now that
$request = request();is captured on line 86, line 88 should prefer$request->getUri()instead of callingrequest()again, matching how$requestis used on line 92. Purely a consistency/readability nit — functionally equivalent becauserequest()returns the DI singleton.Proposed fix
$response = (new MiddlewareManager($matchedRoute))->applyMiddlewares(request(), response()); $request = request(); - if ($this->setupViewCache()->serveCachedView(request()->getUri() ?? '', $response)) { + if ($this->setupViewCache()->serveCachedView($request->getUri() ?? '', $response)) { stop(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/Adapters/WebAppAdapter.php` around lines 85 - 92, Replace the second direct call to the global request() with the already-captured local $request for consistency: use $request->getUri() when calling $this->setupViewCache()->serveCachedView(...). Keep the rest of the method unchanged so MiddlewareManager($matchedRoute) and RouteDispatcher()->dispatch($matchedRoute, $request) continue to use $matchedRoute and the local $request consistently.src/Module/Templates/DemoWeb/src/Controllers/BaseController.php.tpl (1)
49-51: Indentation ofifblock closing brace is off.Line 49's
}closes theif (request()->isMethod('get'))block but sits at the same indent level as the method's closing brace on line 50, making the block structure visually ambiguous. Recommend indenting line 49 to 8 spaces.Proposed fix
$this->view->setParam('langs', config()->get('lang.supported')); - } + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/BaseController.php.tpl` around lines 49 - 51, The closing brace for the if block checking request()->isMethod('get') is mis-indented and sits at the same level as the method's closing brace; fix by moving the `}` that closes the `if (request()->isMethod('get'))` block to 8 spaces (one additional indentation level) so it aligns with the inner block scope inside the method in BaseController.php.tpl and preserves clear block structure for that `if` inside the method.src/Module/Templates/DemoWeb/src/Middlewares/Update.php.tpl (1)
30-35: Missing@return Responsein PHPDoc.Other middleware templates in this PR (Editor, Forget, Auth, Activate) all added
@return Responsealongside the new return type. Align this one for consistency.Proposed fix
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Update.php.tpl` around lines 30 - 35, The PHPDoc for the apply method in the Update middleware is missing the `@return` Response annotation; update the docblock above public function apply(Request $request, Response $response, Closure $next): Response to include "@return Response" so it matches the other middleware templates (Editor, Forget, Auth, Activate) and reflects the declared return type.src/Module/Templates/DemoApi/src/Middlewares/Update.php.tpl (1)
30-35: Add@return Responseto PHPDoc for consistency.Sibling templates updated in this PR (
Verify.php.tpl,Resend.php.tpl) include@return Responsein their PHPDoc. Adding it here keeps docblocks consistent across the middleware templates.📝 Proposed doc tweak
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Middlewares/Update.php.tpl` around lines 30 - 35, The PHPDoc for the apply method is missing a `@return` tag; update the docblock above the apply(Request $request, Response $response, Closure $next): Response method in Update.php.tpl to include "@return Response" so it matches siblings like Verify.php.tpl and Resend.php.tpl and keeps the middleware templates consistent.src/Module/Templates/DemoApi/src/Middlewares/Comment.php.tpl (1)
29-34: Add@return Responseto PHPDoc for consistency.Sibling middleware templates in this PR (
Verify.php.tpl,Signup.php.tpl,Resend.php.tpl) document@return Responsein their PHPDoc. Consider adding it here for consistency.📝 Proposed doc tweak
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Middlewares/Comment.php.tpl` around lines 29 - 34, The PHPDoc for the apply method in Comment.php.tpl is missing the `@return` Response tag; update the docblock above public function apply(Request $request, Response $response, Closure $next): Response to include "@return Response" so it matches sibling middleware templates (e.g., Verify.php.tpl, Signup.php.tpl, Resend.php.tpl) for consistency.src/Module/Templates/DemoApi/src/Middlewares/Password.php.tpl (1)
31-36: Minor: PHPDoc missing@return Response.Same observation as the DemoWeb
Password.php.tpl: other templates updated in this PR also refreshed the PHPDoc@return. Worth keeping it consistent.📝 Proposed doc update
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Middlewares/Password.php.tpl` around lines 31 - 36, The PHPDoc for the Password middleware's apply method is missing a `@return` annotation; update the docblock above public function apply(Request $request, Response $response, Closure $next): Response to include "@return Response" so the PHPDoc matches the method signature and stays consistent with other templates (see Password.php.tpl and method apply).src/Module/Templates/DemoWeb/src/Middlewares/Comment.php.tpl (1)
29-34: Minor: PHPDoc missing@return Response.For consistency with other middleware templates updated in this PR, consider adding
@return Responseto the PHPDoc block.📝 Proposed doc update
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Comment.php.tpl` around lines 29 - 34, The PHPDoc for the middleware method apply is missing the `@return` Response tag; update the docblock above the public function apply(Request $request, Response $response, Closure $next): Response (in the Comment middleware template) to include "@return Response" so the docblock matches the method signature and other middleware templates.src/Module/Templates/DemoWeb/src/Middlewares/Password.php.tpl (1)
32-37: Minor: PHPDoc missing@return Response.Other middleware templates updated in this PR (e.g.,
Resend.php.tpl,Signup.php.tpl,PostOwner.php.tpl) also added a@return Responseline to the PHPDoc block for consistency with the new return type. Consider adding it here too.📝 Proposed doc update
/** * `@param` Request $request * `@param` Response $response * `@param` Closure $next + * `@return` Response */ public function apply(Request $request, Response $response, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Password.php.tpl` around lines 32 - 37, Add a `@return` Response tag to the PHPDoc of the apply method so the docblock matches the method signature; update the PHPDoc above public function apply(Request $request, Response $response, Closure $next): Response to include "@return Response" similar to the other middleware templates (e.g., Resend.php.tpl, Signup.php.tpl, PostOwner.php.tpl).src/Middleware/MiddlewareManager.php (1)
56-68:applyMiddlewares()is not reentrant due to stateful array-pointer iteration.The chain relies on the internal array pointer of
$this->middlewares(current()/next()), which mutates instance state during traversal. A few consequences worth considering:
- Calling
applyMiddlewares()a second time on the sameMiddlewareManagerinstance (e.g., in tests, retry logic, or a cached pipeline) will not re-run the chain because the pointer is left at the end. Areset($this->middlewares)at the top (or tracking an explicit index) would make this safe.- If a middleware invokes
$next(...)more than once, each invocation advances the pointer further instead of re-entering from the same position — so repeated$nextcalls skip middlewares rather than re-execute the remaining chain.if (!current($this->middlewares))also terminates the chain for any falsy middleware entry (e.g.,""or"0"), not just end-of-array.key($this->middlewares) === null(or a dedicated index) would be safer.None of these fire on the normal happy path, but the “pure PSR-15 onion model” goal is easier to uphold with an index/array-walk approach than with the internal pointer.
♻️ Suggested alternative using an explicit index
- public function applyMiddlewares(Request $request, Response $response): Response - { - if (!current($this->middlewares)) { - return $response; - } - - $currentMiddleware = $this->getMiddleware($request, $response); - next($this->middlewares); - - return $currentMiddleware->apply($request, $response, function (Request $request, Response $response): Response { - return $this->applyMiddlewares($request, $response); - }); - } + public function applyMiddlewares(Request $request, Response $response, int $index = 0): Response + { + if (!isset($this->middlewares[$index])) { + return $response; + } + + $currentMiddleware = $this->getMiddleware($this->middlewares[$index], $request, $response); + + return $currentMiddleware->apply($request, $response, function (Request $request, Response $response) use ($index): Response { + return $this->applyMiddlewares($request, $response, $index + 1); + }); + }(
getMiddlewarewould need a small tweak to accept the class suffix instead of readingcurrent($this->middlewares).)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Middleware/MiddlewareManager.php` around lines 56 - 68, applyMiddlewares currently uses the array internal pointer (current()/next()) on $this->middlewares which makes the method non-reentrant and causes incorrect behavior if middlewares call $next() multiple times or if falsy entries exist; change traversal to use an explicit integer index (e.g., $index) passed through the closure instead of current()/next(), update applyMiddlewares to reset/start from index 0 and call getMiddleware($index) (modify getMiddleware to accept an index or key) to retrieve the middleware, and use the closure to invoke $this->applyMiddlewares($request, $response, $index + 1) so the pointer state is not mutated and falsy array values don’t prematurely terminate the chain.tests/Unit/Middleware/MiddlewareManagerTest.php (1)
60-64: UnusedMockery::close()intearDown().No test in this suite uses Mockery, so the
Mockery::close()call and theuse Mockery;import (line 55) are dead code. Either add actual Mockery usage where appropriate or drop the teardown override and import to keep the file tight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Middleware/MiddlewareManagerTest.php` around lines 60 - 64, The test file contains an unused Mockery import and a dead Mockery::close() call in the tearDown() override; remove the Mockery import and either delete the tearDown() method override (or at minimum remove the Mockery::close() call) so the class no longer references Mockery, or alternatively add real Mockery-based mocks in the test methods and keep Mockery::close() in tearDown(); look for the tearDown() method and the top-level use Mockery; declaration to apply the fix.src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl (1)
68-68: Possible typo:createFromlikely should becreateForm.The action displays the post creation form (it renders
'post/form'), so the method name reads as a typo (createFrom→createForm). This isn't introduced by this PR, but since the signature is being touched here it may be worth fixing in the same template pass to avoid shipping it in scaffolded modules. If fixed, update the route definition and any references tocreateFromas well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl` at line 68, The controller method name createFrom is a typo and should be renamed to createForm; update the method signature in PostManagementController (rename createFrom to createForm), update any route definitions and all references/calls to createFrom (including templates, router registrations, and tests) to use createForm, and ensure the rendered view ('post/form') and any type hints (Request, Response) remain unchanged so routing and DI continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl`:
- Line 43: In AuthController.php.tpl there are closing brace tokens that are
mis-indented (specifically standalone '}', '} else {', and '} catch {'
occurrences) — locate these tokens in the template and re-indent them to match
their surrounding block indentation (align them with the opening block level,
e.g., 8 spaces in this scaffold) so the closing braces line up visually with the
corresponding control/try blocks; ensure any replaced lines preserve the file's
existing indentation style and spacing.
In `@src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl`:
- Line 73: The template has misaligned control-closing lines (the "} else {" and
"} catch (AuthException $e) {" occurrences) indented 4 spaces while their
surrounding block statements use 8-space indentation, making the generated
AuthController scaffold unreadable; fix by adjusting each of those
closing-brace+keyword lines in the AuthController.php.tpl so they use the same
8-space indent as the surrounding if/try blocks (update the occurrences of "}
else {" and all "} catch (... ) {" lines to match the surrounding indentation).
In
`@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl`:
- Line 94: In PostManagementController.php.tpl there are misaligned closing
braces for inner if blocks: move each closing curly brace currently indented
only 4 spaces so it aligns with the inner block's indentation (match the
12-space indent used by the inner statements) so the brace sits at the same
indentation as the inner content; update all occurrences of these misplaced
braces in the template (the other similar instances in the file) so the inner if
blocks' opening, body and closing braces use consistent indentation relative to
their surrounding block level.
In `@tests/Unit/Middleware/MiddlewareManagerTest.php`:
- Around line 87-107: The Response instance is reused across tests causing
header leakage; update AppTestCase::tearDown() to reset the Response state after
each test by checking Di::isRegistered(Response::class) and, if registered,
retrieving it via Di::get(Response::class) and calling its flush/reset method
(or otherwise clear headers/body) so tests like
testMiddlewareManagerCanShortCircuit that rely on a fresh Response no longer see
headers set by previous tests; modify AppTestCase::tearDown() to perform this
cleanup.
---
Outside diff comments:
In `@src/Module/Templates/DemoApi/src/Controllers/AccountController.php.tpl`:
- Around line 49-100: The catch blocks in AccountController::update and
AccountController::updatePassword call $response->json(...) but do not return
it, violating the declared ": Response" return type and causing a TypeError when
an AuthException occurs; fix both catch blocks to return the
$response->json(...) value (i.e., add "return" before $response->json(...)) and
correct the catch indentation to align with the corresponding try blocks so the
method returns a Response in all paths.
In `@src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl`:
- Around line 36-54: Several branches in AuthController methods (signin,
signout, verify, resend) call $response->json(...) without returning it, which
breaks the declared ": Response" return type and causes TypeError; update each
non-returning call inside signin (try before catch and the catch block), signout
(the else branch), verify (catch block), and resend (catch block) to "return
$response->json(...)" so the method always returns a Response instance — search
for the methods signin(), signout(), verify(), resend() and replace those
standalone $response->json(...) statements with returned values.
In `@src/Module/Templates/DemoApi/src/Controllers/PostController.php.tpl`:
- Around line 84-111: In PostController::post(), remove the unreachable stop()
call after the 404 return and fix the misplaced closing brace so the if-block is
properly closed; then ensure the success branch returns the Response by adding a
return before the final $response->json([...]) call (so post() always returns a
Response and no unreachable code remains).
In
`@src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl`:
- Around line 62-170: The four controller actions create(), amend(), delete(),
and deleteImage() declare a Response return but never return one (they call
$response->json(...) without returning it) and several closing braces are
mis-indented; fix by prepending return to the final $response->json(...) calls
in create(), amend(), delete(), and deleteImage() so each method returns the
Response, and correct the misplaced/dedented closing braces for the if blocks
(the hasFile('image') checks in create() and amend(), and the if ($post->image)
checks in amend(), delete(), and deleteImage()) so each if/brace pair is
properly matched and indented.
In `@src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl`:
- Around line 65-208: All action methods (signin, signout, signup, activate,
forget, reset, verify, resend) declare ": Response" but currently don't return a
Response; change every redirect(...) call to be followed by "return $response;"
and change calls that render HTML like "$response->html(...)" to "return
$response->html(...)" (apply consistently inside signin, signout, signup,
activate, forget, reset, verify, resend), and also fix the misaligned
indentation of the "} else {" and "} catch {" closing braces so they align with
their corresponding if/catch blocks in those same methods.
In `@src/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tpl`:
- Around line 17-20: The template for CommentController is missing the import
for Quantum\Http\Response causing the declared return types on create() and
delete() to resolve to an undefined class; add a "use Quantum\Http\Response;"
import at the top of the file (alongside the existing use statements) so the
create and delete methods in the CommentController template resolve correctly.
- Around line 52-73: The declared return type Response on
CommentController::create and CommentController::delete is incorrect because
neither returns a Response; update each method to either return a proper
Response object (e.g., convert the redirect call into/return a Response instance
created by your framework) or change the signature to a compatible type (void or
never) and remove : Response; locate the methods named create(Request $request,
?string $lang, string $uuid): Response and delete(?string $lang, string $uuid):
Response and make the change consistently (return a Response from
redirect(get_referrer()) or adjust the method return types).
In `@src/Module/Templates/DemoWeb/src/Controllers/PostController.php.tpl`:
- Around line 88-113: The post() method in PostController has an unreachable
stop() after the 404 return and fails to return a Response on the success path;
remove the dead call to stop(), fix the mis-indented closing brace of the
if-block, and ensure the success branch returns the Response by changing the
final line to return $response->html($this->view->render('post/single')); so
PostController::post() always returns a Response object.
In
`@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl`:
- Around line 84-101: The create, amend, delete, and deleteImage methods declare
a Response return type but only call the redirect helper (which returns void),
causing a TypeError; after the redirect(...) call in each of
PostManagementController::create, ::amend, ::delete, and ::deleteImage,
explicitly return the framework response object (call response() and return it)
so the method satisfies its : Response signature.
In `@src/Module/Templates/Toolkit/src/Controllers/DatabaseController.php.tpl`:
- Around line 87-123: The create(), update(), and delete() methods in
DatabaseController.php.tpl declare a Response return type but never return one;
update these methods (create, update, delete) to return the Response produced by
the redirect(...) helper (or remove the ": Response" annotation if you prefer
the other convention used elsewhere) so the signature matches
behavior—specifically change each method to return the redirect(...) call
instead of just calling redirect(...), ensuring the returned value is the
expected Response from the redirect helper.
In `@src/Module/Templates/Toolkit/src/Controllers/EmailsController.php.tpl`:
- Around line 80-85: The delete() method declares a Response return type but
calls redirect(...) which throws StopExecutionException and never returns;
update the method signature to reflect that it does not return (either change
public function delete(string $emailId): Response to public function
delete(string $emailId): void if you want no return, or to public function
delete(string $emailId): never if you want to document that it always terminates
via redirect/StopExecutionException) and ensure any callers or docblocks are
adjusted accordingly; locate the method by the delete(string $emailId)
declaration and the redirect(...) call to apply the change.
---
Nitpick comments:
In `@src/App/Adapters/WebAppAdapter.php`:
- Around line 85-92: Replace the second direct call to the global request() with
the already-captured local $request for consistency: use $request->getUri() when
calling $this->setupViewCache()->serveCachedView(...). Keep the rest of the
method unchanged so MiddlewareManager($matchedRoute) and
RouteDispatcher()->dispatch($matchedRoute, $request) continue to use
$matchedRoute and the local $request consistently.
In `@src/Middleware/MiddlewareManager.php`:
- Around line 56-68: applyMiddlewares currently uses the array internal pointer
(current()/next()) on $this->middlewares which makes the method non-reentrant
and causes incorrect behavior if middlewares call $next() multiple times or if
falsy entries exist; change traversal to use an explicit integer index (e.g.,
$index) passed through the closure instead of current()/next(), update
applyMiddlewares to reset/start from index 0 and call getMiddleware($index)
(modify getMiddleware to accept an index or key) to retrieve the middleware, and
use the closure to invoke $this->applyMiddlewares($request, $response, $index +
1) so the pointer state is not mutated and falsy array values don’t prematurely
terminate the chain.
In `@src/Module/Templates/DemoApi/src/Middlewares/Comment.php.tpl`:
- Around line 29-34: The PHPDoc for the apply method in Comment.php.tpl is
missing the `@return` Response tag; update the docblock above public function
apply(Request $request, Response $response, Closure $next): Response to include
"@return Response" so it matches sibling middleware templates (e.g.,
Verify.php.tpl, Signup.php.tpl, Resend.php.tpl) for consistency.
In `@src/Module/Templates/DemoApi/src/Middlewares/Password.php.tpl`:
- Around line 31-36: The PHPDoc for the Password middleware's apply method is
missing a `@return` annotation; update the docblock above public function
apply(Request $request, Response $response, Closure $next): Response to include
"@return Response" so the PHPDoc matches the method signature and stays
consistent with other templates (see Password.php.tpl and method apply).
In `@src/Module/Templates/DemoApi/src/Middlewares/Update.php.tpl`:
- Around line 30-35: The PHPDoc for the apply method is missing a `@return` tag;
update the docblock above the apply(Request $request, Response $response,
Closure $next): Response method in Update.php.tpl to include "@return Response"
so it matches siblings like Verify.php.tpl and Resend.php.tpl and keeps the
middleware templates consistent.
In `@src/Module/Templates/DemoWeb/src/Controllers/BaseController.php.tpl`:
- Around line 49-51: The closing brace for the if block checking
request()->isMethod('get') is mis-indented and sits at the same level as the
method's closing brace; fix by moving the `}` that closes the `if
(request()->isMethod('get'))` block to 8 spaces (one additional indentation
level) so it aligns with the inner block scope inside the method in
BaseController.php.tpl and preserves clear block structure for that `if` inside
the method.
In
`@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl`:
- Line 68: The controller method name createFrom is a typo and should be renamed
to createForm; update the method signature in PostManagementController (rename
createFrom to createForm), update any route definitions and all references/calls
to createFrom (including templates, router registrations, and tests) to use
createForm, and ensure the rendered view ('post/form') and any type hints
(Request, Response) remain unchanged so routing and DI continue to work.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Comment.php.tpl`:
- Around line 29-34: The PHPDoc for the middleware method apply is missing the
`@return` Response tag; update the docblock above the public function
apply(Request $request, Response $response, Closure $next): Response (in the
Comment middleware template) to include "@return Response" so the docblock
matches the method signature and other middleware templates.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Password.php.tpl`:
- Around line 32-37: Add a `@return` Response tag to the PHPDoc of the apply
method so the docblock matches the method signature; update the PHPDoc above
public function apply(Request $request, Response $response, Closure $next):
Response to include "@return Response" similar to the other middleware templates
(e.g., Resend.php.tpl, Signup.php.tpl, PostOwner.php.tpl).
In `@src/Module/Templates/DemoWeb/src/Middlewares/Update.php.tpl`:
- Around line 30-35: The PHPDoc for the apply method in the Update middleware is
missing the `@return` Response annotation; update the docblock above public
function apply(Request $request, Response $response, Closure $next): Response to
include "@return Response" so it matches the other middleware templates (Editor,
Forget, Auth, Activate) and reflects the declared return type.
In `@tests/Unit/Middleware/MiddlewareManagerTest.php`:
- Around line 60-64: The test file contains an unused Mockery import and a dead
Mockery::close() call in the tearDown() override; remove the Mockery import and
either delete the tearDown() method override (or at minimum remove the
Mockery::close() call) so the class no longer references Mockery, or
alternatively add real Mockery-based mocks in the test methods and keep
Mockery::close() in tearDown(); look for the tearDown() method and the top-level
use Mockery; declaration to apply the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15741bb7-e5ee-4509-80e6-0bef99104601
📒 Files selected for processing (52)
src/App/Adapters/WebAppAdapter.phpsrc/Middleware/MiddlewareManager.phpsrc/Middleware/QtMiddleware.phpsrc/Module/Templates/DefaultApi/src/Controllers/MainController.php.tplsrc/Module/Templates/DefaultWeb/src/Controllers/MainController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/AccountController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/CommentController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Activate.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Auth.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Comment.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/CommentOwner.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Editor.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Forget.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Password.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/PostOwner.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Resend.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Reset.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Signout.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Signup.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Update.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/Verify.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/AccountController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/BaseController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PageController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PostController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Auth.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Comment.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/CommentOwner.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Editor.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Forget.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Guest.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Password.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Resend.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Signup.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Update.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Verify.php.tplsrc/Module/Templates/Toolkit/src/Controllers/DashboardController.php.tplsrc/Module/Templates/Toolkit/src/Controllers/DatabaseController.php.tplsrc/Module/Templates/Toolkit/src/Controllers/EmailsController.php.tplsrc/Module/Templates/Toolkit/src/Controllers/LogsController.php.tplsrc/Module/Templates/Toolkit/src/Middlewares/BasicAuth.php.tplsrc/Module/Templates/Toolkit/src/Middlewares/CreateTable.php.tpltests/Unit/Middleware/MiddlewareManagerTest.php
…ates, and fix test isolation
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl (1)
61-66:⚠️ Potential issue | 🟠 Major
validateRequestdiscards theResponsereturned byrespondWithError.Now that
respondWithErrorreturns aResponse(and no longer callsstop()), the error response from a failed validation is silently dropped here. Any subclass middleware that callsvalidateRequest(...)without checking its result will continue executing the middleware chain even when validation fails — defeating the purpose of the refactor described in the AI summary ("the returnedResponseobject becoming the value used by middleware chaining").Propagate the
Responseto the caller so subclasses canreturn $this->validateRequest(...)to short-circuit the chain.🔧 Proposed fix
/** * Validate the request and respond with error if invalid. * `@param` Request $request * `@param` Response $response + * `@return` Response|null */ - protected function validateRequest(Request $request, Response $response) + protected function validateRequest(Request $request, Response $response): ?Response { if (!$this->validator->isValid($request->all())) { - $this->respondWithError($request, $response, $this->validator->getErrors()); + return $this->respondWithError($request, $response, $this->validator->getErrors()); } + + return null; }Also verify all subclass middlewares (e.g.
Signout.php.tplmentioned in the summary) consume the new return value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl` around lines 61 - 66, The validateRequest method currently discards the Response returned by respondWithError so a failing validation doesn't short-circuit middleware; change validateRequest (in BaseMiddleware::validateRequest) to return the Response from $this->respondWithError(...) when validation fails (e.g. return $this->respondWithError($request, $response, $this->validator->getErrors());) and adjust its signature/return type accordingly, then update all subclass middlewares (e.g. Signout.php.tpl and any other middleware that calls validateRequest) to consume and return that value (e.g. return $this->validateRequest(...);) so the error Response propagates and stops the chain.src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl (1)
38-47:⚠️ Potential issue | 🔴 CriticalValidation failure response discarded — middleware bypasses error on invalid input due to void return contract mismatch.
The refactor changed
respondWithErrorfrom callingstop()to returning aResponse. However,BaseMiddleware::validateRequest()is declared without a return type (implicitlyvoid) and discards the return value when calling$this->respondWithError(). WhenPostOwner::respondWithError()returns a 404 Response, that Response is lost insidevalidateRequest(), andapply()unconditionally proceeds to$next(), allowing invalid requests through.This pattern affects all middleware templates identically. The fix requires
BaseMiddleware::validateRequest()to capture and return the Response fromrespondWithError()(change return type to?Response), then haveapply()check that return value and short-circuit if an error response was returned.Suggested fix
In
BaseMiddleware.php.tpl,validateRequest()must be updated to propagate the error response:- protected function validateRequest(Request $request, Response $response) + protected function validateRequest(Request $request, Response $response): ?Response { if (!$this->validator->isValid($request->all())) { - $this->respondWithError($request, $response, $this->validator->getErrors()); + return $this->respondWithError($request, $response, $this->validator->getErrors()); } + return null; }Then in
PostOwner.php.tplapply():public function apply(Request $request, Response $response, Closure $next): Response { $uuid = (string)route_param('uuid'); $request->set('uuid', $uuid); - $this->validateRequest($request, $response); + $errorResponse = $this->validateRequest($request, $response); + if ($errorResponse instanceof Response) { + return $errorResponse; + } return $next($request, $response); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl` around lines 38 - 47, BaseMiddleware::validateRequest currently discards the Response returned by respondWithError; change its signature to return ?Response and make it propagate the result of $this->respondWithError(...) (returning the Response when an error occurs, null otherwise). Then update PostOwner::apply to capture the result of $this->validateRequest($request, $response) and if it is a Response, return it immediately instead of always calling $next(); otherwise continue with return $next($request, $response). Ensure you update any other middleware templates that call validateRequest to follow the same pattern.src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl (1)
43-47:⚠️ Potential issue | 🔴 CriticalValidation failure path no longer short-circuits — request proceeds with invalid token.
When
validateRequest()is called at line 43, if validation fails it invokesrespondWithError()(which Activate overrides to return a 404 Response), but the response is immediately discarded sinceBaseMiddleware::validateRequest()has no return statement. Execution continues unconditionally to$request->set('activation_token', $token)andreturn $next($request, $response), passing the invalid token down the middleware chain to the controller instead of returning the error response.Fix:
BaseMiddleware::validateRequest()must be updated to return theResponsewhenrespondWithError()is invoked, or Activate must capture and return the result before proceeding to$next.Expected fix pattern
- $this->validateRequest($request, $response); - - $request->set('activation_token', $token); - - return $next($request, $response); + $errorResponse = $this->validateRequest($request, $response); + if ($errorResponse instanceof Response) { + return $errorResponse; + } + + $request->set('activation_token', $token); + + return $next($request, $response);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl` around lines 43 - 47, The validateRequest/response flow is broken: BaseMiddleware::validateRequest() currently calls respondWithError() but does not return its Response, so Activate (in Activate.php.tpl) continues and calls $request->set('activation_token', $token') and $next even on validation failure; fix by either updating BaseMiddleware::validateRequest() to return the Response object produced by respondWithError() (so callers can return it directly) or modify Activate::middleware (the code calling validateRequest($request, $response)) to capture the result from validateRequest(), and if it is a Response (error) immediately return it instead of continuing to set activation_token and invoking $next.src/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tpl (1)
43-48:⚠️ Potential issue | 🔴 CriticalValidation no longer short-circuits —
apply()still calls$next()on failure.The current implementation discards error responses from
respondWithError(). WhenvalidateRequest()detects invalid data, it callsrespondWithError()(which in Reset's case returns a 404 Response for GET or performs redirect side effects for POST), butBaseMiddleware::validateRequest()neither captures nor returns that result, allowingapply()to unconditionally execute$next().This causes:
- GET with invalid
token: the 404 Response created at line 83 is discarded;$next()executes and downstream controller can overwrite the response.- POST with invalid
password/repeat_password: the redirect side effect occurs, but$next()still executes the reset action against invalid data.To preserve the short-circuit semantics,
BaseMiddleware::validateRequest()must return the error Response (or indicate failure), andapply()must check and return it before calling$next().Suggested fix (requires coordinated change in BaseMiddleware.php.tpl)
public function apply(Request $request, Response $response, Closure $next): Response { $token = (string) route_param('token'); $request->set('token', $token); - $this->validateRequest($request, $response); + if ($errorResponse = $this->validateRequest($request, $response)) { + return $errorResponse; + } $request->set('reset_token', $token); return $next($request, $response); }And in
BaseMiddleware::validateRequest():- protected function validateRequest(Request $request, Response $response) + protected function validateRequest(Request $request, Response $response): ?Response { if (!$this->validator->isValid($request->all())) { - $this->respondWithError($request, $response, $this->validator->getErrors()); + return $this->respondWithError($request, $response, $this->validator->getErrors()); } + return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tpl` around lines 43 - 48, The middleware currently discards error Responses: BaseMiddleware::validateRequest() calls respondWithError() but does not return its Response, so Reset::apply() always continues to call $next() and overwrites or executes side-effects on invalid input; change BaseMiddleware::validateRequest() to return the Response produced by respondWithError() (or null/false on success) and update Reset::apply() (and any other apply() implementations) to check the validateRequest() return value and immediately return that Response if present instead of calling $next(); ensure references include validateRequest(), respondWithError(), and apply() so the short-circuit behavior is preserved.src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl (1)
65-208:⚠️ Potential issue | 🟠 MajorMethods declare
: Responsebut POST/error branches don't explicitly return—they rely onredirect()throwing.Each method now declares
: Response, but every POST/error branch (andsignout,activate,resendin their entirety) terminates via a bareredirect(...)call without a return statement. The helperredirect()returnsvoidand throwsStopExecutionExceptionto exit, so these methods only satisfy the return type at runtime because the exception prevents control flow from reaching the end.This creates two concerns:
Undocumented contract. If
redirect()is ever changed to be non-terminating (or a future maintainer adds code after the call and forgets the exception will prevent it from running), the method would reach its end and returnnull, violating the: Responsedeclaration. PHP will raise aTypeErrorand/orRouteDispatcher::requireResponse()will throw. Static analysis tools (PHPStan) flag this as a missing return unless the helper is annotated@return never.Inconsistency across the PR. DemoApi controllers consistently
return $response->json(...)for all paths. DemoWeb POST handlers should follow the same pattern for clarity. As a scaffolding template, this inconsistency may propagate to downstream modules.To resolve this, either:
- Annotate
redirect()with@return neverand configure PHPStan to honor it (documents the contract), or- Refactor to make
redirect()return aResponseobject so explicit returns are possible, or- Remove the
: Responsereturn type declaration if the design is to treat redirects as terminal operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl` around lines 65 - 208, Several controller actions (signin, signout, signup, activate, forget, reset, verify, resend) declare a return type of ": Response" but call the helper redirect(...) which currently returns void/throws, causing missing/unsafe returns; change the contract so these methods explicitly return a Response: update the redirect(...) helper to return a Response object (instead of being void/throw-only) and then replace all bare calls like redirect(...) in signin, signout, signup, activate, forget, reset, verify and resend with return redirect(...), ensuring every code path returns a Response and satisfies the declared ": Response" type.
🧹 Nitpick comments (4)
src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl (1)
43-46: Optional: add@return Responseto the action docblocks.Now that each action declares an explicit
Responsereturn type, the docblocks would be more accurate with a matching@return Responseline. Purely a documentation polish — no behavior change.📝 Example diff for `myPosts`
/** * Action - get my posts * `@param` Response $response + * `@return` Response */ public function myPosts(Response $response): ResponseAlso applies to: 57-61, 85-91, 121-126, 143-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl` around lines 43 - 46, Update the action docblocks in PostManagementController to include an explicit "@return Response" tag to match the declared Response return types; specifically add "@return Response" to the docblocks for the action methods (e.g., myPosts and the other action methods in the same file/template) so each action's PHPDoc lines include `@return` Response while leaving the method signatures unchanged.src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl (1)
36-189: LGTM — explicitResponsereturns are consistent across all actions.All nine actions (
signin,me,signout,signup,activate,forget,reset,verify,resend) now declare: Responseand explicitlyreturn $response->json(...)on every branch (success,else, andcatch), which aligns with the newMiddlewareManager::applyMiddlewares()contract and theRouteDispatcher::dispatch()requirement that every route handler return aResponseinstance.Optional polish: the PHPDoc blocks above each action could be updated to include
@return Responsefor documentation consistency with the new return type declaration, but this is purely cosmetic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl` around lines 36 - 189, Update the PHPDoc blocks to include the return type for consistency: add "@return Response" to the docblocks above the controller action methods (signin, me, signout, signup, activate, forget, reset, verify, resend) so the PHPDoc matches the declared return type on each method (e.g., public function signin(...): Response). This is purely documentation polish—no behavior changes—so edit the docblocks for AuthController::signin, ::me, ::signout, ::signup, ::activate, ::forget, ::reset, ::verify and ::resend to include the `@return` Response line.src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl (1)
68-86: Add explicitResponsereturn type and@returnPHPDoc.The PR's stated objective is to make middleware/controller methods declare and return
Responseexplicitly.respondWithErrornow returns aResponsebut neither the signature nor the docblock reflect that, which weakens the type contract and PHPStan signal the refactor is meant to strengthen.🔧 Proposed fix
/** * Handles error response logic. * `@param` Request $request * `@param` Response $response * `@param` mixed $message * `@param` int $status + * `@return` Response */ protected function respondWithError( Request $request, Response $response, $message, int $status = StatusCode::UNPROCESSABLE_ENTITY - ) + ): Response { return $response->json([ 'status' => 'error', 'message' => $message, ], $status); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl` around lines 68 - 86, Update the respondWithError method to declare and document that it returns a Response: add a return type of Response to the method signature for respondWithError(Request $request, Response $response, $message, int $status = StatusCode::UNPROCESSABLE_ENTITY): Response and add an `@return` Response line to the PHPDoc block; if Response is not in scope, import or fully-qualify the Response class used by the middleware.src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl (1)
66-69: Return type missing on overriddenrespondWithError.Now that
respondWithErrorreturns aResponse, consider declaring: Responseon this override (and on the baseBaseMiddleware::respondWithError) so callers — particularlyvalidateRequest— can rely on a typed return value when propagating early responses throughapply(). Without an explicit return type on the base method, this concrete override can silently drift again in the future.- protected function respondWithError(Request $request, Response $response, $message) + protected function respondWithError(Request $request, Response $response, $message): Response { return $response->html(partial('errors/404'), StatusCode::NOT_FOUND); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl` around lines 66 - 69, The override of respondWithError in Activate.php.tpl lacks an explicit return type; add a concrete return type declaration (: Response) to this method and also update the base method BaseMiddleware::respondWithError to declare : Response so callers like validateRequest and apply can rely on a typed Response being returned; ensure both method signatures match and update any related docblocks if present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl`:
- Around line 61-66: The validateRequest method currently discards the Response
returned by respondWithError so a failing validation doesn't short-circuit
middleware; change validateRequest (in BaseMiddleware::validateRequest) to
return the Response from $this->respondWithError(...) when validation fails
(e.g. return $this->respondWithError($request, $response,
$this->validator->getErrors());) and adjust its signature/return type
accordingly, then update all subclass middlewares (e.g. Signout.php.tpl and any
other middleware that calls validateRequest) to consume and return that value
(e.g. return $this->validateRequest(...);) so the error Response propagates and
stops the chain.
In `@src/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tpl`:
- Around line 65-208: Several controller actions (signin, signout, signup,
activate, forget, reset, verify, resend) declare a return type of ": Response"
but call the helper redirect(...) which currently returns void/throws, causing
missing/unsafe returns; change the contract so these methods explicitly return a
Response: update the redirect(...) helper to return a Response object (instead
of being void/throw-only) and then replace all bare calls like redirect(...) in
signin, signout, signup, activate, forget, reset, verify and resend with return
redirect(...), ensuring every code path returns a Response and satisfies the
declared ": Response" type.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl`:
- Around line 43-47: The validateRequest/response flow is broken:
BaseMiddleware::validateRequest() currently calls respondWithError() but does
not return its Response, so Activate (in Activate.php.tpl) continues and calls
$request->set('activation_token', $token') and $next even on validation failure;
fix by either updating BaseMiddleware::validateRequest() to return the Response
object produced by respondWithError() (so callers can return it directly) or
modify Activate::middleware (the code calling validateRequest($request,
$response)) to capture the result from validateRequest(), and if it is a
Response (error) immediately return it instead of continuing to set
activation_token and invoking $next.
In `@src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl`:
- Around line 38-47: BaseMiddleware::validateRequest currently discards the
Response returned by respondWithError; change its signature to return ?Response
and make it propagate the result of $this->respondWithError(...) (returning the
Response when an error occurs, null otherwise). Then update PostOwner::apply to
capture the result of $this->validateRequest($request, $response) and if it is a
Response, return it immediately instead of always calling $next(); otherwise
continue with return $next($request, $response). Ensure you update any other
middleware templates that call validateRequest to follow the same pattern.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tpl`:
- Around line 43-48: The middleware currently discards error Responses:
BaseMiddleware::validateRequest() calls respondWithError() but does not return
its Response, so Reset::apply() always continues to call $next() and overwrites
or executes side-effects on invalid input; change
BaseMiddleware::validateRequest() to return the Response produced by
respondWithError() (or null/false on success) and update Reset::apply() (and any
other apply() implementations) to check the validateRequest() return value and
immediately return that Response if present instead of calling $next(); ensure
references include validateRequest(), respondWithError(), and apply() so the
short-circuit behavior is preserved.
---
Nitpick comments:
In `@src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl`:
- Around line 36-189: Update the PHPDoc blocks to include the return type for
consistency: add "@return Response" to the docblocks above the controller action
methods (signin, me, signout, signup, activate, forget, reset, verify, resend)
so the PHPDoc matches the declared return type on each method (e.g., public
function signin(...): Response). This is purely documentation polish—no behavior
changes—so edit the docblocks for AuthController::signin, ::me, ::signout,
::signup, ::activate, ::forget, ::reset, ::verify and ::resend to include the
`@return` Response line.
In
`@src/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tpl`:
- Around line 43-46: Update the action docblocks in PostManagementController to
include an explicit "@return Response" tag to match the declared Response return
types; specifically add "@return Response" to the docblocks for the action
methods (e.g., myPosts and the other action methods in the same file/template)
so each action's PHPDoc lines include `@return` Response while leaving the method
signatures unchanged.
In `@src/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tpl`:
- Around line 68-86: Update the respondWithError method to declare and document
that it returns a Response: add a return type of Response to the method
signature for respondWithError(Request $request, Response $response, $message,
int $status = StatusCode::UNPROCESSABLE_ENTITY): Response and add an `@return`
Response line to the PHPDoc block; if Response is not in scope, import or
fully-qualify the Response class used by the middleware.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tpl`:
- Around line 66-69: The override of respondWithError in Activate.php.tpl lacks
an explicit return type; add a concrete return type declaration (: Response) to
this method and also update the base method BaseMiddleware::respondWithError to
declare : Response so callers like validateRequest and apply can rely on a typed
Response being returned; ensure both method signatures match and update any
related docblocks if present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2288a572-001a-4c45-9a84-b1fd83e375a1
📒 Files selected for processing (14)
src/Module/Templates/DemoApi/src/Controllers/AccountController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PostController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Verify.php.tplsrc/Module/Templates/Toolkit/src/Middlewares/BasicAuth.php.tpltests/Unit/AppTestCase.php
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Module/Templates/DemoWeb/src/Middlewares/Verify.php.tpl
- src/Module/Templates/DemoApi/src/Controllers/PostController.php.tpl
- src/Module/Templates/DemoApi/src/Controllers/AccountController.php.tpl
- src/Module/Templates/DemoWeb/src/Controllers/PostController.php.tpl
- src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl
…re validation, update redirect contract, and polish PHPDocs
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Module/Templates/DemoWeb/src/Middlewares/BaseMiddleware.php.tpl (1)
68-79:⚠️ Potential issue | 🟠 MajorDefault
respondWithErroris a silent-failure footgun.Returning bare
$responsemakesvalidateRequest()resolve to a truthyResponseon validation failure, so any subclass that adds validation rules but forgets to overriderespondWithErrorwill short-circuitapply()with an unmodified response (no status, no body) — i.e., a 200 OK with empty/previous content on invalid input. The DemoApi counterpart avoids this by emitting a JSON error payload by default.Consider one of:
♻️ Option A — return null by default so `validateRequest()` falls through to a real handler
- protected function respondWithError(Request $request, Response $response, $message): Response - { - // default no-op: subclasses override if needed - return $response; - } + protected function respondWithError(Request $request, Response $response, $message): ?Response + { + // default no-op: subclasses must override to provide a meaningful error response + return null; + }(Requires widening the return type and updating
validateRequest()'s short-circuit accordingly.)♻️ Option B — make it abstract so subclasses are forced to implement it
- protected function respondWithError(Request $request, Response $response, $message): Response - { - // default no-op: subclasses override if needed - return $response; - } + abstract protected function respondWithError(Request $request, Response $response, $message): Response;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/BaseMiddleware.php.tpl` around lines 68 - 79, The current protected function respondWithError(Request $request, Response $response, $message): Response silently returns the original Response, causing validateRequest() to treat validation failures as a successful response; change respondWithError to return ?Response (i.e., protected function respondWithError(...): ?Response) and implement a sensible default JSON error payload when returning a Response, but if you prefer falling through, make the default return null; then update validateRequest() (and any short-circuit in apply()) to treat a null result as "no response produced" and continue to the real handler, or to accept a non-null Response as the terminal error response — ensure method signatures and callers are adjusted to the new nullable return type (respondWithError and validateRequest are the key symbols to edit).
🧹 Nitpick comments (2)
src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl (1)
69-72: Minor: drop the default on$messageand the stray double space.
- The base declaration in
DemoWeb/.../BaseMiddleware.php.tplLine 75 has$message(no default). Adding= nullhere only in the override is inconsistent and can hide accidental call sites that omit the argument.- Cosmetic double space between arguments on Line 71.
♻️ Suggested cleanup
- protected function respondWithError(Request $request, Response $response, $message = null): Response - { - return $response->html(partial('errors/404'), StatusCode::NOT_FOUND); - } + protected function respondWithError(Request $request, Response $response, $message): Response + { + return $response->html(partial('errors/404'), StatusCode::NOT_FOUND); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl` around lines 69 - 72, The override of respondWithError in PostOwner.php.tpl is inconsistent with the base signature and has a stray double space; update the method signature to match BaseMiddleware.php.tpl by removing the default value so it reads respondWithError(Request $request, Response $response, $message) and fix the argument spacing in the return call (remove the extra space between arguments) so the call to $response->html(partial('errors/404'), StatusCode::NOT_FOUND) matches the rest of the codebase.src/Http/Helpers/http.php (1)
68-86: LGTM onredirect/redirectWithreturn-type changes.Returning the
Responsefromresponse()->redirect(...)correctly composes with the new return-driven middleware/controller pipeline. One related follow-up worth considering in this PR scope (or a follow-up):page_not_found()(Lines 126–143) still side-effects on the singleton response and returnsvoid, which leaves a single inconsistency in this file relative to the rest of the refactor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Http/Helpers/http.php` around lines 68 - 86, The page_not_found function still mutates the singleton response and returns void; update its signature to return a Response (e.g., Response page_not_found(...)), create and return a new Response instance (or use response()->redirect/response()->withStatus/response()->setBody equivalents that return a Response) instead of side-effecting the global, and update any callers to consume the returned Response; ensure any declared throws remain accurate (e.g., ConfigException|DiException|ReflectionException) and adjust docblock and tests accordingly so page_not_found is consistent with redirect/redirectWith.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Http/Helpers/http.php`:
- Line 72: Callers pass nullable get_referrer(): ?string into non-nullable
redirect(string $url) and redirectWith(string $url, ...); update those call
sites (e.g., in Reset.php.tpl, CommentController.php.tpl, Editor.php.tpl,
Comment.php.tpl) to coalesce a safe fallback like redirect(get_referrer() ??
'/'), or redirectWith(get_referrer() ?? '/', $data), so redirect/redirectWith
signatures can remain non-nullable and no null is passed at runtime.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tpl`:
- Around line 88-89: get_referrer() can return null but redirect() expects a
non-null string, so guard the call in the Reset middleware: check the result of
get_referrer() and use a safe fallback URL (e.g. '/') before calling
redirect(...). Update the code surrounding session()->setFlash('error',
$message) and the subsequent redirect(get_referrer()) call to compute $url =
get_referrer() ?? '/' (or another configured safe route) and pass $url to
redirect() to avoid passing null and emitting a malformed Location header.
---
Outside diff comments:
In `@src/Module/Templates/DemoWeb/src/Middlewares/BaseMiddleware.php.tpl`:
- Around line 68-79: The current protected function respondWithError(Request
$request, Response $response, $message): Response silently returns the original
Response, causing validateRequest() to treat validation failures as a successful
response; change respondWithError to return ?Response (i.e., protected function
respondWithError(...): ?Response) and implement a sensible default JSON error
payload when returning a Response, but if you prefer falling through, make the
default return null; then update validateRequest() (and any short-circuit in
apply()) to treat a null result as "no response produced" and continue to the
real handler, or to accept a non-null Response as the terminal error response —
ensure method signatures and callers are adjusted to the new nullable return
type (respondWithError and validateRequest are the key symbols to edit).
---
Nitpick comments:
In `@src/Http/Helpers/http.php`:
- Around line 68-86: The page_not_found function still mutates the singleton
response and returns void; update its signature to return a Response (e.g.,
Response page_not_found(...)), create and return a new Response instance (or use
response()->redirect/response()->withStatus/response()->setBody equivalents that
return a Response) instead of side-effecting the global, and update any callers
to consume the returned Response; ensure any declared throws remain accurate
(e.g., ConfigException|DiException|ReflectionException) and adjust docblock and
tests accordingly so page_not_found is consistent with redirect/redirectWith.
In `@src/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tpl`:
- Around line 69-72: The override of respondWithError in PostOwner.php.tpl is
inconsistent with the base signature and has a stray double space; update the
method signature to match BaseMiddleware.php.tpl by removing the default value
so it reads respondWithError(Request $request, Response $response, $message) and
fix the argument spacing in the return call (remove the extra space between
arguments) so the call to $response->html(partial('errors/404'),
StatusCode::NOT_FOUND) matches the rest of the codebase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9dcf375-8cf2-4a6b-a285-fb66c257a787
📒 Files selected for processing (11)
src/Http/Helpers/http.phpsrc/Http/Traits/Response/Header.phpsrc/Module/Templates/DemoApi/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/DemoApi/src/Middlewares/PostOwner.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Activate.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/PostOwner.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Reset.php.tpl
✅ Files skipped from review due to trivial changes (1)
- src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tpl
…re validation, update redirect contract, fix referrer fallbacks and polish templates
Closes #471
Summary by CodeRabbit
Refactor
Tests