Return a string from the mocked recursiveTagParse#101
Merged
Conversation
MediaWiki master declares Parser::recursiveTagParse() and recursiveTagParseFully() with a `string` return type. The component test mocks stub them with returnArgument(0), echoing the input verbatim. When a component passes a non-string attribute value (a bool or int) to the parser, the mock returns that non-string and PHPUnit's typed mock throws "Return value must be of type string". Cast the echoed value to string so the mocks honour the method's contract. Behaviour is unchanged for string inputs, and the renderErrorMessage mock is left as-is. Co-Authored-By: Claude Opus 4.8 (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.
Why
MediaWiki master declares
Parser::recursiveTagParse()andrecursiveTagParseFully()with a
stringreturn type (the parameter stays untyped). The component tests mock thesewith
returnArgument(0), echoing the input verbatim. When a component parses an attributevalue that is a bool or int, the mock returns that non-string and PHPUnit's typed mock
throws:
Why an attribute value is a bool
This is not an unrealistic fixture.
ParserRequestbuilds its attribute array from what theparser passes in, and for the parser-function syntax a valueless option is stored as a PHP
true(an empty one asfalse) byParserRequest::getKeyValuePairFrom(). So[ 'disabled' => true ]is exactly what{{#bootstrap_button:text|disabled}}produces; thetag-extension syntax (
<bootstrap_button disabled>) yields a string instead. Both reachrecursiveTagParse().In production the real
recursiveTagParse()coerces such a value and returns a string, so itworks. The mock's
returnArgument(0)did not coerce, which is the only reason the typedreturn now fails.
What
Make the
recursiveTagParse/recursiveTagParseFullymocks return(string)$text, so themock behaves like the real parser and honours the
stringreturn contract. The realisticbool/int inputs are kept and no assertions change (
truestays truthy as"1",0staysfalsy as
"0"). TherenderErrorMessagemock is left as-is.Verification
Test-only change. On MediaWiki master the component tests that hit the
TypeErrornow pass;the older rows are unaffected, since the cast is a no-op for the string inputs they use.