Automated Drupal 12 compatibility fixes for geofield 10.3.x-dev

The pipeline for commit 3e4efe0 reports cspell, PHPCS, PHPStan, and stylelint warnings, and PHPUnit failures.

The commit fixes the PHPCS findings in the two plugin managers and the PHPStan finding in the hook implementation. Local PHP syntax checks, PHPCS on the changed files, and diff checks pass. The PHPStan log no longer reports the changed hook or plugin managers, but the job still reports 45 errors elsewhere, mostly in existing code and unmatched baseline entries. The cspell and stylelint findings are in unchanged files; PHPCS also has findings outside the changed files.

The PHPUnit diagnostics include a `testBoundsWidget` 500 error in core's `field-multiple-value-form.html.twig`, where Twig received an array instead of an escape strategy, and formatter errors where `EscaperRuntime::escape()` received `null` for its autoescape argument. A run with Drupal 11.4.6 and Twig 3.30 produced these errors. They are consistent with the Twig compatibility problem tracked in https://www.drupal.org/project/drupal/issues/3625969, which has since been fixed in core. The `ConstraintsTest` tests for `GeoConstraintValidator` pass. Three other widget assertions could not find the expected `geofield_field` text; I have not established whether those failures are separate from the Twig issue.

After the GitLab Templates update tracked in https://git.drupalcode.org/project/gitlab_templates/-/work_items/3572442, I will manually start a new pipeline and run the Drupal 12 jobs, then check whether the PHPUnit failures remain.

Update: Because of that temporary problem, I pinned automatic pipelines from this branch to Drupal 11.4.8 with CORE_STABLE in .gitlab-ci.yml. Composer, Composer (next major), regular PHPUnit, cspell, ESLint, and stylelint passed.

The Drupal 12-dev 'PHPUnit (next major)' run executed all 41 tests without assertion failures, but the job exited nonzero because of deprecation notices, including notices for plugin annotations scheduled for removal in Drupal 13. PHPCS and PHPStan still report broader project findings, as they do in the standard Drupal 11.4.8 jobs. Targeted checks of the compatibility files pass, and the 'PHPStan (next major)' report does not flag those files.

I also fixed the few reported CSpell and Stylelint findings, a documentation typo, and PHPCS documentation findings in the test file.

Finally, I removed the temporary CORE_STABLE override, and the latest pipeline ran using the just-updated default, with the expected broader PHPCS/PHPStan findings still remaining.

This MR was prepared with AI assistance and definitely needs maintainer review before acceptance.

Tested MR !48 on Drupal 12.0.0-beta1 with Mapillary (https://www.drupal.org/project/mapillary), which stores image, sequence, map feature and detection positions in Geofield fields and shows them with Leaflet.

Environment

• Drupal 12.0.0-beta1 (Symfony Validator 8.1.8), PHP 8.5.9, PHPUnit 12.5.39, SQLite 3.46.1.
• Geofield at the MR branch codex/3598572-d12-compatibility (a9cfa07a).
• Entity API and Leaflet at their Drupal 12 MR branches (#3553240: Convert hooks to OOP MR !47, #3599631: Automated Drupal 12 compatibility fixes for leaflet 10.4.x-dev MR !75); Mapillary 1.0.0-alpha6.

Geofield 10.3.4 is not usable on Drupal 12

10.3.4 declares ^12 since #3572547: Add Drupal 12 to core version requirements to allow next major testing, so Composer installs it on Drupal 12. Any code path that builds the constraint plugin definitions (installing config, saving an entity) then stops with a fatal error:

PHP Fatal error: Declaration of Drupal\geofield\Plugin\Validation\Constraint\GeoConstraintValidator::validate($value, Symfony\Component\Validator\Constraint $constraint) must be compatible with Symfony\Component\Validator\ConstraintValidator::validate(mixed $value, Symfony\Component\Validator\Constraint $constraint): void

With 10.3.4, 38 of Mapillary's 52 Kernel classes failed on it.

Result with the MR

• The new validate(mixed $value, Constraint $constraint): void signature fixes the fatal error.
• Mapillary Unit, Kernel and Functional tests (98 classes) run; no failure comes from Geofield. That includes saving entities with Geofield values, Leaflet widgets and formatters on Geofield fields, and bounding box entity queries on geometry.lat and geometry.lon.
• Geofield still triggers Drupal 13 deprecations: annotation-based plugins (@FieldType, @FieldWidget, @FieldFormatter, @GeofieldBackend, @Constraint and the Views plugins) and return types the DebugClassLoader asks for on the field type, widget and formatter methods.

Given the fatal error, it may be worth a release with this fix before Drupal 12.0.0, since 10.3.4 already claims Drupal 12 support.

Hey ... I have not great availability these days to catch up and review everything here.

And everything is looking quite messy over here.

Would it be possible to have a unique MR that wraps and fixes every Drupal 12 compatibility ?
Or, what should be the apply order on the above, otherwise?

This comment was drafted by an LLM before it was reviewed by me.

@itamair, !50 is now the single merge request for Drupal 12. It contains:

• the GeoConstraintValidator::validate() signature fix (a fatal error on Drupal 12 with Symfony 8)
• GeofieldBackend and GeofieldProximitySource attribute classes for the two plugin managers (Drupal 12 throws an exception for annotation-only plugin managers)
• everything from !48, merged with Nick Hope's commits intact: OOP hooks for hook_theme() and hook_field_views_data() (Drupal 12 no longer loads geofield.views.inc), PHPUnit attributes on the tests, and its lint and CI changes
• attributes instead of annotations for the field type, widgets, formatters, constraint, form elements, migrate process plugins and Views handlers, so the Drupal 12 test job doesn't fail on deprecation notices

Where !48 and !50 overlapped (the plugin managers), !50 keeps dedicated attribute classes instead of the version_compare() branch. Once !50 is merged, !47 and !48 can be closed.

thx @tbcs for these clarifications ... will try to find some time to review asap.
In the meanwhile everybody else is welcome to stress test all this on Drupal 12,
and eventually add a +1 to RTBC ...

I tested !50 on Drupal 12.0.0-beta2 (PHP 8.5). All tests passed, but there were 35 deprecations from Geofield, so the next major job would fail. I pushed three commits on top of !50:

1. Add no_ui to the GeofieldProximitySource attribute. Without it, a plugin using no_ui breaks discovery. Also add PluginDiscoveryTest.
2. Add @return tags to 60 methods. This removes the deprecations. I did not use native return types, because Leaflet and Geofield Map extend these widgets and would break.
3. Fix the PHPCS errors and the PHPStan findings. PHPStan now finds the geoPHP classes, so the baseline is down from 68 to 16 entries.

Results:

• Drupal 12: 47 tests pass, no deprecations from Geofield. The 3 left come from JSON:API Extras.
• Drupal 11: Kernel and Unit tests pass, PHPStan has no findings and PHPCS has no errors.
• Pipeline #1018593 is green, including PHPCS and PHPStan. The next major jobs did not run, so the Drupal 12 results are from my local run.

Not tested: Drupal 10.3, Leaflet and Geofield Map.

These commits were done with the assistance of models like Claude Opus 5.5 and Cyber Tiel Coder.

@nick hope, @tbcs, can you also review my commits too?

@orkutmuratyilmaz, thanks — I reviewed your commits.

• The no_ui property was a real gap in the attribute I added; the manager reads it, and the new test module covers it.
• The @return tags are the right call. The only subclasses I found in current releases are LeafletDefaultWidget (Leaflet 10.4.13) and GeofieldMapWidget (Geofield Map 11.1.9), and both still load since only docblocks changed.
• The widget default values, the protected properties and the ConstraintsTest mock all look correct.

I tested the branch on Drupal 10.3.14 (PHPUnit 9.6), which was missing: all unit and kernel tests pass, including PluginDiscoveryTest, and a temporary kernel test confirmed that every attribute-based plugin (field type, widgets, formatters, constraint, form elements, migrate process plugins, Views handlers) is discovered there. On Drupal 11.5 all tests pass, including the functional widget tests, apart from the JSON:API Extras tests, which need that module.

I ran the Drupal 12 jobs on Gitlab and they had some warnings, so I made a few small changes to the test to make it 100% compatible with Drupal 12.

Thanks both for consolidating and testing this.

I had Codex (GPT-6.1 Sol) review !50 at 769ccb35 and its CI logs. It found the changes sound overall and confirmed all 47 tests passed on Drupal 11 and 12. I haven’t independently rerun them.

Its one suggestion concerns this class-wide suppression:
https://git.drupalcode.org/issue/geofield-3598572/-/blob/769ccb35146afce...

php
#[IgnoreDeprecations]
class GeofieldGeoJsonEnhancerTest extends UnitTestCase {

Could handling be limited to the three known JSON:API Extras notices, keeping future Geofield warnings visible? Codex noted that PHPUnit 11 compatibility needs care. Happy to ask it to investigate a small follow-up here, such as replacing the blanket suppression with message-specific ignores and checking both PHPUnit versions.

Thanks @tbcs, for the review and the Drupal 10.3 run, and @nick hope, for the Codex pass.

Well, we have the #[IgnoreDeprecations] on GeofieldGeoJsonEnhancerTest issue. The three notices come from ResourceFieldEnhancerBase in JSON:API Extras. Its getInternalValidator(), getInputValidator() and getOutputValidator() have no @return tags.

I already created a new issue, #3633496: ResourceFieldEnhancerBase triggers return type deprecations on Drupal 12, for this. Once it is fixed there, the attribute can be removed and Geofield has nothing to ignore.

Until then, I am fine with narrowing it to the three messages if that works on both PHPUnit 11 and 12. If it gets complicated, a @todo next to the attribute pointing to #3633496: ResourceFieldEnhancerBase triggers return type deprecations on Drupal 12 would make clear why it is there and when it can go.

I just created an MR for that issue too.

MR !51 was created upon the MR !50
to remove every Annotation Plugins, both definition and references I found to them.

The Geofield module in this version supports the following Drupal Core(s):

core_version_requirement: ^10.3 || ^11 || ^12

and Attributes plugins were introduced by Drupal core 10.2 ...
Right?

So we should be able to get rid of Annotations plugins.
(I manually tested MR !51 with a clean Drupal 10.3 instance and everything looked working fine ... apparently)

Thanks for opening the upstream fix. A TODO linking to it sounds reasonable while it’s pending.

To help answer #25, I asked Codex (GPT-6.1 Sol) to create a small test module containing annotation-based backend and proximity-source plugins, and check whether Geofield still discovers them. On Drupal 11.4.11 and Drupal 12-dev, !50 passes all eight discovery cases. With the same test patch, !51 passes the six existing cases, but both new cases fail because the annotated plugins are missing from discovery.

So this implies that removing the fallbacks would break discovery of existing annotation-based plugins supplied by other modules (including custom modules that aren’t publicly available). Retaining them would preserve that compatibility while we get the D12 fixes merged. Drupal’s migration guidance also recommends retaining the annotation class alongside the attribute class for existing plugin types, to avoid backwards-compatibility breaks: https://www.drupal.org/node/3395582

I’ve pushed the test-only change as commit 43430d8, available to be incorporated into whichever MR goes forward, if useful: https://git.drupalcode.org/issue/geofield-3598572/-/commit/43430d831e860...

No production code changed; expected annotation deprecations are checked explicitly. I haven’t independently rerun these tests outside Codex.

thanks @nick hope ... definitely agreed.
MR51! cancelled/removed ...

I run some major cleanup ... (there were too many MRs and branches, etc.).
AI coding agents are producing too much and quick code and is very hard to stay after and aligned with all this.

Let's keep the MR!50 the unique one here ... as according @tbcs it should be considered that way.

@tbcs up to you if the latest "codex/3598572-legacy-discovery-test" branch by @nick hope looks not obvious / redundant and should also be merged.

That said, all looks great work and update in the Geofield module and its D12 compatibility on MR!50, at the moment,
with all tests passing.

Let me know when that is finally ready to be merged into the 10.3.x branch (as it looks to me).

Yeah, those changes look reasonable and minimal enough. Cherry-picked.

Ok, let's go. All this looks pretty solid.
Merging MR !50 into 10.3.x branch ...

new geofield 10.3.5 release just deployed with this ...

thanks for the fix:)