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

fix: Allow enum param types: ArrayParameterType and ParameterType #1408

Merged
merged 6 commits into from
Mar 6, 2024

Conversation

michalbundyra
Copy link
Member

@michalbundyra michalbundyra commented Mar 6, 2024

Q A
Type bug
BC Break no
Fixed issues #1407

Summary

Fixes #1407

A bit lame fix, but it looks like we did not really use other int types before anyway

With this change we can use DBAL v4 new enum types: ArrayParameterType and ParameterType.

Bumping version of DBAL to 3.6 as before that ArrayParameterType did not exist.

Just to note - the problem itself is ONLY with DBAL 4.0+ as on 3.6 these types were classes with constants, not enums.

Fixes doctrine#1407

A bit lame fix, but it looks like we did not really use other int types before anyway

With this change we can use DBAL v4 new enum types: `ArrayParameterType` and `ParameterType`.

Bumping version of DBAL to 3.6 as before that `ArrayParameterType` did not exist.
@michalbundyra michalbundyra changed the title fix: Allow new param types fix: Allow enum param types: ArrayParameterType and ParameterType Mar 6, 2024
@derrabus
Copy link
Member

derrabus commented Mar 6, 2024

Thank you. Please add a test that covers your change.

@derrabus derrabus added the Bug label Mar 6, 2024
@derrabus derrabus added this to the 3.7.4 milestone Mar 6, 2024
@@ -53,7 +55,7 @@ public function formatParameters(array $params, array $types): string
return sprintf('with parameters (%s)', implode(', ', $formattedParameters));
}

private function formatParameter(mixed $value, string|int $type): string|int|bool|float|null
private function formatParameter(mixed $value, string|int|ArrayParameterType|ParameterType $type): string|int|bool|float|null
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should change the type to mixed. After all, the method discards any value that is not a string anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

@derrabus pushed - if we ok with mixed, then I can even change back the bump in composer.json, but then we would not have a test case for ArrayParameterType as it does not exist in DBAL 3.5

@derrabus derrabus merged commit 954e0a3 into doctrine:3.7.x Mar 6, 2024
12 checks passed
@michalbundyra michalbundyra deleted the fix/param-type branch March 6, 2024 13:42
@michalbundyra
Copy link
Member Author

@derrabus thanks!

@derrabus
Copy link
Member

derrabus commented Mar 6, 2024

Thank you!

derrabus added a commit that referenced this pull request Mar 6, 2024
* 3.7.x:
  fix: Allow enum param types: ArrayParameterType and ParameterType (#1408)
@derrabus
Copy link
Member

derrabus commented Mar 6, 2024

Let's do the bump in #1411.

derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Fix CS
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DBAL 4.0 and ArrayParameterType
2 participants