Skip to content

fix: use FetchAs::Associative enum for fetchAll() in RlAnalyzer#39

Merged
jjroelofs merged 1 commit into
1.xfrom
fix/drupal-check-fetchall-fetchas-enum
Apr 14, 2026
Merged

fix: use FetchAs::Associative enum for fetchAll() in RlAnalyzer#39
jjroelofs merged 1 commit into
1.xfrom
fix/drupal-check-fetchall-fetchas-enum

Conversation

@jjroelofs
Copy link
Copy Markdown
Contributor

Summary

Fix the two pre-existing drupal-check (PHPStan) failures on 1.x by swapping the legacy \PDO::FETCH_ASSOC int constant for the new Drupal\Core\Database\Statement\FetchAs::Associative enum case.

What changed

Drupal 11 moved the fetch-mode argument of Connection::fetchAll() from the int-based PDO::FETCH_* constants to a typed FetchAs enum. Passing \PDO::FETCH_ASSOC still works at runtime (PDO defines it as an int), but PHPStan via mglaman/phpstan-drupal flags it as an argument.type error:

```
Parameter #1 $mode of method
Drupal\Core\Database\StatementInterface::fetchAll() expects
Drupal\Core\Database\Statement\FetchAs|null, int given.
```

Two call sites in `src/Service/RlAnalyzer.php` (lines 407 and 428) were the only remaining drupal-check failures on `1.x`. Both swapped to `FetchAs::Associative`, plus a matching `use Drupal\Core\Database\Statement\FetchAs;` import.

Why now

These two errors have been present on `1.x` since the repo adopted Drupal 11 compatibility. They were surfaced again during the PR #35 review as the only red CI check that was never blocking a merge but was always noise in the status rollup. Worth fixing in a tiny dedicated PR so every subsequent PR starts from a green baseline.

Verification

  • `docker compose --profile lint run --rm drupal-check` → `[OK] No errors` (was: `Found 2 errors`)
  • `docker compose --profile lint run --rm drupal-lint` → 0 errors, 0 warnings
  • `docker compose --profile test run --rm e2e-test` → 88/88 assertions pass across all 6 test files
  • PHP syntax check clean

Test plan

  • CI runs all three checks (`drupal-lint`, `drupal-check`, `drush-e2e`) and all three pass
  • No runtime regression in analytics endpoints that call `getArmsData()` or `getArmSnapshots()` (already covered by existing e2e tests hitting `rl:analytics:*` commands)

Scope guardrails

  • Only touches two lines + one import in `RlAnalyzer.php`
  • No schema changes, no API changes, no behaviour changes — `FetchAs::Associative` is the exact enum-equivalent of `\PDO::FETCH_ASSOC`
  • Existing call sites in `getAllArmsData()` / `getTotalTurns()` / etc. in `ExperimentDataStorage` are unaffected (they already don't pass a fetch-mode argument, relying on Drupal's default `FetchAs::Object`)

Drupal 11 changed the Connection statement fetch-mode argument from the
legacy int-based PDO::FETCH_* constants to the
Drupal\Core\Database\Statement\FetchAs enum. Passing \PDO::FETCH_ASSOC
still works at runtime (PDO defines it as an int), but PHPStan via
mglaman/phpstan-drupal flags it as an argument.type error because the
type declaration on Connection::fetchAll() is now FetchAs|null:

  Parameter #1 $mode of method
  Drupal\Core\Database\StatementInterface::fetchAll() expects
  Drupal\Core\Database\Statement\FetchAs|null, int given.

Two call sites in src/Service/RlAnalyzer.php (lines 407 and 428) were
the only remaining drupal-check failures on 1.x. Swap both for
FetchAs::Associative and add the corresponding use statement.

Verified:
- drupal-check (PHPStan) is clean: [OK] No errors (was: Found 2 errors)
- drupal-lint (PHPCS) is clean
- e2e: 88/88 assertions pass across all 6 test files
@jjroelofs jjroelofs merged commit a9a0149 into 1.x Apr 14, 2026
3 checks passed
@jjroelofs jjroelofs deleted the fix/drupal-check-fetchall-fetchas-enum branch April 14, 2026 14:13
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