Skip to content

Commit de830c4

Browse files
rpgmemclaude
andauthored
refactor(api,generators,admin): honest row shapes, and a toggle that did nothing (#1060) (#1074)
* refactor(api,generators,admin): honest row shapes, and a toggle that did nothing (#1060) PR 4. The ruler goes from 170 to 135 and all three modules reach zero. Most of it is typing the producer rather than casting at the consumer. AudienceQueryService::find_user_bookings() now declares publicly what it returns instead of list<array<string, mixed>>, which is what left the REST controller reading thirteen unverifiable keys. QRCodeGenerator's defaults and parsed parameters are a declared shape all the way to the library call, and its settings reads go through SettingsReader — the project's own rule for ffc_settings, and also what makes them typed. Declaring that shape turned up a toggle that never worked. The two writers of qr_cache_enabled disagree on its type: the tab's form save stores int 1, while the autosave endpoint — which is what flipping the switch actually calls — stores a PHP boolean through RequestInput::is_truthy(). The check was `1 === $value`, and `1 === true` is false, so turning the QR cache on from the UI left it off. Reading through the existing typed accessor fixes it; the test is verified failing against the old check. SettingsReader gains get_string(), the typed accessor that was missing. It refuses a non-scalar rather than casting — `(string) array()` is the literal 'Array' plus a notice. Its int and bool siblings keep their plain casts on purpose: changing those changes what every existing caller gets back, which is a wider decision than this PR. Two smaller notes. The intersection `UserBookingRow&array{audiences: …}` resolves to *NEVER* in PHPStan — two sealed array shapes cannot be intersected — so the with-audiences shape repeats the keys; the duplication is the cost of saying what the method returns. And a test fixture that omitted `audiences` was fixed rather than the shape widened: the producer always sets that key, so the fixture was describing a row production never emits. get_post_meta() casts are handled by two private helpers in FormListColumns rather than a shared class. The repository has 87 casts of that shape, but one proven user is not duplication yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XWx9qJdjZdAq8crxM9GCU * style(qrcode): align the assignments phpcbf flagged (#1060) The WPCS gate reported two Generic.Formatting.MultipleStatementAlignment warnings on the error-level block. My local phpcs run had been made before that block's last edit, so it passed on a tree that no longer existed — re-running the CI's exact command (phpcs over the changed-file list) reproduces it, and phpcbf fixes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XWx9qJdjZdAq8crxM9GCU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 96e9f47 commit de830c4

12 files changed

Lines changed: 240 additions & 43 deletions

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ jobs:
5555
vendor/bin/phpstan analyse -c phpstan-rows.neon.dist \
5656
--error-format=json --no-progress --memory-limit=2G > rows.json || true
5757
- name: Report the row-reading classes
58-
run: php .github/scripts/phpstan-rows-report.php rows.json 170
58+
run: php .github/scripts/phpstan-rows-report.php rows.json 135
5959

6060
audit:
6161
name: Composer audit

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,16 @@ The format follows [Keep a Changelog] (https://keepachangelog.com/en/1.1.0/).
2828
- **Formas de linha honestas em `frontend` e `core`** (#1060): `ActivityLogQuery` declara a linha do log derivada do `CREATE TABLE`, e `ReprintDetector` declara o formato do resultado que dois consumidores já liam às cegas. A régua de nível 9 baixou de 203 para 170 e os dois módulos ficaram em zero.
2929
- **`Core\ArrayValue`** (#1060): leitura de escalar sobre array sem tipo — JSON decodificado, configuração de formulário, payload de token. Substitui o idioma `(string) ( $data['k'] ?? '' )`, que não confere nada: dado um array ele produz a string `Array`, dado um objeto sem `__toString` é fatal. Não serve para `$_POST` (isso é o `RequestInput`) nem para linha de banco (isso é shape declarada).
3030

31+
- **Formas de linha honestas em `api`, `generators` e `admin`** (#1060): o `QRCodeGenerator` passa a ler `ffc_settings` pelo `SettingsReader` — regra do próprio projeto — e seus parâmetros viraram uma shape tipada até a chamada da biblioteca; `find_user_bookings()` declara publicamente o que devolve, em vez de `array<string, mixed>`. A régua de nível 9 baixou de 170 para 135 e os três módulos ficaram em zero.
32+
- **`SettingsReader::get_string()`** (#1060): acessor tipado que faltava. Recusa valor não escalar em vez de convertê-lo — `(string) array()` é a string `Array` mais um notice. Os acessores de int e bool seguem com o cast simples: mudá-los mudaria o que todo chamador existente recebe.
33+
3134
### Removed
3235

3336
- `Shortcodes::get_new_captcha_data()` (#1053): método público sem nenhum chamador em produção — o único consumidor era o próprio teste. A geração de desafio já é responsabilidade do contrato de captcha.
3437

3538
### Fixed
39+
- **O cache de QR Code nunca ligava pelo toggle** (#1060): os dois gravadores da chave `qr_cache_enabled` discordam do tipo — o save do formulário grava `int 1`, e o autosave, que é o que o interruptor chama de fato, grava um booleano. A checagem era `1 === $valor`, e `1 === true` é falso, então ligar o cache pela interface deixava-o desligado.
40+
3641
- **`ReprintDetector::detect()` devolvia `date` com tipo diferente conforme o ramo** (#1060): inteiro (segundos unix) quando havia reimpressão, string vazia quando não. Só um consumidor lê a chave, e só no ramo de reimpressão, então o ramo vazio passou a devolver `0` — um contrato cujo tipo depende do ramo não pode ser verificado.
3742

3843
- **Ids não-numéricos em reservas de público viravam `0`** (#1060): `audience_ids` e `user_ids` chegam dentro de um `$data` do chamador, e cada entrada era convertida direto com `(int)` — uma string solta ou `null` virava `0` e gravava uma linha de junção apontando para um público que não existe. Agora entradas não-numéricas são descartadas.

‎includes/admin/class-ffc-admin-user-columns.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
namespace FreeFormCertificate\Admin;
2020

21+
use FreeFormCertificate\Core\ArrayValue;
22+
2123
if ( ! defined( 'ABSPATH' ) ) {
2224
exit;
2325
}
@@ -237,9 +239,11 @@ private static function render_user_actions( int $user_id ): string {
237239
// Get dashboard URL from User Access Settings (cached per request).
238240
if ( null === self::$dashboard_url_cache ) {
239241
$user_access_settings = get_option( 'ffc_user_access_settings', array() );
240-
self::$dashboard_url_cache = isset( $user_access_settings['redirect_url'] ) && ! empty( $user_access_settings['redirect_url'] )
241-
? $user_access_settings['redirect_url']
242-
: home_url( '/dashboard' );
242+
self::$dashboard_url_cache = ArrayValue::string(
243+
is_array( $user_access_settings ) ? $user_access_settings : array(),
244+
'redirect_url',
245+
home_url( '/dashboard' )
246+
);
243247
}
244248
$dashboard_url = self::$dashboard_url_cache;
245249

‎includes/admin/class-ffc-csv-exporter.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
namespace FreeFormCertificate\Admin;
2020

21+
use FreeFormCertificate\Core\ArrayValue;
2122
use FreeFormCertificate\Core\BatchedCsvExport;
2223
use FreeFormCertificate\Core\SourceRegistry;
2324
use FreeFormCertificate\Repositories\SubmissionRepository;
@@ -109,10 +110,11 @@ public static function cleanup_stale_export_jobs(): int {
109110

110111
// Read the payload BEFORE deleting so we can unlink the temp
111112
// file the abandoned job left on disk.
112-
$job = get_option( '_transient_' . $transient_key );
113-
if ( is_array( $job ) && ! empty( $job['file'] ) && file_exists( $job['file'] ) ) {
113+
$job = get_option( '_transient_' . $transient_key );
114+
$file = is_array( $job ) ? ArrayValue::string( $job, 'file' ) : '';
115+
if ( '' !== $file && file_exists( $file ) ) {
114116
// phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink -- Deletes the plugin's own temp export file by absolute path. WP_Filesystem would need credentials, and this runs on a cleanup path with no user present.
115-
unlink( $job['file'] );
117+
unlink( $file );
116118
}
117119

118120
delete_transient( $transient_key );

‎includes/admin/class-ffc-form-list-columns.php‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,38 @@ public static function add_columns( array $columns ): array {
123123
return $new;
124124
}
125125

126+
/**
127+
* Read a post meta value as a string.
128+
*
129+
* `get_post_meta()` returns mixed — a meta row can hold a serialised
130+
* array — and `(string)` on one is the literal `'Array'` plus a notice.
131+
* Local to this class on purpose: the repository has 87 casts of this
132+
* shape and a shared helper is worth extracting from two proven users,
133+
* not from one (#1060).
134+
*
135+
* @param int $post_id Post ID.
136+
* @param string $key Meta key.
137+
* @return string
138+
*/
139+
private static function meta_string( int $post_id, string $key ): string {
140+
$value = get_post_meta( $post_id, $key, true );
141+
142+
return is_scalar( $value ) ? (string) $value : '';
143+
}
144+
145+
/**
146+
* Read a post meta value as an integer.
147+
*
148+
* @param int $post_id Post ID.
149+
* @param string $key Meta key.
150+
* @return int
151+
*/
152+
private static function meta_int( int $post_id, string $key ): int {
153+
$value = get_post_meta( $post_id, $key, true );
154+
155+
return is_numeric( $value ) ? (int) $value : 0;
156+
}
157+
126158
/**
127159
* Resolve the on/off state of each toggleable feature for a form.
128160
*
@@ -132,7 +164,7 @@ public static function add_columns( array $columns ): array {
132164
private static function get_feature_states( int $post_id ): array {
133165
$config = get_post_meta( $post_id, '_ffc_form_config', true );
134166
$device_meta = get_post_meta( $post_id, '_ffc_device_limit', true );
135-
$csv_enabled = (string) get_post_meta( $post_id, '_ffc_csv_public_enabled', true );
167+
$csv_enabled = self::meta_string( $post_id, '_ffc_csv_public_enabled' );
136168

137169
return array(
138170
'csv_public_enabled' => '1' === $csv_enabled,
@@ -227,12 +259,12 @@ public static function render_column( string $column_name, int $post_id ): void
227259
break;
228260

229261
case 'ffc_csv_downloads':
230-
$csv_enabled = (string) get_post_meta( $post_id, '_ffc_csv_public_enabled', true );
262+
$csv_enabled = self::meta_string( $post_id, '_ffc_csv_public_enabled' );
231263
if ( '1' !== $csv_enabled ) {
232264
echo '<span class="ffc-empty-value">&mdash;</span>';
233265
} else {
234-
$dl_count = (int) get_post_meta( $post_id, '_ffc_csv_public_count', true );
235-
$limit = (int) get_post_meta( $post_id, '_ffc_csv_public_limit', true );
266+
$dl_count = self::meta_int( $post_id, '_ffc_csv_public_count' );
267+
$limit = self::meta_int( $post_id, '_ffc_csv_public_limit' );
236268
if ( $limit > 0 ) {
237269
printf(
238270
'%s / %s',
@@ -318,11 +350,12 @@ public static function search_by_id( \WP_Query $query ): void {
318350
}
319351

320352
$search = $query->get( 's' );
321-
if ( '' === $search || ! ctype_digit( trim( $search ) ) ) {
353+
$search = is_scalar( $search ) ? trim( (string) $search ) : '';
354+
if ( '' === $search || ! ctype_digit( $search ) ) {
322355
return;
323356
}
324357

325-
$query->set( 'post__in', array( absint( trim( $search ) ) ) );
358+
$query->set( 'post__in', array( absint( $search ) ) );
326359
$query->set( 's', '' );
327360
}
328361
}

‎includes/api/class-ffc-user-audience-rest-controller.php‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
namespace FreeFormCertificate\API;
1818

19+
use FreeFormCertificate\Core\ArrayValue;
20+
1921
if ( ! defined( 'ABSPATH' ) ) {
2022
exit;
2123
}
@@ -170,18 +172,18 @@ public function get_user_audience_bookings( $request ) {
170172

171173
$bookings_formatted[] = array(
172174
'id' => (int) $booking['id'],
173-
'environment_id' => (int) ( $booking['environment_id'] ?? 0 ),
175+
'environment_id' => (int) $booking['environment_id'],
174176
'environment_name' => $booking['environment_name'] ?? __( 'Unknown', 'ffcertificate' ),
175177
'schedule_name' => $booking['schedule_name'] ?? '',
176178
'booking_date' => $date_formatted,
177-
'booking_date_raw' => $booking['booking_date'] ?? '',
179+
'booking_date_raw' => $booking['booking_date'],
178180
'start_time' => $time_formatted,
179181
'end_time' => $end_time_formatted,
180-
'description' => $booking['description'] ?? '',
182+
'description' => $booking['description'],
181183
'status' => $status,
182184
'status_label' => $status_labels[ $status ] ?? $status,
183185
'is_past' => $is_past,
184-
'audiences' => $booking['audiences'] ?? array(),
186+
'audiences' => $booking['audiences'],
185187
);
186188
}
187189

@@ -304,6 +306,23 @@ public function get_joinable_groups( $request ) {
304306
}
305307
}
306308

309+
/**
310+
* Read an integer route parameter.
311+
*
312+
* `WP_REST_Request::get_param()` returns mixed — the route's registered
313+
* type is a runtime validation, not something the analyser can see — and
314+
* `absint()` on an array is a TypeError rather than a rejected request.
315+
*
316+
* @param \WP_REST_Request<array<string, mixed>> $request Request.
317+
* @param string $key Parameter name.
318+
* @return int
319+
*/
320+
private static function param_int( $request, string $key ): int {
321+
$value = $request->get_param( $key );
322+
323+
return is_numeric( $value ) ? absint( $value ) : 0;
324+
}
325+
307326
/**
308327
* Recursively assemble a joinable-tree node from a raw audience row.
309328
*
@@ -319,13 +338,23 @@ public function get_joinable_groups( $request ) {
319338
* reference tallies the user's memberships in button-bearing nodes —
320339
* the on-screen "joined N of max" counter.
321340
*
322-
* @param array<string, mixed> $node Audience row with 'children' array.
323-
* @param int $count Reference counter for joined self-join nodes.
324-
* @return array<string, mixed>|null Assembled node, or null when it does not appear.
341+
* The parameter is `array<array-key, mixed>` rather than
342+
* `array<string, mixed>` because the recursive call feeds it an element of
343+
* the node's own `children`, whose key type nothing guarantees. Every read
344+
* below goes through `ArrayValue`, so a node missing a key yields a
345+
* default instead of a warning.
346+
*
347+
* @param array<array-key, mixed> $node Audience row with 'children' array.
348+
* @param int $count Reference counter for joined self-join nodes.
349+
* @return array<string, mixed>|null Assembled node, or null when it does not appear.
325350
*/
326351
private function build_joinable_node( array $node, int &$count ): ?array {
327352
$children = array();
328-
foreach ( $node['children'] as $child ) {
353+
foreach ( ArrayValue::array( $node, 'children' ) as $child ) {
354+
if ( ! is_array( $child ) ) {
355+
continue;
356+
}
357+
329358
$built = $this->build_joinable_node( $child, $count );
330359
if ( $built ) {
331360
$children[] = $built;
@@ -373,7 +402,7 @@ public function join_audience_group( $request ) {
373402
try {
374403
$ctx = $this->resolve_user_context( $request );
375404
$user_id = $ctx['user_id'];
376-
$group_id = absint( $request->get_param( 'group_id' ) );
405+
$group_id = self::param_int( $request, 'group_id' );
377406

378407
if ( ! $user_id ) {
379408
return new \WP_Error( 'not_logged_in', __( 'You must be logged in', 'ffcertificate' ), array( 'status' => 401 ) );
@@ -448,7 +477,7 @@ public function leave_audience_group( $request ) {
448477
global $wpdb;
449478
$ctx = $this->resolve_user_context( $request );
450479
$user_id = $ctx['user_id'];
451-
$group_id = absint( $request->get_param( 'group_id' ) );
480+
$group_id = self::param_int( $request, 'group_id' );
452481

453482
if ( ! $user_id ) {
454483
return new \WP_Error( 'not_logged_in', __( 'You must be logged in', 'ffcertificate' ), array( 'status' => 401 ) );

‎includes/audience/class-ffc-audience-query-service.php‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,31 @@
7878
* schedule_name: string|null,
7979
* is_all_day?: numeric-string|null
8080
* }
81+
* `UserBookingWithAudiences` repeats every key of `UserBookingRow` instead of
82+
* intersecting with it: PHPStan resolves an intersection of two sealed array
83+
* shapes to *NEVER*, so `UserBookingRow&array{audiences: …}` type-checks as a
84+
* value that cannot exist. The duplication is the cost of saying what the
85+
* method actually returns.
86+
*
87+
* @phpstan-type UserBookingWithAudiences array{
88+
* id: numeric-string,
89+
* environment_id: numeric-string,
90+
* booking_date: string,
91+
* start_time: string,
92+
* end_time: string,
93+
* booking_type: string,
94+
* description: string,
95+
* status: string|null,
96+
* created_by: numeric-string,
97+
* created_at: string|null,
98+
* cancelled_by: numeric-string|null,
99+
* cancelled_at: string|null,
100+
* cancellation_reason: string|null,
101+
* environment_name: string|null,
102+
* schedule_name: string|null,
103+
* is_all_day?: numeric-string|null,
104+
* audiences: list<array{name: string, color: string|null}>
105+
* }
81106
* @phpstan-type BookingAudienceBadgeRow array{
82107
* booking_id: numeric-string,
83108
* name: string,
@@ -213,7 +238,7 @@ public static function find_user_joinable_audiences( int $user_id ): array {
213238
* @since 6.6.2
214239
* @param int $user_id WordPress user ID.
215240
* @param array<string, mixed> $filter Optional filter (see shape above).
216-
* @return list<array<string, mixed>>
241+
* @return list<UserBookingWithAudiences>
217242
*/
218243
public static function find_user_bookings( int $user_id, array $filter = array() ): array {
219244
if ( $user_id <= 0 ) {

0 commit comments

Comments
 (0)