Lint rules: conventions, contracts and Deps

// bad: orders.proto
service OrderService { rpc GetOrder(GetOrderRequest) returns (GetOrderResponse); }
service RefundService { rpc GetRefund(GetRefundRequest) returns (GetRefundResponse); }

// bad
message ListOrdersRequest {
  string customer = 1;
}

// fix: a filter is optional, so unset means "no filter" end to end
message ListOrdersRequest {
  optional string customer = 1;
}

// or, for a parameter the handler insists on, say so
message ListLinesRequest {
  string order_id = 1 [(buf.validate.field).required = true];
}

✗ proto/services/order/v1/order.proto:25 [forgeconv-list-filter-optional] ListOrdersRequest.customer has no field presence: unset and "" are the same value on the wire, so the proto does not say whether it is a filter or a required parameter (as a filter its zero value can never be filtered on)

// bad: any config field whose annotation sets env_var: "OTEL_SERVICE_NAME"
string service_name = 22 [(forge.v1.config) = { env_var: "OTEL_SERVICE_NAME" }];

// fix: delete the field and reserve its tag and name
reserved 22;
reserved "service_name";

// bad
rpc CancelOrder(CancelOrderRequest) returns (CancelOrderResponse);

// fix: declare the event
rpc CancelOrder(CancelOrderRequest) returns (CancelOrderResponse) {
  option (forge.v1.method) = {
    activity: { verb: "cancel", target: { kind: "order", id: "order_id" } }
  };
}

// or declare, with a reason, that it records none
option (forge.v1.method) = { activity: { none: "internal reconciliation, not user-visible" } };

// bad
res, err := s.deps.Lifecycle.ShipOrder(ctx, in)
if err != nil {
    return nil, err
}

// fix
if err != nil {
    return nil, svcerr.Wrap(err)
}

// bad
func toConnectError(err error) error {
    switch {
    case errors.Is(err, ErrNotFound):
        return connect.NewError(connect.CodeNotFound, err)
    }
    return connect.NewError(connect.CodeInternal, err)
}

// fix: delete it, and call svcerr.Wrap(err) at the return

// bad
return nil, connect.NewError(connect.CodeInternal, err)

// fix
return nil, svcerr.Wrap(err)
return nil, svcerr.NotFound("order")            // "order not found"
return nil, svcerr.InvalidArgument("id is required")

// bad
type Deps struct {
    Repo *db.PostgresRepository
}

// fix: declare the narrow interface where it is consumed
type orderStore interface {
    GetOrder(ctx context.Context, id string) (*Order, error)
}
type Deps struct {
    Repo orderStore
}

// bad: the package calls only Get, through a 14-method interface from another package
type Deps struct{ Store otherpkg.Store }

// fix
forge fix narrow-deps internal/handlers/order
// declares the 1-method interface the package uses, here, and retypes the field.
// compose.go needs no change.

// bad
type Deps struct {
    App *app.Components   // the composition root itself
    Any any
}
// fix: take each collaborator as its own typed field, and configuration as the component's own config block.

// bad
real := s.deps.Store.(*postgresStore)

// fix: add the methods you need to the interface, or take a second field

// bad
addr := os.Getenv("ORDER_QUEUE_ADDR")

// fix: declare the setting in proto/<project>/config/v1 and take it through Deps

// bad
s.deps.Notifier.Send(ctx, msg)

// fix
if s.deps.Notifier != nil {
    s.deps.Notifier.Send(ctx, msg)
}
// or, for a site that is provably safe by reasoning the analyzer cannot follow:
s.deps.Notifier.Send(ctx, msg) // forge:optional-checked

// bad
// forge:optional-dep
type Deps struct { Notifier notifier }

// fix
type Deps struct {
    // forge:optional-dep
    Notifier notifier
}

// bad
type OrderService interface { Get(ctx context.Context, id string) (*Order, error) }
type Dependencies struct{ /* … */ }

// fix
type Service interface { Get(ctx context.Context, id string) (*Order, error) }
type Deps struct{ /* … */ }
func New(deps Deps) Service { /* … */ }

// bad: useCancelOrder is a useMutation hook now, but the starter row still reads it as a query
const { result } = renderHook(() => useCancelOrder(), { wrapper });
await waitFor(() => expect(result.current.isSuccess).toBe(true));

// fix (the replacement forge prints for a mutation row): fire it, then wait for it to settle
const { result } = renderHook(() => useCancelOrder(), { wrapper });
await result.current.mutateAsync({} as never).catch(() => {});
await waitFor(() => expect(["success", "error"]).toContain(result.current.status));

// bad
s.deps.DB.RunTx(ctx, orm.Serializable, func(ctx context.Context) error {
    row, err := q.For("UPDATE").Where("id = ?", id).Scan(ctx) // lock under SERIALIZABLE
    // …
})

// fix: if the lock is what makes it correct, say so
s.deps.DB.RunTx(ctx, orm.ReadCommitted, func(ctx context.Context) error { /* same body */ })
// or drop the lock and rely on SERIALIZABLE plus a retry

// bad: $1 in a bun-formatted query is sent literally while bun drops the argument
db.Bun().NewRaw("SELECT * FROM orders WHERE id = $1", id)
// fails at run time: "there is no parameter $1"

// fix
db.Bun().NewRaw("SELECT * FROM orders WHERE id = ?", id)

// bad: the ? in the jsonb operator is read as a placeholder
db.Bun().NewRaw("SELECT * FROM orders WHERE tags ? ? ", tag)

// fix: escape it
db.Bun().NewRaw(`SELECT * FROM orders WHERE tags \? ?`, tag)
// or use jsonb_exists(tags, ?)

// bad
const issuer = process.env.NEXT_PUBLIC_OIDC_ISSUER;

// fix: declare it in the frontend_config message, run forge generate, then
import { loadConfig } from "@/lib/config_gen";
const issuer = loadConfig().NEXT_PUBLIC_OIDC_ISSUER;

// bad: a Zustand store that fetches
import { orderClient } from "@/gen/order_connect";
export const useOrders = create<State>((set) => ({ orders: [], load: async () => set({ orders: (await orderClient.listOrders({})).orders }) }));

// fix: use the generated hook
const { data } = useListOrders({});