Skip to content

[detector-report] ArrayReturnBagDetector #423

Description

@jessegall

Detector: ArrayReturnBagDetector

Report (why the flagged code is CORRECT and the detector is wrong):
The return is a homogeneous lookup dictionary, not a record: 131 entries of retired-permission-string => Permission, every value the same type, consumed only by keyed lookup in remap(). ArrayReturnBagDetector targets a record returned as an array — heterogeneous named fields like ['currency' => ..., 'gross' => ..., 'net' => ...] — where each key is a distinct field of one concept. A dictionary has no fields to name; array<string, Permission> is already a total type.

Cleanest design the reporter can conceive:
A private static method returning array<string, Permission>, the table reachable only through remap() and retiredGrants(). Turning the entries into a value object list would make lookup O(n) and replace 131 declarative rows with 131 constructor calls, and a keyed collection class would be array<string, Permission> wearing a wrapper.

⚖️ Maintainer litmus: a valid detector-report needs the flagged code to ALREADY BE the
cleanest design. If the design above differs from the flagged code at all, THAT design is
the owed fix — close this report; the fix is still owed.

Where: app/Auth/Permissions/PermissionGrantMigrationMap.php:30

Code (app/Auth/Permissions/PermissionGrantMigrationMap.php:30):

  27       */
  28      private static function table(): array
  29      {
→ 30          return [
  31              // ----------[ Shop floor ]----------
  32  
  33              'dashboard.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  34              'web-register.access' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  35              'web-register.process-payments' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  36              'quick-pads.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  37              'customers.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  38              'customers.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  39              'customers.create' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  40              'customers.update' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  41              'orders.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  42              'orders.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  43              'orders.update-status' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  44              'products.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  45              'products.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  46              'stockpiles.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  47              'stockpiles.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  48              'warehouses.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  49              'warehouses.view' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  50              'reviews.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  51              'shops.index' => Permission::SHOPFLOOR_SERVES_CUSTOMERS,
  52  
  53              // Amending a paid order and mailing its invoice are the shift lead's, not the assistant's.
  54              'orders.create' => Permission::SHOPFLOOR_RUNS_THE_SHIFT,

Filed via commandments report from a consumer project.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions