Add custom ESLint rule scaffolding for Rig

[Copilot]

Rig lacked project-specific linting for generated programs, leaving common schema mistakes to typechecking or runtime validation.

Changes

• Added the exported rig/eslint plugin and ESLint flat configuration.
• Added an autofixable rule for object literals passed directly to record schema helpers:

// Invalid
s.record({ status: s.string })

// Valid
s.record(s.object({ status: s.string }))

• Applied the rule to s.record and s.nonEmptyObject.
• Added rule coverage and CI integration.
• Documented generated-program linting and custom-rule extension in the Rig skill.

[pelikhan]

@copilot the skill does not have access to npm, the whole eslint infra must be in the skill folder.

[Copilot]

Addressed in f704cc4, with follow-up 52922ee: linting now runs via the dependency-free skills/rig/eslint/lint.js, and the root ESLint config/dependencies and npm-based skill instructions were removed.

[pelikhan]

@copilot agentic workflows to also suggest linting rules for patterns that confuse the model

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[Copilot]

@copilot agentic workflows to also suggest linting rules for patterns that confuse the model

Addressed in 27e4363. The daily Rig task generator now proposes focused lint rules for repeated model-confusing patterns, including invalid/valid examples and autofix feasibility, while excluding one-off and already-covered mistakes.

[github-actions]

Skills-Based Review 🧠

Applied /tdd and /codebase-design — commenting, not requesting changes. The feature is well-structured and the dual-implementation approach is clever, but there are a few correctness gaps worth addressing before the implementation is extended.

📋 Key Themes & Findings

Correctness risks

• Trailing-comment tokenizer bug (lint.js:22): a // comment with no trailing newline causes break, silently dropping subsequent tokens. Easy one-line fix; currently no test covers this.
• Wrapper-paren + comment edge case (lint.js:80): the closing-paren check reads at fixed offsets from closingIndex, which can mis-count when block comments appear between } and ). Needs a targeted test.
• --fix write failures are uncaught (lint.js:147): a permission error during writeFile propagates as an unhandled rejection while exit code stays 0. Low probability in CI but a footgun in dev.

Architecture / maintainability

• Dual implementation drift (lint.js vs. no-object-literal-record.js): the tokenizer linter and the ESLint AST rule implement the same logic independently. The test suite spot-checks one path but doesn't cross-validate both against the same fixture corpus — drift will go undetected when rules are extended.
• Scaffolding docs (linting.md): the adding

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 53.5 AIC · ⌖ 7.46 AIC · ⊞ 6.3K
_{Comment /matt to run again}

[github-actions]

[/codebase-design] lint.js and no-object-literal-record.js are two parallel implementations of the same rule — both must be kept in sync whenever the rule evolves.

The test at eslint-rules.test.js:41 only spot-checks one call path and doesn't cross-validate that both implementations accept/reject the same inputs.

💡 Suggestion: share fixtures across both implementations

Extract valid/invalid case arrays and run them against both lintSource and an ESLint RuleTester to detect drift automatically:

const INVALID = [
  'const x = s.record({ count: s.number });',
  'const x = s.nonEmptyObject({ count: s.number });',
];
it.each(INVALID)('lintSource flags %s', (src) => {
  expect(lintSource(src)).toHaveLength(1);
});
// same list fed to RuleTester

[github-actions]

[/tdd] Line 22: if a file ends with a // comment and no trailing newline, source.indexOf(' ', index + 2) returns -1, the loop breaks, and tokens after the comment are silently skipped.

This means a file ending in // comment (no newline) is tokenised differently from the same file with a trailing newline — a silent correctness bug with no test coverage.

💡 Suggested fix + test

// In tokenize, for single-line comments:
const newline = source.indexOf('
', index + 2);
index = newline === -1 ? source.length : newline;
// (no break — continue the loop)

Test:

it('handles trailing single-line comment without newline', () => {
  expect(lintSource('s.record({ a: s.string }) // trailing')).toHaveLength(1);
});

[github-actions]

[/tdd] --fix silently suppresses errors even when a file fails to write. If writeFile rejects (e.g. permission error), the await rejects, the loop aborts with an unhandled rejection, but failures stays 0 and exit code is 0.

💡 Suggestion

Either let the error propagate naturally (already handled by main's .catch), or collect write errors and set process.exitCode = 1:

try {
  await writeFile(file, fixSource(source, problems));
} catch (err) {
  console.error(`Failed to write ${file}: ${err.message}`);
  failures += 1;
}

Also add a test for the fixSource round-trip so that a broken fix doesn't silently produce malformed TypeScript.

[github-actions]

[/tdd] The parenthesis-skip loop (while tokens[objectIndex]?.value === '(') advances past any number of open parens but the closing-parens check on line 86-89 uses a fixed offset from closingIndex. If the object is nested (({...})) and there is a comment token between the closing brace and the closing parens, wrappersClose will read the wrong tokens and either suppress a real problem or false-positive.

💡 Missing test case

it('handles nested parens with comment between braces', () => {
  const src = 's.record(/* x */({ count: s.number }))';
  expect(lintSource(src)).toHaveLength(1);
});

Adding this test will surface whether the wrapper-check logic is robust.

[github-actions]

[/codebase-design] The scaffolding instructions say to add the equivalent check to lint.js alongside the ESLint rule, but there is no explicit note that both the lintSource path and the ESLint create path need a test. This risks future contributors adding only one of the two when they extend the ruleset.

💡 Suggestion

Strengthen the last bullet:

Add the rule to skills/rig/eslint/rules/, export it from index.js, add the equivalent tokenizer check to lint.js, and cover both code paths in src/eslint-rules.test.js using shared valid/invalid fixtures.

This makes the dual-implementation expectation explicit.