#232 - Fix edge cases looping forever or failing with a TypeError - #233
Merged
Merged
Conversation
…Error`: validate FileSplitterTask `max_lines` (greater than 0, also when given as input), catch any `\Throwable` in PropertySetterTask, type SlugifyTransformer options, explicit exceptions in InputFolderBrowserTask (no folder path) and `XmlFile::write()` (`saveXML()` failure). Update documentation, 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
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 #232.
Five edge cases, found while fixing #202, #203, #204, #205 and #208, either looped forever or failed with a raw PHP
TypeErrorinstead of an explicit error. This PR:FileSplitterTask:max_linesmust be an integer greater than 0 (it used to loop forever withmax_lineslower than 1). The options given as input ({ file_path: ..., max_lines: ... }) are now validated like the task options: they are resolved again after the merge, and input keys that are not options are ignored.PropertySetterTask: catches\Throwableinstead of\Exception, so theproperty/valueerror context is also set when the PropertyAccessor throws aTypeError(e.g. on a scalar input).SlugifyTransformer:transliterator,replaceandseparatormust be strings (setAllowedTypes(), as documented), instead of aTypeErrorinTransliterator::create()/trim().InputFolderBrowserTask: an empty input (null,'') with no folder being browsed throws an explicit\UnexpectedValueException('No folder path given as input')instead ofTypeError: is_dir().XmlFile::write()(XmlWriterTask): throws the intended\RuntimeException('Could not generate the XML content')whenDOMDocument::saveXML()fails, instead ofTypeError: fwrite().The reference documentation of the five classes is updated.
The 10 new tests fail on
mainand pass with this fix. PHPUnit (with coverage), PHPStan, PHP-CS-Fixer and Rector pass, and the changed files are valid PHP 8.2.Requirements
Breaking changes
None: every affected case looped forever or threw a
TypeError; it now throws an explicit exception (anInvalidOptionsExceptionwhen resolving the options, for 1 and 3). ForFileSplitterTask, input keys that are not options are no longer merged into the options (they were not used).🤖 Generated with Claude Code