Problem/Motivation
The module code predates modern PHP/Drupal typing conventions:
- Functions and methods lack parameter types and return types.
- Services (`PdfManager`, `PdfEncoder`) type-hint concrete classes
(`Renderer`, `FileSystem`) instead of their interfaces, coupling the module to implementation details.
- The `.info.yml` still declares the removed `core: 8.x` key and advertises `^8 || ^9` support, both of which are end-of-life. ┃
- The unit test uses untyped properties, an incorrect `@var` docblock for the stream wrapper manager mock, and a fragile `assertNotEmpty(strpos(...))` assertion that fails when the needle is at offset 0.
Proposed resolution
- Add parameter and return type declarations to hook implementations in `pdf_serialization.module` and to methods in `PdfManager` and `PdfEncoder` (including `decode()`/`supportsDecoding()`). - Type constructor arguments and properties against interfaces:
`RendererInterface` and `FileSystemInterface` instead of the concrete
`Renderer`/`FileSystem` classes. - Update `pdf_serialization.info.yml` to `core_version_requirement:
^10 || ^11` and remove the obsolete `core: 8.x` key; bump the test submodule info accordingly.
- Modernize `PdfEncoderTest`: native (intersection) property types, `createMock(LoggerInterface::class)`, a corrected `@var` for the stream wrapper manager, and `assertStringContainsString('PDF-1.4', $encoded)` in place of the `strpos()` check.
API changes
`PdfManager::__construct()` and `PdfEncoder::__construct()` now type-hint `RendererInterface`/`FileSystemInterface` rather than the concrete classes.
This is backward compatible: the injected core services already implement those interfaces, so no consumer change is required.
Comments
Comment #6
irobertas commented