-
-
Notifications
You must be signed in to change notification settings - Fork 4
# 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
base: master
Are you sure you want to change the base?
Conversation
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.
There was a problem hiding this 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 |
---|---|---|
Incorrect Operator Precedence in Grouped Conditions ▹ view | ||
Inconsistent Object State After Array Constructor ▹ view | ||
Incorrect return type for current() ▹ view | ||
Unsafe String Value Escaping ▹ view | ||
Violates Row Type Implementation ▹ view | ||
Inconsistent type documentation ▹ view | ||
Inconsistent Array Initialization on Append ▹ view | ||
Validator Property Overwrite ▹ view | ||
Missing filter array structure validation ▹ view | ||
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
if ($operator == self::CLOSE_GROUP) { | ||
$result = $this->evaluateFilterRecursive($row, $subList, $previousOperator); | ||
$subList = []; | ||
continue; | ||
} |
There was a problem hiding this comment.
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 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/AnyDataset.php
Outdated
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; | ||
} |
There was a problem hiding this comment.
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 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/AnyIterator.php
Outdated
public function current(): mixed | ||
{ | ||
if (!$this->hasNext()) { | ||
return null; | ||
} | ||
return $this->list[$this->curRow++]; | ||
return $this->list[$this->curRow] ?? null; | ||
} |
There was a problem hiding this comment.
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() 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/IteratorFilterFormatter.php
Outdated
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'"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsafe String Value Escaping 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/RowOutput.php
Outdated
public function apply(RowInterface $row): RowInterface | ||
{ | ||
$newRow = new Row(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Violates Row Type Implementation 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/RowArray.php
Outdated
/** | ||
* @var mixed | ||
*/ | ||
private array $entity = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent type documentation 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/RowArray.php
Outdated
if (!isset($this->entity[$name])) { | ||
$this->entity[$name] = $value; | ||
return; | ||
} |
There was a problem hiding this comment.
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 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/RowValidator.php
Outdated
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 ]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validator Property Overwrite 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
foreach ($filterList as $filter) { | ||
$operator = $filter[self::OPERATOR]; | ||
$field = $filter[self::FIELD]; | ||
$relation = $filter[self::RELATION]; | ||
$value = $filter[self::VALUE]; |
There was a problem hiding this comment.
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 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/Row.php
Outdated
*/ | ||
protected bool $fieldNameCaseSensitive = true; | ||
private RowInterface $entity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflicting Type Documentation 
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
💬 Looking for more details? Reply to this comment to chat with Korbit.
|
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
Row
class to use a thin facade patternRowInterface
to standardize operationsRowArray
andRowObject
classes to handle different data typesRow
class now implementsRowInterface
and delegates to the appropriate implementationhasNext()
andmoveNext()
methods in favor of the standard PHPIterator
interfaceIteratorInterface
now extends PHP's nativeIterator
interfacevalid()
,current()
,next()
)count()
method from iteratorsBreaking Changes Table
Row
RowInterface
IteratorInterface
hasNext()
andmoveNext()
while ($iterator->valid()) { $row = $iterator->current(); $iterator->next(); }
GenericIterator
count()
count()
on the resultbyjg/xmlutil:^6.0
,byjg/serializer:^6.0
Other Enhancements
docs/
directorybyjg/anydataset-implementation: 1.0
provide in composer.jsonPHP Compatibility
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.