Skip to content

Removed the redundant default value from the 'mainProperty' field handler property. - #388

Merged
AlexSkrypnyk merged 1 commit into
masterfrom
feature/fresh-deps-lint
Jul 28, 2026
Merged

Removed the redundant default value from the 'mainProperty' field handler property.#388
AlexSkrypnyk merged 1 commit into
masterfrom
feature/fresh-deps-lint

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Removed the redundant = NULL default from AbstractHandler::$mainProperty in src/Drupal/Driver/Core/Field/AbstractHandler.php. The property is a nullable string, but every non-throwing path through AbstractHandler::__construct() assigns it unconditionally from $this->fieldInfo->getMainPropertyName(), so the NULL default was never actually observed by any caller. A fresh dependency install (vendor/ and composer.lock removed, then composer update re-resolved) pulled in rector/rector 2.5.8, and its RemoveDefaultValueFromAssignedPropertyRector rule (deadCode set) flagged the declaration as dead code.

Changes

  • src/Drupal/Driver/Core/Field/AbstractHandler.php: changed protected ?string $mainProperty = NULL; to protected ?string $mainProperty;, keeping the nullable type (normalise() still checks $this->mainProperty === NULL for handlers without a main property) while removing the default value that the constructor always overwrites.

Before / After

BEFORE
┌──────────────────────────────────────────────────┐
│ protected ?string $mainProperty = NULL;           │
└──────────────────────────────────────────────────┘
                       │
                       ▼
           property initialized to NULL
                       │
                       ▼
              __construct() runs
                       │
                       ▼
 $this->mainProperty = $this->fieldInfo->getMainPropertyName();
                       │
                       ▼
        NULL default silently overwritten
   (redundant: every non-throwing path reaches this line)

AFTER
┌──────────────────────────────────────────────────┐
│ protected ?string $mainProperty;                  │
└──────────────────────────────────────────────────┘
                       │
                       ▼
          property left uninitialized
                       │
                       ▼
              __construct() runs
                       │
                       ▼
 $this->mainProperty = $this->fieldInfo->getMainPropertyName();
                       │
                       ▼
          first and only assignment

Summary by CodeRabbit

  • Refactor
    • Simplified an internal property declaration without changing user-visible behavior.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 98fcf6af-21bd-4c5c-b82d-d1dc7b838f9a

📥 Commits

Reviewing files that changed from the base of the PR and between 8176be0 and 29336d5.

📒 Files selected for processing (1)
  • src/Drupal/Driver/Core/Field/AbstractHandler.php

📝 Walkthrough

Walkthrough

AbstractHandler::$mainProperty is now declared as a nullable string without an explicit NULL initializer. Its constructor assignment and all other behavior remain unchanged.

Changes

Field handler property

Layer / File(s) Summary
Nullable property declaration
src/Drupal/Driver/Core/Field/AbstractHandler.php
Removes the explicit NULL default from $mainProperty while retaining its nullable string declaration.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change to the mainProperty field handler property.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fresh-deps-lint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 95%)



Code Coverage Report Summary:
  Classes: 58.82% (20/34)
  Methods: 89.52% (222/248)
  Lines:   96.28% (1241/1289)

Per-class coverage
Drupal\Driver\Alias\CreationAliasRegistryTrait               100.00%
Drupal\Driver\Alias\RolesAlias                                87.50%
Drupal\Driver\BlackboxDriver                                 100.00%
Drupal\Driver\Core\Alias\AuthorAlias                          90.48%
Drupal\Driver\Core\Alias\ParentTermAlias                      88.24%
Drupal\Driver\Core\Alias\VocabularyMachineNameAlias           88.89%
Drupal\Driver\Core\Core                                       95.81%
Drupal\Driver\Core\Field\AbstractHandler                      95.74%
Drupal\Driver\Core\Field\AddressHandler                       94.83%
Drupal\Driver\Core\Field\BooleanHandler                      100.00%
Drupal\Driver\Core\Field\ColorFieldTypeHandler                 0.00%
Drupal\Driver\Core\Field\DateRecurHandler                    100.00%
Drupal\Driver\Core\Field\DaterangeHandler                    100.00%
Drupal\Driver\Core\Field\DatetimeHandler                     100.00%
Drupal\Driver\Core\Field\DefaultHandler                      100.00%
Drupal\Driver\Core\Field\EmbridgeAssetItemHandler              0.00%
Drupal\Driver\Core\Field\EntityReferenceHandler               94.44%
Drupal\Driver\Core\Field\EntityReferenceRevisionsHandler      86.05%
Drupal\Driver\Core\Field\FieldClassifier                      94.44%
Drupal\Driver\Core\Field\FieldShapeClassifier                100.00%
Drupal\Driver\Core\Field\FileHandler                         100.00%
Drupal\Driver\Core\Field\ImageHandler                        100.00%
Drupal\Driver\Core\Field\LinkHandler                          94.00%
Drupal\Driver\Core\Field\ListFloatHandler                    100.00%
Drupal\Driver\Core\Field\ListHandlerBase                     100.00%
Drupal\Driver\Core\Field\ListIntegerHandler                    0.00%
Drupal\Driver\Core\Field\ListStringHandler                     0.00%
Drupal\Driver\Core\Field\NameHandler                          96.36%
Drupal\Driver\Core\Field\OgStandardReferenceHandler            0.00%
Drupal\Driver\Core\Field\SmartdateHandler                    100.00%
Drupal\Driver\Core\Field\SupportedImageHandler               100.00%
Drupal\Driver\Core\Field\TextHandler                           0.00%
Drupal\Driver\Core\Field\TextLongHandler                       0.00%
Drupal\Driver\Core\Field\TextWithSummaryHandler                0.00%
Drupal\Driver\Core\Field\TimeHandler                          91.67%
Drupal\Driver\DrupalDriver                                    98.86%
Drupal\Driver\Drush\DrushResult                              100.00%
Drupal\Driver\DrushDriver                                    100.00%
Drupal\Driver\Entity\EntityStub                              100.00%
Drupal\Driver\Exception\BootstrapException                   100.00%
Drupal\Driver\Exception\CreationAliasResolutionException       0.00%
Drupal\Driver\Exception\Exception                            100.00%
Drupal\Driver\Exception\UnsupportedDriverActionException     100.00%

@AlexSkrypnyk
AlexSkrypnyk merged commit 18ae58f into master Jul 28, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant