Skip to content

Conversation

@ljonesfl
Copy link
Member

@ljonesfl ljonesfl commented Dec 28, 2025

Summary by CodeRabbit

  • New Features
    • Added enum validation for DTO properties, restricting values to a specified enumerated set
    • Supports both strict and loose comparison modes with configurable behavior
    • Handles validation of string and integer enum values
    • Fully compatible with nested objects, optional properties, and multiple enums per DTO
    • Includes case-sensitive validation

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 28, 2025

📝 Walkthrough

Walkthrough

Introduces enum validation support for DTO properties. The Factory now detects enum definitions in property configurations and calls Property's new setEnum method, which manages enum validators using an IsInSet validator with configurable strict comparison mode.

Changes

Cohort / File(s) Summary
Enum Validation Implementation
src/Dto/Factory.php, src/Dto/Property.php
Factory detects enum arrays in property definitions and invokes Property.setEnum(). New Property.setEnum() method replaces existing enum validators with IsInSet validator supporting strict/loose comparison modes.
Enum Validation Test Suite
tests/Dto/EnumValidationTest.php
Comprehensive test coverage for enum validation across 11 test methods, including valid/invalid values, case sensitivity, integer enums, strict/loose modes, YAML-sourced enums, optional properties, nested objects, and multiple enum properties.

Sequence Diagram

sequenceDiagram
    participant Factory as Factory
    participant Property as Property
    participant IsInSet as IsInSet Validator
    participant Value as Data Source

    Factory->>Factory: Read property definition<br/>(includes enum array)
    Factory->>Property: setEnum(allowedValues, strict)
    activate Property
    Property->>Property: Remove existing<br/>enum validator
    Property->>IsInSet: Create new IsInSet validator<br/>(allowedValues, strict)
    Property-->>Factory: Return self (fluent)
    deactivate Property
    
    Note over Value,IsInSet: During validation
    Value->>IsInSet: Check if value in enum
    alt Valid
        IsInSet-->>Value: ✓ Pass
    else Invalid
        IsInSet-->>Value: ✗ Fail (throws)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops with glee so bright, ✨🐰
Enum validation now shines just right!
Values checked, strict or loose at will,
DTOs validated with utmost skill.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'adds enums' is vague and overly generic, failing to specify what enum functionality was added or which components were modified. Provide a more descriptive title such as 'Add enum validation support to DTO properties' to clearly convey the specific feature being introduced.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/enums

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/Dto/EnumValidationTest.php (1)

198-203: Optional: Consider using tearDown() for YAML cleanup.

The temporary file cleanup is handled in a finally block, which works correctly. For consistency with PHPUnit conventions, you could alternatively use a tearDown() method or @after annotation to manage temporary file cleanup across tests.

Alternative approach using tearDown
private $tempFiles = [];

protected function tearDown(): void
{
    parent::tearDown();
    foreach ($this->tempFiles as $file) {
        if (file_exists($file)) {
            unlink($file);
        }
    }
    $this->tempFiles = [];
}

public function testEnumLoadedFromYamlFile()
{
    // ... existing code ...
    $tempFile = sys_get_temp_dir() . '/test_enum_' . uniqid() . '.yaml';
    file_put_contents($tempFile, $yamlContent);
    $this->tempFiles[] = $tempFile;
    
    // ... rest of test without finally block ...
}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad896f and 17748b4.

📒 Files selected for processing (3)
  • src/Dto/Factory.php
  • src/Dto/Property.php
  • tests/Dto/EnumValidationTest.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Dto/Property.php (1)
src/Dto/Validation.php (1)
  • Validation (11-29)
src/Dto/Factory.php (1)
src/Dto/Property.php (1)
  • setEnum (200-207)
tests/Dto/EnumValidationTest.php (4)
src/Dto/Dto.php (1)
  • Dto (15-261)
src/Dto/Factory.php (2)
  • Factory (17-318)
  • create (44-71)
src/Dto/Property.php (1)
  • validate (234-256)
src/Dto/Validation.php (1)
  • Validation (11-29)
🪛 GitHub Actions: CI
src/Dto/Property.php

[error] 204-204: Class "Neuron\Validation\IsInSet" not found during Enum validation.

🔇 Additional comments (2)
src/Dto/Factory.php (1)

188-192: LGTM - Enum handling logic is correct.

The implementation follows the same pattern as other validators (length, range, pattern) and correctly defaults strict to true for type-safe comparisons.

tests/Dto/EnumValidationTest.php (1)

8-311: Comprehensive test coverage.

The test suite thoroughly exercises enum validation across multiple scenarios including valid/invalid values, case sensitivity, integer enums, strict vs loose comparison, YAML definitions, optional properties, nested objects, and multiple enums. The test structure follows PHPUnit best practices.

However, all tests will fail until the missing Validation\IsInSet class is implemented (see review comment on Property.php).

Comment on lines +200 to +207
public function setEnum( array $allowedValues, bool $strict = true ): Property
{
$this->validators->remove( 'enum' );

$this->validators->add( 'enum', new Validation\IsInSet( $allowedValues, $strict ) );

return $this;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for IsInSet class definition
echo "=== Searching for IsInSet class definition ==="
rg -n --type=php "class IsInSet" || echo "No IsInSet class found"

echo ""
echo "=== Searching for any references to IsInSet ==="
rg -n --type=php "IsInSet" || echo "No IsInSet references found"

echo ""
echo "=== Listing all files in Validation namespace ==="
fd -e php . -x rg -l "namespace.*Validation" {} \; | sort

echo ""
echo "=== Checking existing validators ==="
fd -e php . -path "*/Validation/*" -type f

Repository: Neuron-PHP/dto

Length of output: 563


🏁 Script executed:

#!/bin/bash
# Find the Validation namespace location
echo "=== Searching for Validation namespace declarations ==="
rg -n "namespace.*Validation" --type php

echo ""
echo "=== Finding all PHP files in src directory ==="
fd -e php . src/

echo ""
echo "=== Checking structure of src/Validation or similar ==="
find src -type d -name "*[Vv]alidat*" 2>/dev/null

echo ""
echo "=== Searching for other validators pattern (e.g., class Is*) ==="
rg -n "class Is[A-Z]" --type php

Repository: Neuron-PHP/dto

Length of output: 579


🏁 Script executed:

#!/bin/bash
# Read the Validation.php file to understand the structure
echo "=== Content of src/Dto/Validation.php ==="
cat -n src/Dto/Validation.php

echo ""
echo "=== Check how IsInSet is used in Property.php around line 204 ==="
sed -n '190,210p' src/Dto/Property.php | cat -n

Repository: Neuron-PHP/dto

Length of output: 1363


🏁 Script executed:

#!/bin/bash
# Search for other validator usage patterns in Property.php
echo "=== Searching for validator usage patterns ==="
rg -n "validators->add" --type php -B 2 -A 2

echo ""
echo "=== Searching for Validation namespace usage ==="
rg -n "Validation\\\\" --type php

echo ""
echo "=== Check what other setters look like ==="
sed -n '150,250p' src/Dto/Property.php | grep -A 10 "public function set"

Repository: Neuron-PHP/dto

Length of output: 4574


🏁 Script executed:

#!/bin/bash
# Search for where the validators are defined
echo "=== Searching for IsStringLength definition ==="
rg -n "class IsStringLength" --type php

echo ""
echo "=== Searching for IsRegExPattern definition ==="
rg -n "class IsRegExPattern" --type php

echo ""
echo "=== Searching for imports or use statements in Property.php ==="
head -20 src/Dto/Property.php

echo ""
echo "=== Check what Validation\Collection is ==="
rg -n "class Collection" --type php -A 5

Repository: Neuron-PHP/dto

Length of output: 976


🏁 Script executed:

#!/bin/bash
# The validators are from Neuron\Validation namespace, let's check for external dependencies
echo "=== Checking for composer.json ==="
cat composer.json 2>/dev/null || echo "No composer.json found"

echo ""
echo "=== Checking for any package/vendor references ==="
find . -name "composer.lock" -o -name "vendor" -type d 2>/dev/null

echo ""
echo "=== Searching for all Validation\Is* usage in Property.php ==="
grep -n "Validation\\\\Is" src/Dto/Property.php

Repository: Neuron-PHP/dto

Length of output: 2250


Missing IsInSet validator in external dependency.

Line 204 uses Validation\IsInSet, which should come from the external neuron-php/validation package (currently required as ^0.7.0). The class does not exist in this repository because validators are provided by that dependency, not implemented locally. Verify that the installed version of neuron-php/validation includes the IsInSet class, or update the package version if this validator was added in a later release.

🧰 Tools
🪛 GitHub Actions: CI

[error] 204-204: Class "Neuron\Validation\IsInSet" not found during Enum validation.

@codecov
Copy link

codecov bot commented Dec 28, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@ljonesfl ljonesfl closed this Dec 28, 2025
@ljonesfl ljonesfl deleted the feature/enums branch December 28, 2025 02:44
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