feat: add strong PHP Types, cleanup

[Chris53897]

use LanguageLevel Features
simplify code
fix 3 PHpUnit Asserts (wrong order)

Summary by CodeRabbit

• Refactor

  • Modernized code for PHP 8+: stronger type hints, constructor-promoted public properties, narrowed signatures, and updated string checks.

• Tests

  • Simplified and unified test setups by removing version-specific branches, consolidating cache helpers, converting a provider data method to static, and clarifying assertions.

• Documentation

  • Test bootstrap now fetches Composer over HTTPS.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Warning

Rate limit exceeded

@Chris53897 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 3 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 1188909 and 6bb7b6a.

📒 Files selected for processing (1)

• src/DependencyInjection/BazingaGeocoderExtension.php

Walkthrough

Remove PHP-version branching and traits in tests; add stricter typing and return types in DI/config/provider factories and validator constraint; minor docblock updates; adjust PHPUnit assertion argument order; update test bootstrap Composer URL to HTTPS.

Changes

Cohort / File(s)	Summary
Dependency Injection src/DependencyInjection/BazingaGeocoderExtension.php, src/DependencyInjection/Configuration.php	Add @throws docblock on load(); replace string checks with str_starts_with/str_ends_with; add ArrayNodeDefinition return types for private config helpers; tighten/type-hint private factory provider signature.
Provider Factories src/ProviderFactory/GeoIP2Factory.php, src/ProviderFactory/PluginProviderFactory.php	Import MaxMind\Db\Reader\InvalidDatabaseException and annotate @throws; change createPluginProvider() signature to `callable
Validator Constraint src/Validator/Constraint/Address.php	Convert service and message into constructor-promoted public string properties and refactor constructor to call parent::__construct(null, ...).
Test bootstrap & config loading tests/Functional/BundleInitializationTest.php, tests/Functional/GeocodeEntityListenerTest.php, tests/Functional/PluginInteractionTest.php, tests/Functional/ProviderFactoryTest.php, tests/DependencyInjection/Compiler/FactoryValidatorPassTest.php	Remove version-gated loading of framework_sf<MAJOR_VERSION>.yml (now always loaded); make ProviderFactoryTest::getProviders() static; drop conditional ReflectionProperty::setAccessible usage in tearDown.
Test cache helper consolidation tests/Functional/Helper/CacheHelper.php, tests/Functional/Helper/CacheHelperV7.php, tests/Functional/Helper/CacheHelperV8.php	Delete CacheHelperV7/CacheHelperV8 traits and consolidate into a single CacheHelper class implementing PSR-16 methods with stub/default returns.
Plugin tests — assertions tests/Plugin/FakeIpPluginTest.php	Swap PHPUnit assertion argument order in three assertions to use (expected, actual).
Tests bootstrap security tests/bootstrap.php	Change Composer download URL to HTTPS (https://getcomposer.org/composer.phar).

Sequence Diagram(s)

(omitted — changes are refactors/tests/docblocks without a new multi-component sequential control flow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: allow doctrine/doctrine-bundle 3 #384 — Modifies BazingaGeocoderExtension::load() and DI compatibility checks; overlaps with DI changes.
• Make classes final and add some types #390 — Edits PluginProviderFactory and type/doc improvements around provider factories; closely related to factory signature changes.
• Switch from yaml to php format services definition #373 — Also edits the DependencyInjection extension (load/config handling); related to load-time service logic.

Suggested reviewers

• norkunas

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add strong PHP Types, cleanup' directly reflects the main objectives of the PR: adding type hints/declarations throughout the codebase and simplifying code, making it clear and relevant to the primary changes.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/ProviderFactory/PluginProviderFactory.php (1)

33-39: Remove unreachable else branch.

With the new union type declaration on line 31, PHP guarantees that $factory is either callable or ProviderFactoryInterface before the method executes. The else branch at lines 37-39 is now unreachable dead code.

🔎 Proposed refactor to remove dead code

 if ($factory instanceof ProviderFactoryInterface) {
     $client = $factory->createProvider($config);
-} elseif (is_callable($factory)) {
+} else {
+    // $factory is guaranteed to be callable due to union type
     $client = $factory($config);
-} else {
-    throw new \RuntimeException(sprintf('Second argument to PluginProviderFactory::createPluginProvider must be a "%s" or a callable.', ProviderFactoryInterface::class));
 }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb2322b and 870cc21.

📒 Files selected for processing (15)

• src/DependencyInjection/BazingaGeocoderExtension.php
• src/DependencyInjection/Configuration.php
• src/ProviderFactory/GeoIP2Factory.php
• src/ProviderFactory/PluginProviderFactory.php
• src/Validator/Constraint/Address.php
• tests/DependencyInjection/Compiler/FactoryValidatorPassTest.php
• tests/Functional/BundleInitializationTest.php
• tests/Functional/GeocodeEntityListenerTest.php
• tests/Functional/Helper/CacheHelper.php
• tests/Functional/Helper/CacheHelperV7.php
• tests/Functional/Helper/CacheHelperV8.php
• tests/Functional/PluginInteractionTest.php
• tests/Functional/ProviderFactoryTest.php
• tests/Plugin/FakeIpPluginTest.php
• tests/bootstrap.php

💤 Files with no reviewable changes (3)

• tests/Functional/Helper/CacheHelperV8.php
• tests/DependencyInjection/Compiler/FactoryValidatorPassTest.php
• tests/Functional/Helper/CacheHelperV7.php

🧰 Additional context used

🧬 Code graph analysis (1)

src/Validator/Constraint/Address.php (1)

src/Validator/Constraint/AddressValidator.php (1)

• AddressValidator (26-68)

🪛 PHPMD (2.15.0)

tests/Functional/Helper/CacheHelper.php

24-24: Avoid unused parameters such as '$key'. (undefined)

(UnusedFormalParameter)

---

24-24: Avoid unused parameters such as '$default'. (undefined)

(UnusedFormalParameter)

---

28-28: Avoid unused parameters such as '$key'. (undefined)

(UnusedFormalParameter)

---

28-28: Avoid unused parameters such as '$value'. (undefined)

(UnusedFormalParameter)

---

28-28: Avoid unused parameters such as '$ttl'. (undefined)

(UnusedFormalParameter)

---

33-33: Avoid unused parameters such as '$key'. (undefined)

(UnusedFormalParameter)

---

43-43: Avoid unused parameters such as '$keys'. (undefined)

(UnusedFormalParameter)

---

43-43: Avoid unused parameters such as '$default'. (undefined)

(UnusedFormalParameter)

---

48-48: Avoid unused parameters such as '$values'. (undefined)

(UnusedFormalParameter)

---

48-48: Avoid unused parameters such as '$ttl'. (undefined)

(UnusedFormalParameter)

---

53-53: Avoid unused parameters such as '$keys'. (undefined)

(UnusedFormalParameter)

---

58-58: Avoid unused parameters such as '$key'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (15)

src/ProviderFactory/GeoIP2Factory.php (1)

21-21: LGTM! Good documentation practice.

The addition of the InvalidDatabaseException import and @throws annotation properly documents the exception that can be thrown by the Reader constructor on Line 42.

Also applies to: 32-32

tests/bootstrap.php (1)

15-15: LGTM! Security improvement.

Switching from HTTP to HTTPS for downloading Composer is a good security practice.

src/DependencyInjection/BazingaGeocoderExtension.php (3)

44-45: LGTM! Proper exception documentation.

The @throws \Exception annotation documents that the load method can throw exceptions, improving API clarity for consumers.

---

222-222: LGTM! Improved type safety.

Adding the mixed parameter type and bool return type improves type safety and code clarity.

---

214-214: PHP 8.0+ functions are supported by project's PHP 8.1+ requirement.

The code uses str_ends_with() and str_starts_with(), which require PHP 8.0. The project's composer.json specifies php: ^8.1, which exceeds this requirement, so no compatibility issues exist.

tests/Functional/ProviderFactoryTest.php (2)

77-77: LGTM! Consistent test configuration.

The unconditional loading of framework configuration aligns with the broader pattern across functional tests.

---

93-93: LGTM! Modern PHPUnit convention.

Changing the data provider to static follows PHPUnit best practices and is required in PHPUnit 10+.

tests/Functional/PluginInteractionTest.php (1)

60-60: LGTM! Consistent test configuration.

The unconditional loading of framework configuration standardizes the test setup across all Symfony versions.

Also applies to: 82-82

tests/Functional/BundleInitializationTest.php (1)

55-55: LGTM! Consistent test configuration.

The unconditional loading of framework configuration across all test methods standardizes the test setup and aligns with the broader refactoring pattern.

Also applies to: 70-70, 86-86, 104-104, 130-130, 148-148

tests/Plugin/FakeIpPluginTest.php (1)

33-33: LGTM! Corrected PHPUnit assertion argument order.

The assertions now follow the correct PHPUnit convention of assertSame(expected, actual), which improves test readability and ensures proper failure messages.

Also applies to: 48-48, 59-59

tests/Functional/GeocodeEntityListenerTest.php (1)

109-109: Framework configuration files present for all supported Symfony versions.

All required framework_sf{MAJOR_VERSION}.yml configuration files exist for supported versions: framework_sf6.yml, framework_sf7.yml, and framework_sf8.yml, matching the composer.json constraints (^6.4 || ^7.0 || ^8.0).

src/DependencyInjection/Configuration.php (1)

86-86: LGTM! Excellent type safety improvements.

The explicit return type declarations enhance type safety and align with modern PHP practices. The assertions at lines 90 and 126 confirm these types are correct.

Also applies to: 122-122

src/Validator/Constraint/Address.php (1)

32-32: LGTM! Property types correctly declared.

The explicit string type declarations on these public properties improve type safety and match their actual usage throughout the codebase.

Also applies to: 34-34

tests/Functional/Helper/CacheHelper.php (1)

28-61: Stub implementations look correct.

The remaining PSR-16 method stubs return appropriate default values for testing purposes. The static analysis warnings about unused parameters are false positives—this is expected behavior for stub/mock implementations.

src/ProviderFactory/PluginProviderFactory.php (1)

27-27: Good type safety enhancement.

The explicit callable|ProviderFactoryInterface union type improves type safety and makes the API contract clearer.

Also applies to: 31-31

[norkunas]

also coderabbit nitpick comment is probably applicable?

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/Validator/Constraint/Address.php (1)

34-37: Past review feedback successfully implemented.

The constructor refactoring correctly implements the suggestions from the past review comment:

• ✅ $message moved to promoted property with strong typing
• ✅ $options parameter removed
• ✅ null passed to parent constructor

The implementation aligns well with the PR objectives of using modern PHP language features and simplifying code.

Optional: Consider adding type hint to $payload.

For consistency with the "strong PHP Types" objective, you could add the mixed type hint to the $payload parameter:

public function __construct(public string $message = 'Address {{ address }} is not valid.', ?array $groups = null, mixed $payload = null)

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0229db and c3ec697.

📒 Files selected for processing (2)

• src/ProviderFactory/PluginProviderFactory.php
• src/Validator/Constraint/Address.php

🧰 Additional context used

🧬 Code graph analysis (2)

src/Validator/Constraint/Address.php (1)

src/Validator/Constraint/AddressValidator.php (1)

• AddressValidator (26-68)

src/ProviderFactory/PluginProviderFactory.php (2)

src/ProviderFactory/AbstractFactory.php (1)

• createProvider (43-52)

src/ProviderFactory/ProviderFactoryInterface.php (1)

• createProvider (31-31)

🪛 GitHub Check: PHPStan

src/ProviderFactory/PluginProviderFactory.php

[failure] 32-32:
Method Bazinga\GeocoderBundle\ProviderFactory\PluginProviderFactory::createPluginProvider() has parameter $factory with no value type specified in iterable type array.

🔇 Additional comments (4)

src/Validator/Constraint/Address.php (1)

32-32: LGTM: Strong type added to service property.

The string type hint correctly reflects that this property holds a class name string, improving type safety in line with the PR objectives.

src/ProviderFactory/PluginProviderFactory.php (3)

17-17: LGTM!

The import is necessary for the callable return type annotation in the docblock.

---

32-32: Excellent type safety improvements!

The addition of the return type PluginProvider and the union type callable|ProviderFactoryInterface for the $factory parameter strengthens type safety and makes the API contract more explicit.

---

34-38: LGTM! Clean simplification enabled by strong typing.

The removal of the runtime is_callable() check is appropriate since the union type declaration on line 32 ensures $factory is either ProviderFactoryInterface or callable. This is a good example of leveraging PHP's type system to eliminate unnecessary runtime checks.

[norkunas]

Thank you @Chris53897