Skip to content

Conversation

@simonLeary42
Copy link
Collaborator

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables PHPStan static analysis for the webroot directory by adding it to the PHPStan configuration and refactoring code to address type safety issues. The main changes involve replacing direct $_POST array access with a safer UnityHTTPD::getPostData() method and extracting repeated user/group initialization logic into reusable closures.

  • Adds webroot directory to PHPStan analysis paths
  • Refactors POST data access to use UnityHTTPD::getPostData() for better type safety
  • Extracts user and group initialization logic into closures to reduce code duplication

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
phpstan.neon Adds webroot directory to analysis paths and ignores global variables set by init.php
webroot/panel/pi.php Refactors to use getPostData() for uid parameter and removes unused code
webroot/panel/groups.php Extracts PI group initialization into closure and uses getPostData() for pi parameter
webroot/admin/user-mgmt.php Removes unreachable break statement after redirect
webroot/admin/pi-mgmt.php Extracts user initialization into closure and uses getPostData() for uid parameter
webroot/admin/ajax/get_page_contents.php Adds missing UserFlag import for type checking
webroot/admin/ajax/get_group_members.php Adds missing UserFlag import for type checking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 53 to +54
UnityHTTPD::redirect();
break;
break; /** @phpstan-ignore deadCode.unreachable */
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP Code Sniffer requires a break or a comment explaining why it's OK to continue the switch case. PHPStan complains that the break is dead code because redirect() never returns.

Comment on lines +15 to +24
$getUserFromPost = function () {
global $LDAP, $SQL, $MAILER, $WEBHOOK;
return new UnityUser(UnityHTTPD::getPostData("uid"), $LDAP, $SQL, $MAILER, $WEBHOOK);
};

if ($_SERVER["REQUEST_METHOD"] == "POST") {
UnityHTTPD::validatePostCSRFToken();
if (isset($_POST["uid"])) {
$form_user = new UnityUser($_POST["uid"], $LDAP, $SQL, $MAILER, $WEBHOOK);
}

switch ($_POST["form_type"]) {
case "req":
$form_user = $getUserFromPost();
Copy link
Collaborator Author

@simonLeary42 simonLeary42 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPStan complains that $form_user may not be defined, so I make sure to define it everywhere that it is needed. But then PHP Code Sniffer complains that a file should define classes/functions or create side effects but not both. So I use an anonymous function.

@simonLeary42 simonLeary42 merged commit ce7869d into main Dec 19, 2025
3 checks passed
@simonLeary42 simonLeary42 deleted the phpstan-for-webpages branch December 19, 2025 20:30
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.

2 participants