-
Notifications
You must be signed in to change notification settings - Fork 0
[rig-eslint] feat(eslint): add agents-must-be-object rule #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| function closingBracket(tokens, openingIndex) { | ||
| let depth = 0; | ||
| for (let index = openingIndex; index < tokens.length; index += 1) { | ||
| if (tokens[index].value === "[") depth += 1; | ||
| if (tokens[index].value === "]") depth -= 1; | ||
| if (depth === 0) return index; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| export function scanTokens(tokens) { | ||
| const problems = []; | ||
|
|
||
| for (let index = 0; index <= tokens.length - 3; index += 1) { | ||
| const [key, colon, openBracket] = tokens.slice(index, index + 3); | ||
| if ( | ||
| key.value !== "agents" | ||
| || colon.value !== ":" | ||
| || openBracket.value !== "[" | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| const closeIndex = closingBracket(tokens, index + 2); | ||
| if (closeIndex === undefined) continue; | ||
|
|
||
| const inner = tokens.slice(index + 3, closeIndex); | ||
|
|
||
| // Collect comma-separated identifier tokens; bail if non-identifiers found. | ||
| const identifiers = []; | ||
| let valid = true; | ||
| for (const token of inner) { | ||
| if (token.value === ",") continue; | ||
| if (/^[A-Za-z_$][\w$]*$/.test(token.value)) { | ||
| identifiers.push(token.value); | ||
| } else { | ||
| valid = false; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (!valid || identifiers.length === 0) continue; | ||
|
|
||
| const fixedText = `{ ${identifiers.join(", ")} }`; | ||
|
|
||
| problems.push({ | ||
| start: openBracket.start, | ||
| end: tokens[closeIndex].end, | ||
| message: "agents must be an object, not an array. Use agents: { name } instead of agents: [name].", | ||
| kind: "agents-must-be-object", | ||
| edits: [{ start: openBracket.start, end: tokens[closeIndex].end, text: fixedText }], | ||
| }); | ||
| } | ||
|
|
||
| return problems; | ||
| } | ||
|
|
||
| export default { | ||
| meta: { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/grill-with-docs] The 💡 CheckRun |
||
| type: "problem", | ||
| docs: { | ||
| description: "Require agents to be declared as a named object, not an array", | ||
| }, | ||
| fixable: "code", | ||
| schema: [], | ||
| messages: { | ||
| mustBeObject: "agents must be an object, not an array. Use agents: { name } instead of agents: [name].", | ||
| }, | ||
| }, | ||
| create(context) { | ||
| return { | ||
| Property(node) { | ||
| if ( | ||
| node.key.type !== "Identifier" | ||
| || node.key.name !== "agents" | ||
| || node.value.type !== "ArrayExpression" | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const elements = node.value.elements; | ||
| if ( | ||
| elements.length === 0 | ||
| || elements.some((el) => el === null || el.type !== "Identifier") | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const names = elements.map((el) => el.name); | ||
| context.report({ | ||
| node: node.value, | ||
| messageId: "mustBeObject", | ||
| fix(fixer) { | ||
| return fixer.replaceText(node.value, `{ ${names.join(", ")} }`); | ||
| }, | ||
| }); | ||
| }, | ||
| }; | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,117 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { fixSource, lintSource } from "../skills/rig/eslint/lint.js"; | ||
| import agentsMustBeObjectRule from "../skills/rig/eslint/rules/agents-must-be-object.js"; | ||
| import rule from "../skills/rig/eslint/rules/no-object-literal-record.js"; | ||
| import repairNoArgsRule from "../skills/rig/eslint/rules/repair-no-args.js"; | ||
|
|
||
| describe("agents-must-be-object", () => { | ||
| it.each([ | ||
| "agents: { extractor }", | ||
| "agents: { diagnose, fix }", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] The 💡 Suggested clarifying testit("does not flag agents array inside a string literal", () => {
const source = `const text = 'agents: [extractor]';`;
expect(lintSource(source).filter((p) => p.kind === "agents-must-be-object")).toEqual([]);
});This makes the string-context protection explicit and independently verifiable. |
||
| "agents: { a, b, c }", | ||
| "const text = 'agents: [extractor]';", | ||
| // Non-identifier elements must not be flagged (no safe autofix) | ||
| ])("accepts %s", (source) => { | ||
| const problems = lintSource(source).filter((p) => p.kind === "agents-must-be-object"); | ||
| expect(problems).toEqual([]); | ||
| }); | ||
|
|
||
| it.each([ | ||
| [ | ||
| "agents: [extractor]", | ||
| "agents: { extractor }", | ||
| ], | ||
| [ | ||
| "agents: [diagnose, fix]", | ||
| "agents: { diagnose, fix }", | ||
| ], | ||
| [ | ||
| "const x = agent({ model: \"small\", agents: [summarizer] });", | ||
| "const x = agent({ model: \"small\", agents: { summarizer } });", | ||
| ], | ||
| ])("fixes %s", (source, expected) => { | ||
| const problems = lintSource(source).filter((p) => p.kind === "agents-must-be-object"); | ||
| expect(problems).toHaveLength(1); | ||
| expect(fixSource(source, problems)).toBe(expected); | ||
| }); | ||
|
|
||
| it("is idempotent", () => { | ||
| const source = "agents: [extractor]"; | ||
| const once = fixSource(source); | ||
| const twice = fixSource(once); | ||
| expect(twice).toBe(once); | ||
| expect(lintSource(once).filter((p) => p.kind === "agents-must-be-object")).toEqual([]); | ||
| }); | ||
|
|
||
| it("does not flag empty array", () => { | ||
| const source = "agents: []"; | ||
| const problems = lintSource(source).filter((p) => p.kind === "agents-must-be-object"); | ||
| expect(problems).toEqual([]); | ||
| }); | ||
|
|
||
| it("keeps the ESLint rule aligned", () => { | ||
| const reports = []; | ||
| const visitor = agentsMustBeObjectRule.create({ | ||
| sourceCode: {}, | ||
| report: (problem) => reports.push(problem), | ||
| }); | ||
|
|
||
| visitor.Property({ | ||
| key: { type: "Identifier", name: "agents" }, | ||
| value: { | ||
| type: "ArrayExpression", | ||
| elements: [ | ||
| { type: "Identifier", name: "extractor" }, | ||
| { type: "Identifier", name: "summarizer" }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| expect(reports).toHaveLength(1); | ||
| expect(reports[0].messageId).toBe("mustBeObject"); | ||
| expect(reports[0].fix({ replaceText: (_node, text) => text })) | ||
| .toBe("{ extractor, summarizer }"); | ||
| }); | ||
|
|
||
| it("does not flag agents object", () => { | ||
| const reports = []; | ||
| const visitor = agentsMustBeObjectRule.create({ | ||
| sourceCode: {}, | ||
| report: (problem) => reports.push(problem), | ||
| }); | ||
|
|
||
| visitor.Property({ | ||
| key: { type: "Identifier", name: "agents" }, | ||
| value: { | ||
| type: "ObjectExpression", | ||
| properties: [], | ||
| }, | ||
| }); | ||
|
|
||
| expect(reports).toHaveLength(0); | ||
| }); | ||
|
|
||
| it("does not flag array with non-identifier elements", () => { | ||
| const reports = []; | ||
| const visitor = agentsMustBeObjectRule.create({ | ||
| sourceCode: {}, | ||
| report: (problem) => reports.push(problem), | ||
| }); | ||
|
|
||
| visitor.Property({ | ||
| key: { type: "Identifier", name: "agents" }, | ||
| value: { | ||
| type: "ArrayExpression", | ||
| elements: [ | ||
| { type: "CallExpression" }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| expect(reports).toHaveLength(0); | ||
| }); | ||
| }); | ||
|
|
||
| describe("no-object-literal-record", () => { | ||
| it.each([ | ||
| "const output = s.record(s.object({ count: s.number }));", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/tdd] The identifier regex
/^[A-Za-z_$][\w$]*$/is redundant — the tokenizer inlint.jsalready guarantees that identifier-shaped tokens are the only word tokens emitted (lines 43–46). Only single-char punctuation tokens can ever reach this branch, and only,passes the skip check. Consider replacing the regex with a comment that names this invariant, making the guard's intent clearer.💡 Clarified guard
This avoids re-testing the regex on every identifier token.