Field item stubs should allow null for stored properties

Open Beginner friendly
  • Is something wrong?
#1,078 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
php
Domain
tooling

Research direction

Start with the affected field item stubs and matching assertions in tests/src/Type/data/field-types.php, using the table and core behavior described in the issue. Update each listed property to its nullable type and include |null in the matching assertions. Done when all listed cases are reflected in the stubs and assertions.

Written by the indexing model from the issue text.

Description

Several field item stubs type stored properties as non-nullable, but core expects them to be NULL sometimes. StringItemBase is the clearest case:

/**
 * @property string $value
 */
abstract class StringItemBase extends FieldItemBase {
}

// Core's StringItemBase::isEmpty()
public function isEmpty() {
  $value = $this->get('value')->getValue();
  return $value === NULL || $value === '';
}

An item from appendItem(), or one on an unsaved entity, returns NULL from ->value. setRequired(TRUE) on a property definition only adds a validation constraint, and none of these columns are 'not null' in the schema. With the current stubs, PHPStan reports $item->value ?? '' as unnecessary, and $item->value = NULL; fails with "does not accept null."

The PathItem stub from #1007 already uses nullable types. Found while reviewing #1073, which adds a TelephoneItem stub.

Affected stubs

Stub Property Change to Evidence in core
StringItemBase value ?string isEmpty() checks === NULL. Also covers StringItem, StringLongItem, UriItem, UuidItem, PasswordItem.
EmailItem value ?string isEmpty() checks === NULL.
DecimalItem value ?string NumericItemBase::isEmpty() uses empty($value).
IntegerItem value ?int Same as DecimalItem.
FloatItem value ?float Same as DecimalItem.
BooleanItem value ?int No isEmpty() override. Map::isEmpty() treats NULL as empty.
TimestampItem value ?int Same as BooleanItem.
LanguageItem value ?string Same as BooleanItem.
LinkItem uri ?string isEmpty() checks uri === NULL.
LinkItem title ?string Not required. getTitle() returns $this->title ?: NULL.
FileItem description ?string Not required. Only set when the description field is enabled.
FileItem display ?bool Not required. Only set when the display field is enabled.

Not affected

  • FileUriItem::$url is computed. ComputedFileUrl::getValue() always returns a string.
  • TextItemBase::$processed and TextWithSummaryItem::$summary_processed are computed and read-only.

Needs a closer look

These probably can be NULL too, but I didn't confirm it:

  • LanguageItem::$language is a computed reference built from value. When value is NULL, there is no language to load.
  • LinkItem::$options is a Map. Map::$values has no default, so an item with no options set may return NULL instead of [].
  • PasswordItem::$pre_hashed is a boolean property with no default value.
  • MapItem::$value has its own storage handling.

To do

  1. Update each stub in the table above to the nullable type.
  2. Update the matching assertions in tests/src/Type/data/field-types.php to include |null.
Dominant language
PHP
Stars
208
Forks
88
Avg merge
2d 17h
Merged PRs (30d)
15

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from mglaman/phpstan-drupal

All issues in mglaman/phpstan-drupal

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.