# Code Quality Conventions — packages/core

## Naming Conventions

### Concerns (model/resource traits)
Use `Has*` prefix for all traits. No deprecated aliases — update references directly.

**Model concerns** (live in `Foundation/Concerns/`, used by Eloquent models):

| Trait | Purpose |
|---|---|
| `HasBuilderAccess` | Auth: `canAccessBuilder()`, `builderPermissions()`, widget toggle |
| `HasRevisionHistory` | Eloquent: `revisions()` morphMany + `latestRevision()` |
| `HasPublishing` | Status lifecycle for non-translatable models |
| `HasTranslatablePublishing` | Status lifecycle for translatable models (status on translation row) |
| `HasTranslations` | Translation relationships + `setTranslation()` / `getTranslation()` |
| `HasHierarchy` | Parent/child tree relationships |
| `HasGlobalSearch` | Global search integration |

**Resource concerns** (live in `Foundation/Concerns/`, used by Resource classes):

| Trait | Purpose |
|---|---|
| `HasRevisions` | Resource marker: `isRevisable(): bool` — enables revision capture |

### Contracts (interfaces)
Use `-able` / `-er` / `Definition` suffix — never bare nouns:

| Interface | Suffix | Purpose |
|---|---|---|
| `BuilderAccessible` | `-able` | Can log into Builder (`canAccessBuilder`, `builderPermissions`) |
| `Translatable` | `-able` | Has translation table + `getTranslation()` |
| `Publishable` | `-able` | Has status lifecycle |
| `GloballySearchable` | `-able` | Returns global search results |
| `BlockFieldHandler` | `Handler` | Handles block field persistence |
| `MediaFieldHandler` | `Handler` | Handles media field persistence |
| `AccountProfileDefinition` | `Definition` | Defines account profile fields, rules, hooks |

No deprecated aliases — update all references directly when renaming.

### Models
Use simple descriptive names — no `Builder` prefix:

| Old | New |
|---|---|
| `BuilderRole` | `Role` |

---

## Single-Action Controller Pattern

Resource HTTP actions live in `src/Http/Controllers/Resource/` as single-action controllers:

```
ResourceIndexController      → GET  /{resource}
ResourceCreateController     → GET  /{resource}/create
ResourceStoreController      → POST /{resource}
ResourceEditController       → GET  /{resource}/{id}/edit
ResourceUpdateController     → PUT  /{resource}/{id}
ResourceDestroyController    → DELETE /{resource}/{id}
ResourceExportController     → GET  /{resource}/export
ResourceBulkController       → POST /{resource}/bulk
ResourceQuickMetaController  → GET  /{resource}/quick-create-meta
ResourceQuickStoreController → POST /{resource}/quick-store
ResourceOptionsController    → GET  /{resource}/options
ResourceRecordActionController → POST /{resource}/{id}/action/{action}
ResourceDuplicateController  → POST /{resource}/{id}/duplicate
```

Each controller:
- Has `__invoke()` as its only public method
- Uses only the Concerns it needs (see below)
- Has `private readonly Registry $registry` in the constructor (required by `HasResourceResolving`)

**Routes** (`routes/web.php`): bind directly to the class — `ResourceIndexController::class` (no method tuple).

---

## Controller Concerns

Shared logic lives in `src/Http/Controllers/Concerns/`. All method names are `protected`.

### `HasResourceResolving`
Accesses `$this->registry` (requires using class to have `private/protected Registry $registry`).

Methods: `resolveOrFail()`, `authorizeResource()`, `resolveContentLocale()`, `resolveAvailableLanguages()`

**Use in:** every resource controller.

### `HasFieldTypes`
Methods: `getTranslatableFieldNames()`, `getVirtualFieldNames()`, `getMediaFields()`, `getBlockFields()`, `stripRestrictedFields()`

**Use in:** controllers that read or write field data (store, update, edit, bulk, export, quick-store).

### `HasIndexQuery`
Calls `$this->resolveContentLocale()` and `$this->getTranslatableFieldNames()` — requires `HasResourceResolving` + `HasFieldTypes`.

Methods: `buildIndexQuery()`

**Use in:** index, export, bulk controllers.

### `HasColumnFormats`
Methods: `applyColumnFormats()`

**Use in:** index controller (alongside `HasIndexQuery`).

### `HasSlugGeneration`
Methods: `applyGenerateFrom()` (protected entry point), `makeUniqueSlug()`, `slugExists()`, `prependParentSlug()` (private helpers)

**Use in:** store, update, quick-store controllers.

---

## Test Setup Patterns

### PageResourceTest (root app — `HasTranslatablePublishing`)
Status lives on `page_translations`, NOT on `pages`. Always:

```php
// WRONG:
$page = Page::create(['status' => 'draft']);

// CORRECT:
$page = Page::create([]);
$page->setTranslation('en', ['name' => 'Test', 'slug' => 'test', 'status' => 'draft']);
```

### PSR-4 Test Users
Test user classes (e.g. `SkeletonTestUser`) must be in their own file — never defined inline at the bottom of `TestCase.php`. The namespace must match the file path so Composer can autoload them.

### Inertia Config Keys
- Root app (full Laravel): `inertia.pages.paths`
- Package tests (Orchestra Testbench): `inertia.testing.page_paths`

---

## Exception Conventions

Builder uses typed exceptions for domain-level errors. Never use `bool|string` union return types as a stand-in for "success or error message".

### `DeletionDeniedException`

Throw from `canDelete()` to prevent deletion. Use the static factory:

```php
use Builder\Core\Exceptions\DeletionDeniedException;

public function canDelete(mixed $record): void
{
    if ($record->isRoot()) {
        throw DeletionDeniedException::withReason('Root records cannot be deleted.');
    }
}
```

The exception message is shown to the user as an error toast. Controllers catch `DeletionDeniedException` at the HTTP boundary — resources should never catch it themselves.

### General Rule

When a Builder operation fails for a domain reason, throw a typed exception from `Builder\Core\Exceptions\`. All exceptions must extend `RuntimeException`. Callers catch at the HTTP boundary only.

```php
// WRONG — mixed return type:
public function canDelete(mixed $record): bool|string

// CORRECT — exception-based:
public function canDelete(mixed $record): void
```

---

## Architecture Tests

The `tests/ArchTest.php` in `packages/core` enforces structural conventions with Pest `arch()`:

- No `dd`/`dump`/`ray` in source
- All source classes use `declare(strict_types=1)`
- Foundation enums are real PHP enums (`Builder\Core\Foundation\Enums`)
- Controllers in `Resource/` are `final` and have `__invoke()`
- Exceptions in `Builder\Core\Exceptions` extend `RuntimeException`
- Schema/resource primitives do not depend on `Illuminate\Http`
- Concerns only have `protected`/`private` methods

When adding a new controller, enum, exception, or concern — verify these arch tests still pass. Add new rules to `ArchTest.php` when you introduce a new structural convention.

**Note:** Other packages (skeleton, media, menu, block-editor) cannot run `toUseStrictTypes()` arch tests because their vendors lack `staudenmeir/laravel-adjacency-list` (a core dependency). They only enforce the debug-function check.

---

## Test Quality Standards

**Error cases are mandatory.** Happy-path-only tests are insufficient. For every controller action or service method, write tests for:

- Validation failure (422 + error structure)
- Authorization denial (403)
- Domain-level denial (e.g. `DeletionDeniedException` → redirect + error toast, record not deleted)
- Not found (404)

Use `describe()` to group related scenarios:

```php
describe('canDelete denial', function (): void {
    beforeEach(function (): void {
        app(Registry::class)->resource(MyDeniedResource::class);
    });

    it('redirects with error toast when deletion denied', function (): void { ... });
    it('does not delete the record when denied', function (): void { ... });
});
```

---

## Type Coverage

All closures passed to `array_filter`, `array_map`, `fn()` must have typed parameters. PHPStan type-coverage is enforced at 100% — untyped lambdas break the build:

```php
// WRONG:
array_filter($fields, fn ($f) => $f->isTranslatable())

// CORRECT:
array_filter($fields, fn (Field $f): bool => $f->isTranslatable())
```

---

## PHPStan Level 8 Patterns

All packages run PHPStan at **level 8**. A `phpstan-baseline.php` records known Eloquent inference limitations (no larastan). When fixing code:

- Do NOT grow the baseline — fix the code instead
- After fixes, regenerate: `vendor/bin/phpstan analyse --generate-baseline phpstan-baseline.php`
- Then run `vendor/bin/pint --dirty phpstan-baseline.php` (Pint checks it too)

### Translation functions

`__()` returns `array|string|null`. Always cast to `string` before passing to typed methods:

```php
// WRONG — argument.type error:
->label(__('builder::builder.nav.users'))

// CORRECT:
->label((string) __('builder::builder.nav.users'))
```

Applies to: `->label()`, `->hint()`, `->confirm()`, `->options([...values...])`, `badge(['x' => ['label' => ...]])`, `messages()` return values, `getLabel(): string`.

### Eloquent collection → typed array

PHPStan without larastan can't infer Eloquent collection item types. Use `array_values(array_map(..., $collection->all()))` instead of `$collection->map(...)->values()->all()`:

```php
// PHPStan-friendly:
array_values(array_map(
    fn (Language $l): array => ['code' => $l->code, 'name' => $l->name],
    Language::allLocales()->all(),
))
```

### Model @property annotations

Eloquent models need explicit `@property` annotations for PHPStan to know about their columns. Without them, `$model->code` etc. raise `property.notFound`. Add annotations for all DB columns and relations:

```php
/**
 * @property int $id
 * @property string $code
 * @property bool $is_active
 * @property \Illuminate\Support\Carbon $created_at
 * @property-read \Illuminate\Database\Eloquent\Collection<int, Translation> $translations
 */
```

### Arrow function scope capture

Arrow functions (`fn()`) automatically capture outer scope. Never use `use`:

```php
// WRONG — syntax error:
fn (string $code) use ($map): array => [...]

// CORRECT:
fn (string $code): array => [...]  // $map captured automatically
```

### Return type covariance

Subclasses may narrow return types covariantly. If a parent declares `?string` but the override always returns `string`, change to `string`:

```php
// Parent (Resource.php):
public function getPermission(): ?string { return null; }

// Subclass — valid PHP covariance:
public function getPermission(): string { return 'read:users'; }
```
