Fix Angular Fixmes Agent Skill

Key rules

• Fix the underlying issue — never just delete the suppression comment and leave broken code.
• Remove the complete comment block: FIXME line (if any) + TODO: Skipped block (if any) + eslint-disable-next-line line.
• Both FIXME-paired and standalone suppressions are the same migration debt.
• For Angular migration rules, prefer CLI schematics over manual edits.
• Do NOT convert service observables to signals (ADR-0027).
• For OnPush and signals patterns, the angular-modernization skill is the authoritative source — this skill only owns the ESLint suppression mechanics.

Step 1: Discover all suppressions

Use the Grep tool to find suppressions in the target path:

• Pattern eslint-disable — finds all eslint suppressions
• Pattern FIXME.*CL- — finds Angular FIXME-tracked ones specifically

Group results by rule name. Two forms appear in this codebase:

Form A — FIXME-paired (a FIXME tracking comment sits above the disable):

// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection

Form B — Standalone (disable without a FIXME, or with a CLI skip comment):

// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
// TODO: Skipped for signal migration because:
//  Accessor inputs cannot be migrated as they are too complex.
// FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
// eslint-disable-next-line @angular-eslint/prefer-signals

Both forms must be fixed the same way.

Rule reference

| Category | Rule | Section below |

| ---------------- | ----------------------------------------------------------- | --------------------------------------------------- |

| Angular | @angular-eslint/prefer-on-push-component-change-detection | OnPush |

| Angular | @angular-eslint/prefer-signals | Signals |

| Angular | @angular-eslint/prefer-output-emitter-ref | Signals |

| Angular template | @angular-eslint/template/button-has-type | HTML rules |

| TypeScript | @typescript-eslint/no-floating-promises | no-floating-promises |

| TypeScript | @typescript-eslint/no-unused-vars | no-unused-vars |

| TypeScript | @typescript-eslint/no-unsafe-function-type | no-unsafe-function-type |

| RxJS | rxjs/no-async-subscribe | rxjs rules |

| RxJS | rxjs-angular/prefer-takeuntil | rxjs rules |

| Bitwarden | @bitwarden/platform/no-enums | no-enums |

| Bitwarden | @bitwarden/components/no-bwi-class-usage | HTML rules |

| General | no-restricted-imports | no-restricted-imports |

| General | no-console | no-console |

| General | no-empty | no-empty |

| General | bare // eslint-disable-next-line | bare disable |

| Tailwind | tailwindcss/no-custom-classname | HTML rules |

OnPush

Rule: @angular-eslint/prefer-on-push-component-change-detection

Follow the OnPush guidance in the angular-modernization skill (add changeDetection: ChangeDetectionStrategy.OnPush, remove ChangeDetectorRef if only used for detectChanges()). Then remove the FIXME + eslint-disable-next-line lines.

> @Directive does not support changeDetection — skip OnPush for pure directives.

Signals

Rules: @angular-eslint/prefer-signals, @angular-eslint/prefer-output-emitter-ref

Applies to @Input(), @Output(), @ViewChild, @ContentChild.

Follow the Signal Inputs, Outputs, and Queries guidance in the angular-modernization skill (prefer CLI schematics, then manual conversion). After each migration, manually remove the FIXME and eslint-disable-next-line lines, and any // TODO: Skipped for signal migration because: comment blocks.

> Do NOT convert service observables to signals — only component-local state and decorator bindings (ADR-0027).

no-floating-promises

Rule: @typescript-eslint/no-floating-promises

A returned Promise is not handled. Pick one fix:

// 1. Await it (preferred in async functions)
await this.router.navigate(["/login"]);

// 2. void — explicit fire-and-forget
void this.router.navigate(["/login"]);

// 3. Chain .catch() for explicit error handling
this.router.navigate(["/login"]).catch((err) => this.logService.error(err));

Use void for navigation or toast calls that genuinely don't need awaiting. Use await when the result matters or you're already in an async context.

no-unused-vars

Rule: @typescript-eslint/no-unused-vars

// Remove unused variable
const unused = computeSomething(); // delete this line

// Or prefix with _ if it must be declared (e.g. destructuring)
const [_first, second] = array;

// Or suppress a catch variable (TypeScript 4.0+)
try { ... } catch { ... } // omit the variable entirely

no-unsafe-function-type

Rule: @typescript-eslint/no-unsafe-function-type

Replace the generic Function type with a specific signature:

Before

private callback: Function;

After — use the actual signature

private callback: () => void;
// or for unknown signatures:
private callback: (...args: unknown[]) => unknown;

RxJS rules

Rules: rxjs/no-async-subscribe, rxjs-angular/prefer-takeuntil

rxjs/no-async-subscribe — async callback inside .subscribe() swallows errors:

Before

this.service.data$.subscribe(async (value) => {
  await this.process(value);
});

After — move async work into the pipe

this.service.data$
  .pipe(
    switchMap((value) => this.process(value)),
    takeUntilDestroyed(),
  )
  .subscribe();

rxjs-angular/prefer-takeuntil — subscription without cleanup:

Before

this.service.data$.subscribe((value) => {
  this.data = value;
});

After — add takeUntilDestroyed() (call in constructor or use destroyRef)

constructor() {
  this.service.data$
    .pipe(takeUntilDestroyed())
    .subscribe((value) => { this.data = value; });
}

no-enums

Rule: @bitwarden/platform/no-enums

Convert TypeScript enums to const objects with type aliases (ADR-0025):

Before

enum CipherType {
  Login = 1,
  SecureNote = 2,
}

After

export const CipherType = Object.freeze({ Login: 1, SecureNote: 2 } as const);
export type CipherType = (typeof CipherType)[keyof typeof CipherType];

Update all import sites — the usage (CipherType.Login) stays the same.

no-restricted-imports

Rule: no-restricted-imports

The import is from a path that the ESLint config forbids. Steps:

• Read the context around the import to understand what is being imported.
• Check eslint.config.mjs at the repo root (or the nearest config) for the no-restricted-imports rule to find the allowed alternative path.
• Replace the import with the allowed path and remove the suppression.

Common cases: importing platform-internal modules directly instead of through the public API, or test-only helpers in non-test files.

no-console

Rule: no-console

// Remove debug statements
console.log("debug"); // delete

// Replace with the application logging service
this.logService.error("Something failed", error);

In test files (*.spec.ts), a console.error or console.warn spy may be intentional — in that case, set up the spy properly rather than suppressing:

jest.spyOn(console, "error").mockImplementation(() => {});

no-empty

Rule: no-empty

Empty catch blocks silently swallow errors:

Before

try {
  await something();
  // eslint-disable-next-line no-empty
} catch {}

After — handle or log the error

try {
  await something();
} catch (e) {
  // Intentionally ignored — operation is best-effort
}

// Or log it
try {
  await something();
} catch (e) {
  this.logService.warning("Operation failed", e);
}

Bare disable

Rule: bare // eslint-disable-next-line (no rule specified)

This disables ALL rules for the next line, which is always wrong. Steps:

• Remove the suppression and run npm run lint:fix to see which specific rule triggers.
• Fix the underlying issue using the appropriate section above.
• If the violation truly cannot be fixed (rare), replace the bare disable with a specific named rule.

HTML rules

@angular-eslint/template/button-has-type — Add an explicit type to every <button>:

Before

<button (click)="save()">Save</button>

After

<button type="button" (click)="save()">Save</button>
<!-- or type="submit" inside a <form> -->

@bitwarden/components/no-bwi-class-usage — Replace raw bwi-* icon classes with the <bit-icon> component or the appropriate icon token.

tailwindcss/no-custom-classname — Use a Tailwind utility class with the tw- prefix, or register the class in the Tailwind safelist. Never use arbitrary custom class names.

Step 2: Cleanup checklist per fixed instance

• [ ] // FIXME(…) line removed (if present)
• [ ] // TODO: Skipped for signal migration because: … block removed (all lines, if present)
• [ ] // eslint-disable-next-line … line removed
• [ ] Unused imports removed; new imports added as needed
• [ ] All in-class usages updated (e.g. signal reads need ())

Step 3: Validate

npm run lint:fix

Fix any errors that remain. Run npm run test if behaviour-critical code was changed.