Config::get return type extension

[mstrelan]

I vibe-coded a solution to #753. Please accept my apologies, as I don't have enough experience with this repo to verify if this is a good approach, so please only review as a proof of concept.

The TL;DR is:

• Narrows Config::get() return types using Drupal's config schema files, so calls like \Drupal::config('system.cron')->get('logging') resolve to bool|null instead of mixed. Only applies to configs with the FullyValidatable constraint, which guarantees the schema is complete. Types are always nullable since any key can be absent at runtime, but maybe we should assume they are not?
• Also adds an opt-in rule (configGetUnknownKeyRule) that reports calls to ->get() with a key that doesn't exist in the schema. This catches typos at analysis time rather than runtime.

I've attached a more verbose explanation, but fair warning about the use of the LLM here.

config-get-return-type-extension.md

[mstrelan]

This passes on 11.3 but fails on 10.4, which kind of proves that this is working, since the same config in 10.4 is not FullyValidatable. It falls back to mixed which is what we expect.

[mglaman]

🤖 review incoming:

Nice proof-of-concept — the core instinct (gate everything on FullyValidatable so the schema is authoritative) is the right one, and the conservative mixed/null fallbacks mean the type extension can only add info, not break existing code. Tests pass locally (14/14).

Left inline notes. Two blocking items: a confirmed sequence false-positive in the rule, and the type extension being on-by-default (release/versioning implications). The rest are smells worth cleaning up before merge. On your open question — yes, keep the type nullable: Config::get() returns null for an absent key regardless of schema completeness, so T|null is correct.

[mglaman]

Blocking — confirmed false positive on sequence children. This only walks mapping, so a path segment that lands on a sequence returns false and any child key is reported as unknown. A sequence has arbitrary keys, so this is wrong. Reproduced against system.mail (FullyValidatable, interface is a sequence):

\Drupal::config('system.mail')->get('interface.default');
// => Config key "interface.default" does not exist in the schema for "system.mail".  ← false positive

When traversal hits a sequence, accept the remaining key path. The same gap exists in resolveKeyType below, but there it just yields mixed (safe), so only the rule emits a bad error.

[mglaman]

These are static on a service that's registered as a singleton — no reason for static state here. It leaks across container instances and risks test cross-contamination (one test's schema bleeding into another). Make $definitions and $fullyValidatable instance properties.

[mglaman]

Blocking — on by default. Unlike the rule (opt-in via configGetUnknownKeyRule: false), this type extension is registered unconditionally, so every user gets mixed → bool|null etc. on upgrade. That can surface new errors (e.g. a now-string|null value passed into a non-null param). Per the versioning policy this can't ship in a patch — either gate it behind a parameter too, or release as a minor and call it out in the changelog.

[mglaman]

resolveConfigName() and extractFirstStringArg() are copy-pasted verbatim from ConfigGetDynamicReturnTypeExtension (line 79 there) — ~60 lines, and the comments are already drifting apart. Hoist to one place (a trait, or methods on ConfigSchemaData) so the four config-name patterns stay in sync.

[mglaman]

Non-blocking, worth noting: this parses every *.schema.yml (core + all modules + all themes) on every autoloader boot, even when neither feature is active. On a large site that's real startup latency. Consider lazy-loading the schemas, or only parsing when the extension/rule is enabled.

[mstrelan]

1. Sequence traversal fix (BLOCKING) — ConfigSchemaData.php

keyExistsInDefinition() and resolveKeyType() now handle sequence nodes: when a definition has type: sequence, the next path segment is treated as a dynamic element key (consumed without validation), then recursion continues into the element definition. This fixes the false positive on \Drupal::config('system.mail')->get('interface.default').

New test cases added:

• Rule fixture: interface.default on system.mail no longer reports an error
• Type fixture: assertType('string|null', ...) for interface.default

2. Opt-in gate for type extension (BLOCKING) — extension.neon + ConfigGetDynamicReturnTypeExtension.php

Added drupal.configGetReturnType: false parameter. The extension returns null early when the flag is false. A new test config neon (phpunit-drupal-phpstan-config-get-return-type.neon) enables it for the type extension tests.

3. Static property improvements — ConfigSchemaData.php

Properties kept static (required for PHPStan test infrastructure cross-container survival) but given proper type declarations: private static array $definitions = [] and private static array $fullyValidatable = [], with doc comments explaining why static is necessary.

4. ConfigNameResolverTrait — new src/Drupal/ConfigNameResolverTrait.php

resolveConfigName() and extractFirstStringArg() extracted from both ConfigGetUnknownKeyRule and ConfigGetDynamicReturnTypeExtension into a shared trait. Both classes now use ConfigNameResolverTrait.

5. Lazy-loading TODO — DrupalAutoloader.php

Added // TODO: lazy-load schemas when neither configGetReturnType nor configGetUnknownKeyRule is active. comment at the top of loadConfigSchemas().