[REF][PHP8.2] Avoid dynamic properties in civicrm_api3 class #25253
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.
Overview
Avoid dynamic properties in civicrm_api3 class.
Before
Several properties set and retreived dynamically, which is deprecated in PHP 8.2. See https://lab.civicrm.org/dev/core/-/issues/3833 for context.
After
The properties are defined upfront, and are not dynamic.
Technical Details
local
public. It felt like there could be use-cases for third-party code to be able to see whether an API3 object is local or not. Furthermore, the property needs to be public as it is used in tests.input
,lastResult
,uri
,key
,api_key
,referer
anduseragent
are also public. Some of these are also used by tests, but I also gathered that given how widely usedcivicrm_api3
is in the ecosystem there may well be use-cases where these are being read. That said, I'm not sure we really want to be advertisting these as part of the public API, and so I have marked them as@internal
.cfg
andcurrentEntity
felt very implementation specific and so have been made protected.I've also changed the initial value of
lastResult
set in the constructor, to match the type it has elsewhere in the class (once the request has been made)Happy to hear arguments for why any of these should change.
Comments
Reading through the code I couldn't really understand what
$this->input
is doing, and I suspect it's a bit buggy. Given everything is moving to APIv4 it probably doesn't warrant too much time spent looking into it, and for this PR I just wanted to get the dynamic properties righted out.