Skip to content

# PHP AnyDataset 6.0.0 Release #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

# PHP AnyDataset 6.0.0 Release #19

wants to merge 15 commits into from

Conversation

byjg
Copy link
Owner

@byjg byjg commented Mar 23, 2025

This is a major update with significant architectural changes and interface improvements. These changes enhance flexibility and compatibility with standard PHP patterns, but do require updates to client code.

Major Changes

  • Completely refactored the Row class to use a thin facade pattern
    • Added new RowInterface to standardize operations
    • Implemented RowArray and RowObject classes to handle different data types
    • Old Row class now implements RowInterface and delegates to the appropriate implementation
  • Removed hasNext() and moveNext() methods in favor of the standard PHP Iterator interface
    • IteratorInterface now extends PHP's native Iterator interface
    • All iteration should now use the standard PHP pattern (valid(), current(), next())
  • Removed the count() method from iterators
    • Use the array length of converted iterators instead
  • Updated dependencies to require the 6.0 series of related packages
  • Extended PHP compatibility to include PHP 8.4

Breaking Changes Table

Component What Changed Migration Path
Row Refactored to a thin facade delegating to implementations Update to use new interface methods; direct property access no longer available
RowInterface New interface for row operations Implement this interface if you have custom row implementations
IteratorInterface No longer has hasNext() and moveNext() Replace with while ($iterator->valid()) { $row = $iterator->current(); $iterator->next(); }
GenericIterator No longer implements count() Convert to array and use count() on the result
Dependencies Updated requirements to byjg/xmlutil:^6.0, byjg/serializer:^6.0 Update your composer dependencies
Dev Dependencies Updated to PHPUnit 10.5/11.5 and Psalm 5.9/6.2 Update your development environment

Other Enhancements

  • Added documentation in the docs/ directory
  • Added support for IS_NULL and IS_NOT_NULL relations
  • Added a byjg/anydataset-implementation: 1.0 provide in composer.json

PHP Compatibility

  • Supports PHP 8.1 to 8.4

Description by Korbit AI

What change is being made?

Release version 6.0.0 of PHP AnyDataset library, updating PHP compatibility to versions 8.1 through 8.4, simplifying Row object management, and expanding documentation.

Why are these changes being made?

These updates ensure compatibility with recent PHP versions, utilize modern PHP features for cleaner code (e.g., type hints, match expressions), and provide extensive documentation to enhance usability and accessibility of the library, thereby improving developer experience and performance.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

byjg added 15 commits December 10, 2024 20:05
Implemented new relation types IS_NULL and IS_NOT_NULL in the Relation enum and IteratorFilter class. Updated filter evaluation logic to handle these relations and adjusted test cases to validate the new functionality.
Updated method return types for consistency across `RowOutput`, `RowValidator`, and `IteratorFilter` classes. Adjusted documentation with clearer examples and added new features such as NULL filtering, row importing, and field additions to datasets. Also reorganized sidebar positions to improve navigation.
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Fix Detected
Functionality Incorrect Operator Precedence in Grouped Conditions ▹ view
Functionality Inconsistent Object State After Array Constructor ▹ view
Functionality Incorrect return type for current() ▹ view
Security Unsafe String Value Escaping ▹ view
Design Violates Row Type Implementation ▹ view
Readability Inconsistent type documentation ▹ view
Functionality Inconsistent Array Initialization on Append ▹ view
Functionality Validator Property Overwrite ▹ view
Error Handling Missing filter array structure validation ▹ view
Readability Conflicting Type Documentation ▹ view
Files scanned
File Path Reviewed
src/Formatter/FormatterInterface.php
src/RowInterface.php
src/IteratorInterface.php
src/Formatter/JsonFormatter.php
src/Formatter/BaseFormatter.php
src/Formatter/XmlFormatter.php
src/IteratorFilterXPathFormatter.php
src/AnyIterator.php
src/GenericIterator.php
src/RowObject.php
src/RowOutput.php
src/RowArray.php
src/RowValidator.php
src/IteratorFilterFormatter.php
src/Row.php
src/AnyDataset.php
src/IteratorFilter.php

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

  • You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.
  • You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.
  • Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.
  • On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.
  • Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

  • Check out our docs on how you can make Korbit work best for you and your team.
  • Customize Korbit for your organization through the Korbit Console.

Current Korbit Configuration

General Settings
Setting Value
Review Schedule Automatic excluding drafts
Max Issue Count 10
Automatic PR Descriptions
Issue Categories
Category Enabled
Documentation
Logging
Error Handling
Readability
Design
Performance
Security
Functionality

Feedback and Support

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

Comment on lines +116 to +120
if ($operator == self::CLOSE_GROUP) {
$result = $this->evaluateFilterRecursive($row, $subList, $previousOperator);
$subList = [];
continue;
}
Copy link

Choose a reason for hiding this comment

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

Incorrect Operator Precedence in Grouped Conditions category Functionality

Tell me more
What is the issue?

The evaluation of grouped conditions doesn't properly handle the operator precedence when combining the sublist result with the main result.

Why this matters

The current implementation may produce incorrect results when mixing AND/OR operators with grouped conditions because it doesn't consider how the sublist result should be combined with the existing result based on the previous operator.

Suggested change ∙ Feature Preview

Update the closing group evaluation to properly combine results:

if ($operator == self::CLOSE_GROUP) {
    $subResult = $this->evaluateFilterRecursive($row, $subList, $previousOperator);
    if ($position == 0) {
        $result = $subResult;
    } elseif ($previousOperator == self::AND_OPERATOR) {
        $result = $result && $subResult;
    } else {
        $result = $result || $subResult;
    }
    $subList = [];
    continue;
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +72 to +80
public function __construct(string|array|null $filename = null)
{
$this->collection = array();
$this->file = null;
$this->currentRow = -1;
$this->collection = [];
if (is_array($filename)) {
$this->collection = array_map(fn($row) => Row::factory($row), $filename);
return;
}
Copy link

Choose a reason for hiding this comment

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

Inconsistent Object State After Array Constructor category Functionality

Tell me more
What is the issue?

Early return in constructor prevents defineSavePath from being called when filename is an array, which could lead to inconsistent object state.

Why this matters

If the code needs to save data later using the save() method, it will fail because the file property hasn't been properly initialized.

Suggested change ∙ Feature Preview
public function __construct(string|array|null $filename = null)
{
    $this->file = null;
    $this->currentRow = -1;
    $this->collection = [];
    if (is_array($filename)) {
        $this->collection = array_map(fn($row) => Row::factory($row), $filename);
        $this->defineSavePath(null, fn() => null);
        return;
    }
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +51 to 54
public function current(): mixed
{
if (!$this->hasNext()) {
return null;
}
return $this->list[$this->curRow++];
return $this->list[$this->curRow] ?? null;
}
Copy link

Choose a reason for hiding this comment

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

Incorrect return type for current() category Functionality

Tell me more
What is the issue?

The current() method should return Row|null to match the array type declaration in the constructor (@param Row[] $list).

Why this matters

Using mixed return type instead of the specific Row|null type breaks type safety and makes it harder for consumers to handle the return value correctly.

Suggested change ∙ Feature Preview

Change the return type to Row|null:

public function current(): ?Row
{
    return $this->list[$this->curRow] ?? null;
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +21 to +28
if (is_array($value)) {
foreach ($value as $key => $val) {
$value[$key] = is_numeric($val) ? $val : "'$val'";
}
$value = "[" . implode(",", $value) . "]";
} else {
$value = is_numeric($value) ? $value : "'$value'";
}
Copy link

Choose a reason for hiding this comment

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

Unsafe String Value Escaping category Security

Tell me more
What is the issue?

The value escaping logic is vulnerable to SQL injection when handling string values that contain single quotes.

Why this matters

Malicious input containing single quotes could break the generated SQL query structure and potentially lead to SQL injection attacks.

Suggested change ∙ Feature Preview

Use proper string escaping or prepared statements:

if (is_array($value)) {
    foreach ($value as $key => $val) {
        $value[$key] = is_numeric($val) ? $val : "'" . addslashes($val) . "'";
    }
    $value = "[" . implode(",", $value) . "]";
} else {
    $value = is_numeric($value) ? $value : "'" . addslashes($value) . "'";
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +49 to 51
public function apply(RowInterface $row): RowInterface
{
$newRow = new Row();
Copy link

Choose a reason for hiding this comment

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

Violates Row Type Implementation category Design

Tell me more
What is the issue?

The apply method instantiates a concrete Row class instead of respecting the original row implementation type passed as parameter.

Why this matters

This breaks the polymorphic behavior as it forces conversion of any RowInterface implementation (like RowObject or RowArray) into a concrete Row class, potentially losing specific implementation features or behavior.

Suggested change ∙ Feature Preview

The row should be cloned or created using the same type as the input. Example:

public function apply(RowInterface $row): RowInterface
{
    $newRow = clone $row;
    $newRow->clear();
    
    foreach ($row->toArray() as $key => $value) {
        $newRow->set($key, $this->print($row, $key));
    }
    
    return $newRow;
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

src/RowArray.php Outdated
Comment on lines +7 to +10
/**
* @var mixed
*/
private array $entity = [];
Copy link

Choose a reason for hiding this comment

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

Inconsistent type documentation category Readability

Tell me more
What is the issue?

The PHPDoc comment states @var mixed but the property is typed as array, creating confusion about the actual type.

Why this matters

Inconsistent type documentation can mislead developers about the expected type and usage of the property.

Suggested change ∙ Feature Preview
/**
 * @var array<string, mixed>
 */
private array $entity = [];
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

src/RowArray.php Outdated
Comment on lines +27 to +30
if (!isset($this->entity[$name])) {
$this->entity[$name] = $value;
return;
}
Copy link

Choose a reason for hiding this comment

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

Inconsistent Array Initialization on Append category Functionality

Tell me more
What is the issue?

The set() method's logic for handling non-existent keys is inconsistent with append behavior. If the key doesn't exist and append is true, it won't create an array as expected.

Why this matters

This could lead to inconsistent data structures when appending to non-existent keys, breaking the expected array handling functionality.

Suggested change ∙ Feature Preview

Modify the set() method to handle non-existent keys with append flag:

if (!isset($this->entity[$name])) {
    $this->entity[$name] = $append ? [$value] : $value;
    return;
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +24 to 28
protected function setProperty(array|string $fieldList, string $property, mixed $value)
{
foreach ((array)$fieldList as $field) {
if (!isset($this->fieldValidator[$field])) {
$this->fieldValidator[$field] = [];
}
$this->fieldValidator[$field] = [ $property => $value ];
}
Copy link

Choose a reason for hiding this comment

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

Validator Property Overwrite category Functionality

Tell me more
What is the issue?

The setProperty method overwrites all existing validations for a field instead of adding/updating the specific property

Why this matters

This will cause any previously set validations on the same field to be lost, preventing the ability to set multiple validation rules for the same field

Suggested change ∙ Feature Preview

Modify the setProperty method to preserve existing validations:

protected function setProperty(array|string $fieldList, string $property, mixed $value)
{
    foreach ((array)$fieldList as $field) {
        if (!isset($this->fieldValidator[$field])) {
            $this->fieldValidator[$field] = [];
        }
        $this->fieldValidator[$field][$property] = $value;
    }
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +109 to +113
foreach ($filterList as $filter) {
$operator = $filter[self::OPERATOR];
$field = $filter[self::FIELD];
$relation = $filter[self::RELATION];
$value = $filter[self::VALUE];
Copy link

Choose a reason for hiding this comment

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

Missing filter array structure validation category Error Handling

Tell me more
What is the issue?

Array access operations in evaluateFilterRecursive() lack validation of filter array structure before accessing indices.

Why this matters

If a malformed filter array is passed, accessing undefined array indices will trigger PHP notices/warnings and could lead to undefined behavior.

Suggested change ∙ Feature Preview

Add validation before accessing filter array elements:

foreach ($filterList as $filter) {
    if (!is_array($filter) || count($filter) < 4) {
        throw new InvalidArgumentException('Invalid filter structure. Expected [operator, field, relation, value]');
    }
    $operator = $filter[self::OPERATOR];
    $field = $filter[self::FIELD];
    $relation = $filter[self::RELATION];
    $value = $filter[self::VALUE];
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

src/Row.php Outdated
*/
protected bool $fieldNameCaseSensitive = true;
private RowInterface $entity;
Copy link

Choose a reason for hiding this comment

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

Conflicting Type Documentation category Readability

Tell me more
What is the issue?

The property type hint '@var mixed' conflicts with the actual type declaration 'RowInterface', causing confusion about the actual type.

Why this matters

Inconsistent type information makes it harder for developers to understand the property's expected type and could mislead IDE tooling.

Suggested change ∙ Feature Preview
/**
 * @var RowInterface
 */
private RowInterface $entity;
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

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.

1 participant