Add param strictness where possible.

[dereuromark]

rebased https://github.com/propelorm/Propel2/pull/1831

WIP

[dereuromark]

Any idea about the failing tests and errors?

[oojacoboo]

@dereuromark haven't looked into the code, but this one is related to the number of columns in the select not matching up with the table. This is a common problem with Propel due to its design. Not sure why the typing caused this, but I guess the query builder isn't properly interpreting types.

On second look, seems like it could be related to query params in a prepared statement.

[dereuromark]

For some reason it affects only PHP81 builds

[oojacoboo]

For StatementWrapper::execute we should drop support for $inputParameters being nullable and work our way backwards from that. The issue appears to be in the execution of the prepared statement there. I'm assuming that args and $inputParameters are an issue. I don't know why without debugging it at runtime. But tightening up the typing will help pinpoint the issue.

[dereuromark]

Well, that didnt really work, broke all builds with really unclear messages. It seems to be quite cloaked, what the actual issue is.

[oojacoboo]

Well, that would, likely break a lot - anything explicitly passing in null to execute. The default value would become an empty array though for all the cases where there aren't any arguments. But that's the point, to ensure that they're properly passing in an array. A null value being passed in is prime for errors in unforeseen ways.

What's in your SQL logs from Propel? callUserFunctionWithLogging logs the error. You need to find out why there is a mismatch in the args being passed

[codecov-commenter]

Codecov Report

Merging #1833 (f6e5558) into master (a9b02ae) will decrease coverage by 14.47%. The diff coverage is 69.72%.

@@              Coverage Diff              @@
##             master    #1833       +/-   ##
=============================================
- Coverage     87.69%   73.22%   -14.48%     
- Complexity     7759     7763        +4     
=============================================
  Files           282      282               
  Lines         21315    21220       -95     
=============================================
- Hits          18693    15539     -3154     
- Misses         2622     5681     +3059     

Flag	Coverage Δ
5-max	73.22% <69.72%> (-14.48%)	:arrow_down:
7.4	73.22% <69.72%> (-14.48%)	:arrow_down:
agnostic	?
mysql	68.98% <67.88%> (+0.05%)	:arrow_up:
pgsql	69.00% <67.39%> (+0.05%)	:arrow_up:
sqlite	66.84% <65.84%> (+0.05%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pel/Common/Config/Exception/JsonParseException.php	0.00% <0.00%> (-20.00%)	:arrow_down:
src/Propel/Common/Config/Loader/LoaderResolver.php	100.00% <ø> (ø)
...l/Common/Exception/SetColumnConverterException.php	0.00% <0.00%> (-100.00%)	:arrow_down:
...nerator/Behavior/Archivable/ArchivableBehavior.php	0.00% <0.00%> (-91.47%)	:arrow_down:
...ivable/ArchivableBehaviorObjectBuilderModifier.php	0.00% <0.00%> (-97.02%)	:arrow_down:
...hivable/ArchivableBehaviorQueryBuilderModifier.php	0.00% <0.00%> (-95.24%)	:arrow_down:
...l/Generator/Behavior/Delegate/DelegateBehavior.php	0.00% <0.00%> (-90.84%)	:arrow_down:
...Generator/Behavior/NestedSet/NestedSetBehavior.php	0.00% <0.00%> (-92.86%)	:arrow_down:
...stedSet/NestedSetBehaviorObjectBuilderModifier.php	0.00% <0.00%> (-96.66%)	:arrow_down:
...estedSet/NestedSetBehaviorQueryBuilderModifier.php	0.00% <0.00%> (-91.12%)	:arrow_down:
... and 307 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a9b02ae...f6e5558. Read the comment docs.

[dereuromark]

@oojacoboo After https://github.com/propelorm/Propel2/pull/1841 merged in, we could then merge to master. Does all look good now?

[oojacoboo]

@dereuromark Overall, this looks great. There is obviously a ton of changes here. It'd be nice to have typed properties as well. I find it's easier to type methods and properties at the same time. However, seeing as this is already done, that'd be better in another PR. I added some comments.

[dereuromark]

Changes done as per review. Shall we continue? We can make follow up PRs then for more cleanup and get things ready for new release.

[dereuromark]

@oojacoboo Do you want to make follow up PRs regarding more types? Especially also around generated code?

[oojacoboo]

@dereuromark I think we need to get property typing done.