#228 - Add missing setAllowedTypes() on several options - #230
Merged
Merged
Conversation
…mer (`raw_output`), SimpleBatchTask (`batch_count`), ConditionTrait (`empty`, `not_empty`), NormalizerTask / SerializerTask / DeserializerTask (`context`) and ObjectUpdaterTask (`property_path`): a wrong type is now reported when the options are resolved. Add tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #228.
Several options have no
setAllowedTypes(): a value of the wrong type is accepted when the options are resolved, and fails later (during the execution) with an unclear error, or is silently misused.HashTransformerraw_outputboolSimpleBatchTaskbatch_countint|nullConditionTrait(FilterTask,array_filter,RulesTransformer...)empty,not_empty(the other condition keys are already typedarray)arrayNormalizerTask,SerializerTask,DeserializerTaskcontext(already typed inDenormalizerTask)arrayObjectUpdaterTaskproperty_pathstring(
CsvSplitterTask::max_lines, also listed during the documentation review, was fixed in #221.)This PR:
setAllowedTypes(), matching the reference documentation:raw_output→bool,batch_count→int|null(null= only output on flush, as already supported by the code and the documentation, and asIterableBatchTaskafter Several tasks mishandle edge cases: AdvancedStatCounterTask off-by-one, IterableBatchTask rejects a null batch_count, ConditionTrait TypeError on scalars, ColumnAggregatorTask treats null as missing #206),empty/not_empty→array,context→array,property_path→string|PropertyPathInterface(the PropertyAccessor accepts both).InvalidOptionsException, valid values are accepted).The new regression tests fail on
mainand pass with this fix. PHPUnit, PHPStan, PHP-CS-Fixer and Rector pass.Requirements
Breaking changes
Almost none: only values that do not match the documented types are now rejected when the options are resolved. Most of them already failed at execution time; a few may have worked by accident (e.g.
batch_count: '10'as a string, orraw_output: 1in non-strict code), they must now use the documented type.🤖 Generated with Claude Code