Problem/Motivation

The preSave() method in PrimaryEntityReferenceFieldItemList uses the spaceship operator (<=>) to sort items by their 'primary' property. While Drupal's typed data system should enforce the correct boolean type, adding explicit type casting provides defense in depth and makes the code's intent clearer.

Location: src/Plugin/Field/PrimaryEntityReferenceFieldItemList.php, lines 32-34

Current Code

public function preSave() {
  usort($this->list, function ($a, $b) {
    return $b->get('primary')->getValue() <=> $a->get('primary')->getValue();
  });

  parent::preSave();
}

Potential Issues

  • If the 'primary' property isn't properly validated, type juggling could cause unexpected sort behavior
  • The wrong item could be marked as primary if values are strings like "1" vs integers
  • PHP's loose typing could cause "0" (string) and 0 (integer) to compare differently than expected

Likelihood

Low - Drupal's typed data system enforces types and the 'primary' property is defined as a boolean in the field schema. However, explicit type casting improves code robustness and clarity.

Proposed resolution

Add explicit type casting to ensure consistent integer comparison regardless of the actual data type stored.

Recommended Implementation

public function preSave() {
  usort($this->list, function ($a, $b) {
    // Explicit casting ensures consistent comparison
    return (int) $b->get('primary')->getValue() <=> (int) $a->get('primary')->getValue();
  });

  parent::preSave();
}

Benefits

  • Defense in depth: Protects against unexpected type issues
  • Code clarity: Makes the intent explicit that we're comparing integers
  • Consistency: Ensures predictable behavior regardless of data source
  • Performance: No performance penalty as type casting is negligible

Remaining tasks

  • Add explicit (int) type casting to both sides of the spaceship operator
  • Verify existing unit tests still pass
  • Add unit test cases for edge cases (null values, string values, etc.)
  • Review other comparison operations in the module for similar issues
  • Update code comments to document the type expectation

User interface changes

None

API changes

None. This is an internal implementation detail that doesn't affect the public API.

Behavioral Changes

The behavior should remain unchanged for correctly typed data. If any edge cases existed where incorrect types were being compared, those will now be handled consistently.

Data model changes

None

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bluegeek9 created an issue. See original summary.

  • bluegeek9 committed 61e48ced on 1.0.x
    feat: #3570441 Add Explicit Type Casting in Primary Item Sort Comparison
    
bluegeek9’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.