Skip to content
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

RexFunctionsDynamicReturnTypeExtension - fix default arg handling #678

Merged
merged 3 commits into from
Mar 16, 2024

Conversation

gharlan
Copy link
Member

@gharlan gharlan commented Mar 13, 2024

Bei den Funktionen rex_get()/rex_post() etc. wurde mit dem Default-Wert falsch umgegangen.

Wenn man den dritten Param weglässt oder explizit '' übergibt, ging rexstan bisher davon aus, dass der leere String ein möglicher Rückgabewert ist.
Tatsächlich hat der leere String als Defaultwert aber eine andere Bedeutung. Wenn der Key nicht existiert, dann wird der leere String auf den gewünschten Typ gecastet, also bei int auf 0 etc.

Nur wenn man also einen anderen Wert als Defaultwert übergibt, muss dieser als möglicher Rückgabewert ergänzt werden.

if (count($args) >= 3) {
$defaultArgType = $scope->getType($args[2]->value);
if (!$defaultArgType instanceof ConstantStringType || $defaultArgType->getValue() !== '') {
Copy link
Member

@staabm staabm Mar 14, 2024

Choose a reason for hiding this comment

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

Hier muss muss stattdessen mit getConstantStrings() gearbeitet werden.

instanceof XYType ist in der regel ein fehler da man damit implementierungs-details vorraussetzt. nicht nur der "ConstantStringType" kann konstante strings enthalten.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah verstehe. War mir da auch unsicher, wie ich es notieren muss. Wobei ich mir auch jetzt unsicher bin, ob es so jetzt richtig notiert ist?

Copy link
Member

Choose a reason for hiding this comment

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

Der test sagt es passt

8e89b81

@staabm staabm force-pushed the rex-functions-default-arg branch from 601b721 to a874ce4 Compare March 16, 2024 05:20
@staabm staabm merged commit d55091f into main Mar 16, 2024
7 checks passed
@staabm staabm deleted the rex-functions-default-arg branch March 16, 2024 05:23
@staabm
Copy link
Member

staabm commented Mar 16, 2024

released

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.

2 participants