Add Larastan configuration and dependencies for static analysis #9

Merged

jothi merged 8 commits from ranjith-dev into master 2025-11-26 09:11:37 +00:00

Conversation 4 Commits 8 Files Changed 8 +267 -32

jothi commented 2025-11-25 09:09:50 +00:00

Owner

Copy Link

No description provided.

jothi added 1 commit 2025-11-25 09:09:50 +00:00

Add Larastan configuration and dependencies for static analysis bfb4dd3643

dhanabalan added 1 commit 2025-11-25 09:15:53 +00:00

Update PHP version to 8.4 in Larastan and Laravel Pint workflows ecac9fe0e9

gitea-actionsbot commented 2025-11-25 10:04:52 +00:00

Copy Link

📋 Review Summary

This Pull Request introduces Larastan for static analysis, enhancing code quality checks within the project's CI/CD pipeline. It also updates the PHP version to 8.4 across relevant workflow files and composer.json dependencies. The changes are well-structured and contribute positively to the project's maintainability and code standards.

🔍 General Feedback

• The addition of Larastan is a great step towards improving code quality and catching potential issues early in the development cycle.
• The update to PHP 8.4 across the workflow files ensures consistency in the CI environment.

--- a/.github/workflows/larastan.yaml
+++ b/.github/workflows/larastan.yaml
@@ -0,0 +1,32 @@
+# Filenme: .github/workflows/larastan.yaml
+
+name: Laravel Larastan
+
+on:
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  larastan:
+    runs-on: ubuntu-latest
+
+    steps:
+      # Reinstall system libraries to ensure compatibility
+      - name: Ensure system libraries are up-to-date
+        run: |
+          sudo apt-get update
+          sudo apt-get install --reinstall --yes git libc6
+
+      - uses: actions/checkout @v4
+
+      - name: Set up PHP
+        uses: shivammathur/setup-php @v2
+        with:
+          php-version: "8.4"
+
+      - name: Install dependencies
+        run: composer install --prefer-dist --no-interaction --quiet
+
+      # Run larastan code quality checks
+      - name: Run Larastan
+        run: vendor/bin/phpstan analyse  --no-progress --memory-limit=2G

🔴 Line 1: Typo: Filenme should be Filename.

# Filename: .github/workflows/larastan.yaml

🟢 Line 17: There should be a space between actions/checkout and @v4.

      - uses: actions/checkout@v4

🟡 Line 22: While PHP 8.4 is the latest development version, it's generally recommended to use a stable, released version for CI/CD environments unless specific features of the development version are required. Consider using a stable PHP 8.x version if 8.4 is not critical for your immediate needs.

--- a/.github/workflows/laravel-pint.yaml
+++ b/.github/workflows/laravel-pint.yaml
@@ -22,7 +22,7 @@ jobs:
       - name: Set up PHP
         uses: shivammathur/setup-php @v2
         with:
-          php-version: "8.2"
+          php-version: "8.4"
 
       - name: Install dependencies
         run: composer install --prefer-dist --no-interaction --quiet

🟡 Line 25: Similar to the Larastan workflow, consider using a stable PHP version (e.g., 8.2 or 8.3) for the laravel-pint workflow unless there's a specific need for PHP 8.4's development features.

## 📋 Review Summary This Pull Request introduces Larastan for static analysis, enhancing code quality checks within the project's CI/CD pipeline. It also updates the PHP version to 8.4 across relevant workflow files and `composer.json` dependencies. The changes are well-structured and contribute positively to the project's maintainability and code standards. ## 🔍 General Feedback - The addition of Larastan is a great step towards improving code quality and catching potential issues early in the development cycle. - The update to PHP 8.4 across the workflow files ensures consistency in the CI environment. ```diff --- a/.github/workflows/larastan.yaml +++ b/.github/workflows/larastan.yaml @@ -0,0 +1,32 @@ +# Filenme: .github/workflows/larastan.yaml + +name: Laravel Larastan + +on: + pull_request: + workflow_dispatch: + +jobs: + larastan: + runs-on: ubuntu-latest + + steps: + # Reinstall system libraries to ensure compatibility + - name: Ensure system libraries are up-to-date + run: | + sudo apt-get update + sudo apt-get install --reinstall --yes git libc6 + + - uses: actions/checkout @v4 + + - name: Set up PHP + uses: shivammathur/setup-php @v2 + with: + php-version: "8.4" + + - name: Install dependencies + run: composer install --prefer-dist --no-interaction --quiet + + # Run larastan code quality checks + - name: Run Larastan + run: vendor/bin/phpstan analyse --no-progress --memory-limit=2G ``` 🔴 Line 1: Typo: `Filenme` should be `Filename`. ```suggestion # Filename: .github/workflows/larastan.yaml ``` 🟢 Line 17: There should be a space between `actions/checkout` and `@v4`. ```suggestion - uses: actions/checkout@v4 ``` 🟡 Line 22: While PHP 8.4 is the latest development version, it's generally recommended to use a stable, released version for CI/CD environments unless specific features of the development version are required. Consider using a stable PHP 8.x version if 8.4 is not critical for your immediate needs. ```diff --- a/.github/workflows/laravel-pint.yaml +++ b/.github/workflows/laravel-pint.yaml @@ -22,7 +22,7 @@ jobs: - name: Set up PHP uses: shivammathur/setup-php @v2 with: - php-version: "8.2" + php-version: "8.4" - name: Install dependencies run: composer install --prefer-dist --no-interaction --quiet ``` 🟡 Line 25: Similar to the Larastan workflow, consider using a stable PHP version (e.g., 8.2 or 8.3) for the `laravel-pint` workflow unless there's a specific need for PHP 8.4's development features.

gitea-actionsbot commented 2025-11-25 10:06:17 +00:00

Copy Link

# 📋 Review Summary

This Pull Request introduces Laravel Larastan for static analysis, integrating it into a new GitHub Actions workflow and configuring it via `phpstan.neon`. The changes are generally well-structured and align with best practices for incorporating static analysis into a Laravel project.

# 🔍 General Feedback

- The addition of Larastan is a positive step towards improving code quality and catching potential issues early in the development cycle.
- The workflow setup is straightforward and uses common GitHub Actions patterns.

• The phpstan.neon configuration provides a good starting point for static analysis.

🟢 Low

File: .github/workflows/larastan.yaml

--- a/.github/workflows/larastan.yaml
+++ b/.github/workflows/larastan.yaml
@@ -0,0 +1,32 @@
+# Filenme: .github/workflows/larastan.yaml
+
+name: Laravel Larastan
+

Line 1: There is a typo in "Filenme". It should be "Filename".

# Filename: .github/workflows/larastan.yaml

🟡 Medium

File: phpstan.neon

--- a/phpstan.neon
+++ b/phpstan.neon
@@ -0,0 +1,17 @@
+includes:
+    - vendor/larastan/larastan/extension.neon
+    - vendor/nesbot/carbon/extension.neon
+
+parameters:
+
+    paths:
+        - app/
+
+    # Level 10 is the highest level
+    level: 3
+
+#    ignoreErrors:
+#        - '#PHPDoc tag @var#'
+#
+#    excludePaths:
+#        - ./*/*/FileToBeExcluded.php

Line 10: The current PHPStan level is set to 3. While this is a good starting point, consider gradually increasing the level to a higher value (e.g., 5 or 6) over time to enforce stricter type checking and uncover more potential issues. The highest level is 9.

``` # 📋 Review Summary This Pull Request introduces Laravel Larastan for static analysis, integrating it into a new GitHub Actions workflow and configuring it via `phpstan.neon`. The changes are generally well-structured and align with best practices for incorporating static analysis into a Laravel project. # 🔍 General Feedback - The addition of Larastan is a positive step towards improving code quality and catching potential issues early in the development cycle. - The workflow setup is straightforward and uses common GitHub Actions patterns. ``` - The `phpstan.neon` configuration provides a good starting point for static analysis. ## 🟢 Low **File: .github/workflows/larastan.yaml** ```diff --- a/.github/workflows/larastan.yaml +++ b/.github/workflows/larastan.yaml @@ -0,0 +1,32 @@ +# Filenme: .github/workflows/larastan.yaml + +name: Laravel Larastan + ``` **Line 1**: There is a typo in "Filenme". It should be "Filename". ```suggestion # Filename: .github/workflows/larastan.yaml ``` ## 🟡 Medium **File: phpstan.neon** ```diff --- a/phpstan.neon +++ b/phpstan.neon @@ -0,0 +1,17 @@ +includes: + - vendor/larastan/larastan/extension.neon + - vendor/nesbot/carbon/extension.neon + +parameters: + + paths: + - app/ + + # Level 10 is the highest level + level: 3 + +# ignoreErrors: +# - '#PHPDoc tag @var#' +# +# excludePaths: +# - ./*/*/FileToBeExcluded.php ``` **Line 10**: The current PHPStan `level` is set to 3. While this is a good starting point, consider gradually increasing the level to a higher value (e.g., 5 or 6) over time to enforce stricter type checking and uncover more potential issues. The highest level is 9.

dhanabalan added 1 commit 2025-11-25 10:41:22 +00:00

Enhance get_testing_data response with additional fields as requested_date and null safety d7b6e8c111

gitea-actionsbot commented 2025-11-25 10:42:27 +00:00

Copy Link

📋 Review Summary

This Pull Request introduces Larastan for static analysis, updates the PHP version used in the Laravel Pint workflow, and enhances the UserController by adding defensive null coalescing operators to API responses and including a requested_at timestamp. The changes generally improve code quality and development tooling.

🔍 General Feedback

• The introduction of Larastan is a significant step towards improving code quality and catching potential bugs early in the development cycle.
• The consistency in updating PHP versions across CI workflows is good practice.
• The use of null coalescing operators in the API response in UserController.php is a good defensive measure, though some instances might be redundant.

🟡 Medium: app/Http/Controllers/UserController.php line 7 (LEFT diff)

The Carbon import is commented out. While now() is a global helper in Laravel that returns a Carbon instance, explicitly importing Carbon can improve readability and provide better IDE support, making the code's dependencies clearer.

use App\Models\Plant;
use App\Models\User;
use Carbon\Carbon;
use Hash;
use Illuminate\Http\Request;

🟢 Low: app/Http/Controllers/UserController.php line 87 (RIGHT diff)

The now()->format('Y-m-d H:i:s') ?? "" is redundant. The now() helper always returns a Carbon instance, and its format() method will always return a string. Therefore, the null coalescing operator (?? "") is not necessary here.

                'requested_at' => now()->format('Y-m-d H:i:s'), //Carbon::now(config('app.timezone'))->format('Y-m-d H:i:s') ?? "",

## 📋 Review Summary This Pull Request introduces Larastan for static analysis, updates the PHP version used in the Laravel Pint workflow, and enhances the `UserController` by adding defensive null coalescing operators to API responses and including a `requested_at` timestamp. The changes generally improve code quality and development tooling. ## 🔍 General Feedback - The introduction of Larastan is a significant step towards improving code quality and catching potential bugs early in the development cycle. - The consistency in updating PHP versions across CI workflows is good practice. - The use of null coalescing operators in the API response in `UserController.php` is a good defensive measure, though some instances might be redundant. ### `🟡` Medium: `app/Http/Controllers/UserController.php` line 7 (LEFT diff) The `Carbon` import is commented out. While `now()` is a global helper in Laravel that returns a `Carbon` instance, explicitly importing `Carbon` can improve readability and provide better IDE support, making the code's dependencies clearer. ```php use App\Models\Plant; use App\Models\User; use Carbon\Carbon; use Hash; use Illuminate\Http\Request; ``` ### `🟢` Low: `app/Http/Controllers/UserController.php` line 87 (RIGHT diff) The `now()->format('Y-m-d H:i:s') ?? ""` is redundant. The `now()` helper always returns a `Carbon` instance, and its `format()` method will always return a string. Therefore, the null coalescing operator (`?? ""`) is not necessary here. ```php 'requested_at' => now()->format('Y-m-d H:i:s'), //Carbon::now(config('app.timezone'))->format('Y-m-d H:i:s') ?? "", ```

dhanabalan added 1 commit 2025-11-25 11:52:35 +00:00

Added schema line in console.php 346aaf1438

gitea-actionsbot commented 2025-11-25 11:53:55 +00:00

Copy Link

🔴 app/Http/Controllers/UserController.php:88

Unnecessary Null Coalescing Operator

The null coalescing operator (?? "") is redundant here. The format() method on a Carbon instance returns a string, not null. If $existUser->created_at were null, this line would throw a "Call to a member function format() on null" error before the ?? operator is even evaluated. Given that created_at is a standard Eloquent timestamp, it's not nullable by default. This operator should be removed for cleaner, more accurate code.

'created_at' => $existUser->created_at->format('Y-m-d H:i:s'),

🔴 app/Http/Controllers/UserController.php:89

Unnecessary Null Coalescing Operator

The null coalescing operator (?? "") is redundant here. The format() method on a Carbon instance returns a string, not null. If $existUser->updated_at were null, this line would throw a "Call to a member function format() on null" error before the ?? operator is even evaluated. Given that updated_at is a standard Eloquent timestamp, it's not nullable by default. This operator should be removed for cleaner, more accurate code.

'updated_at' => $existUser->updated_at->format('Y-m-d H:i:s'),

🔴 app/Http/Controllers/UserController.php:90

Unnecessary Null Coalescing Operator

The null coalescing operator (?? "") is redundant here. The now() helper returns a Carbon instance, so its format() method will always return a string and never null. The ?? "" adds unnecessary clutter and should be removed.

'requested_at' => now()->format('Y-m-d H:i:s'), //Carbon::now(config('app.timezone'))->format('Y-m-d H:i:s') ?? "",

🔴 app/Http/Controllers/UserController.php:91

Unnecessary Null Coalescing Operator

The expression (String)$existPlant will cast the value of $existPlant to a string. If $existPlant is null, it will be converted to an empty string "". Therefore, the result of the cast can never be null, making the null coalescing operator (?? "") redundant. The code can be simplified.

'plant' => (String)$existPlant,

🔴 app/Http/Controllers/UserController.php:92

Unnecessary Null Coalescing Operator

The email property on the User model is typically not nullable, as it's used for authentication. Applying the null coalescing operator here is likely unnecessary. If the email column were nullable and a null value was encountered, it would be better to handle that case explicitly rather than silently casting it to an empty string. Assuming the column is non-nullable, this operator should be removed for clarity.

'email' => $existUser->email,

🟡 routes/console.php:35

Scheduler Output Logging Disabled

The line that appends scheduler command output to a log file has been commented out. While the command will still run, removing the output logging makes it significantly harder to debug scheduled tasks if they fail or behave unexpectedly in a production environment. It is highly recommended to keep scheduler logging enabled.

$command = $schedule->command('send:production-report', [$type, $plantId])
->appendOutputTo(storage_path('logs/scheduler.log'));

## 📋 Review Summary

This pull request introduces Larastan for static analysis, a valuable addition for improving code quality. The changes also include updating the PHP version in CI workflows for consistency and adding a defensive check in the console scheduler.

🔍 General Feedback

• Positive: Adding Larastan is an excellent step towards ensuring code correctness and maintainability. The initial configuration is well-structured.
• Improvement: There are several instances in the UserController where the null coalescing operator (?? "") is used unnecessarily. This suggests a potential misunderstanding of when values can be null and should be corrected to improve code clarity.
• Caution: Disabling scheduler output logging in routes/console.php is discouraged as it removes a crucial tool for debugging potential issues with scheduled commands.

<details> <summary> 🔴 <strong>app/Http/Controllers/UserController.php:88</strong> </summary> **Unnecessary Null Coalescing Operator** The null coalescing operator (`?? ""`) is redundant here. The `format()` method on a Carbon instance returns a string, not null. If `$existUser->created_at` were null, this line would throw a "Call to a member function format() on null" error before the `??` operator is even evaluated. Given that `created_at` is a standard Eloquent timestamp, it's not nullable by default. This operator should be removed for cleaner, more accurate code. ```suggestion 'created_at' => $existUser->created_at->format('Y-m-d H:i:s'), ``` </details> <details> <summary> 🔴 <strong>app/Http/Controllers/UserController.php:89</strong> </summary> **Unnecessary Null Coalescing Operator** The null coalescing operator (`?? ""`) is redundant here. The `format()` method on a Carbon instance returns a string, not null. If `$existUser->updated_at` were null, this line would throw a "Call to a member function format() on null" error before the `??` operator is even evaluated. Given that `updated_at` is a standard Eloquent timestamp, it's not nullable by default. This operator should be removed for cleaner, more accurate code. ```suggestion 'updated_at' => $existUser->updated_at->format('Y-m-d H:i:s'), ``` </details> <details> <summary> 🔴 <strong>app/Http/Controllers/UserController.php:90</strong> </summary> **Unnecessary Null Coalescing Operator** The null coalescing operator (`?? ""`) is redundant here. The `now()` helper returns a Carbon instance, so its `format()` method will always return a string and never null. The `?? ""` adds unnecessary clutter and should be removed. ```suggestion 'requested_at' => now()->format('Y-m-d H:i:s'), //Carbon::now(config('app.timezone'))->format('Y-m-d H:i:s') ?? "", ``` </details> <details> <summary> 🔴 <strong>app/Http/Controllers/UserController.php:91</strong> </summary> **Unnecessary Null Coalescing Operator** The expression `(String)$existPlant` will cast the value of `$existPlant` to a string. If `$existPlant` is `null`, it will be converted to an empty string `""`. Therefore, the result of the cast can never be `null`, making the null coalescing operator (`?? ""`) redundant. The code can be simplified. ```suggestion 'plant' => (String)$existPlant, ``` </details> <details> <summary> 🔴 <strong>app/Http/Controllers/UserController.php:92</strong> </summary> **Unnecessary Null Coalescing Operator** The `email` property on the `User` model is typically not nullable, as it's used for authentication. Applying the null coalescing operator here is likely unnecessary. If the `email` column were nullable and a `null` value was encountered, it would be better to handle that case explicitly rather than silently casting it to an empty string. Assuming the column is non-nullable, this operator should be removed for clarity. ```suggestion 'email' => $existUser->email, ``` </details> <details> <summary> 🟡 <strong>routes/console.php:35</strong> </summary> **Scheduler Output Logging Disabled** The line that appends scheduler command output to a log file has been commented out. While the command will still run, removing the output logging makes it significantly harder to debug scheduled tasks if they fail or behave unexpectedly in a production environment. It is highly recommended to keep scheduler logging enabled. ```suggestion $command = $schedule->command('send:production-report', [$type, $plantId]) ->appendOutputTo(storage_path('logs/scheduler.log')); ``` </details> <SUMMARY> ## 📋 Review Summary This pull request introduces Larastan for static analysis, a valuable addition for improving code quality. The changes also include updating the PHP version in CI workflows for consistency and adding a defensive check in the console scheduler. ## 🔍 General Feedback - **Positive:** Adding Larastan is an excellent step towards ensuring code correctness and maintainability. The initial configuration is well-structured. - **Improvement:** There are several instances in the `UserController` where the null coalescing operator (`?? ""`) is used unnecessarily. This suggests a potential misunderstanding of when values can be null and should be corrected to improve code clarity. - **Caution:** Disabling scheduler output logging in `routes/console.php` is discouraged as it removes a crucial tool for debugging potential issues with scheduled commands. </SUMMARY>

dhanabalan added 1 commit 2025-11-26 08:36:58 +00:00

Refactor composer install command in workflow files for Larastan and Laravel Pint 699103b047

dhanabalan added 1 commit 2025-11-26 08:43:31 +00:00

Change database connection from SQLite to PostgreSQL in .env.example b62505d064

dhanabalan added 1 commit 2025-11-26 08:59:35 +00:00

Refactor console.php to clean up commented-out scheduling code 88d8bd6c2c

dhanabalan added 1 commit 2025-11-26 09:09:18 +00:00

Uncomment scheduling logic in console.php for production, invoice, and invoice data reports ae3d8224ad

jothi merged commit 32ce6da2c1 into master 2025-11-26 09:11:37 +00:00

jothi referenced this issue from a commit 2025-11-26 09:11:37 +00:00

Merge pull request 'Add Larastan configuration and dependencies for static analysis' (#9) from ranjith-dev into master

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

dhanabalan jothi ranjith renovate-bot srimathi

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: poc/pds#9