diff --git a/src/action/isEntityAssociationIncluded.ts b/src/action/isEntityAssociationIncluded.ts new file mode 100644 index 00000000..f342d987 --- /dev/null +++ b/src/action/isEntityAssociationIncluded.ts @@ -0,0 +1,20 @@ +import { Entity } from "../types/entities"; + +/** + * Determines if an entity should be included based on its author association. + * Uses the author_association field from GitHub's webhook payload. + * @param entity The entity to check + * @param includeAssociations Set of allowed author associations + * @returns true if the entity should be included, false if it should be skipped + */ +export function isEntityAssociationIncluded( + entity: Entity, + includeAssociations: Set, +) { + if ("author_association" in entity.data) { + const association = entity.data.author_association; + return includeAssociations.has(association); + } + + return true; +} diff --git a/src/action/isEntityFromBot.ts b/src/action/isEntityFromBot.ts new file mode 100644 index 00000000..1ace7a06 --- /dev/null +++ b/src/action/isEntityFromBot.ts @@ -0,0 +1,15 @@ +import { Entity } from "../types/entities"; + +/** + * Determines if an entity was created by a bot based on the user.type field. + * @param entity The entity to check + * @returns true if the entity was created by a bot (user.type === "Bot"), false otherwise + */ +export function isEntityFromBot(entity: Entity) { + return ( + "user" in entity.data && + !!entity.data.user && + "type" in entity.data.user && + entity.data.user.type === "Bot" + ); +} diff --git a/src/action/runOctoGuideAction.ts b/src/action/runOctoGuideAction.ts index 7a85ecfa..a61acf5c 100644 --- a/src/action/runOctoGuideAction.ts +++ b/src/action/runOctoGuideAction.ts @@ -3,6 +3,8 @@ import type * as github from "@actions/github"; import * as core from "@actions/core"; import { parseCommentId, parseEntityUrl } from "../actors/parseEntity.js"; +import { parseIncludeAssociations } from "../execution/parseIncludeAssociations.js"; +import { parseRulesConfig } from "../execution/parseRulesConfig.js"; import { runOctoGuideRules } from "../index.js"; import { cliReporter } from "../reporters/cliReporter.js"; import { allRules } from "../rules/all.js"; @@ -75,6 +77,12 @@ export async function runOctoGuideAction(context: typeof github.context) { }, {}); const settings = { + baseOptions: { + includeAssociations: parseIncludeAssociations( + core.getInput("include-associations"), + ), + includeBots: core.getInput("include-bots") === "true", + }, comments: { footer: core.getInput("comment-footer") || @@ -124,66 +132,33 @@ export async function runOctoGuideAction(context: typeof github.context) { entityType, ); - /** - * Determines if an entity was created by a bot based on the user.type field. - * @param entity The entity to check - * @returns true if the entity was created by a bot (user.type === "Bot"), false otherwise - */ - const isEntityFromBot = (entity: Entity): boolean => { - return ( - "user" in entity.data && - !!entity.data.user && - "type" in entity.data.user && - entity.data.user.type === "Bot" - ); - }; - - const includeBots = core.getInput("include-bots") === "true"; - if (!includeBots && isEntityFromBot(entityInput)) { - core.info(`Skipping OctoGuide rules for bot-created ${entityType}: ${url}`); - return; - } - - /** - * Determines if an entity should be included based on its author association. - * Uses the author_association field from GitHub's webhook payload. - * @param entity The entity to check - * @param includeAssociations Set of allowed author associations - * @returns true if the entity should be included, false if it should be skipped - */ - const shouldIncludeEntity = ( - entity: Entity, - includeAssociations: Set, - ): boolean => { - if ("author_association" in entity.data) { - const association = entity.data.author_association; - return includeAssociations.has(association); - } - - return true; - }; - - const includeAssociationsInput = core.getInput("include-associations"); - - const includeAssociations = new Set( - includeAssociationsInput - .split(",") - .map((a) => a.trim()) - .filter((a) => a.length > 0), - ); - - includeAssociations.add("NONE"); - - if (!shouldIncludeEntity(entityInput, includeAssociations)) { - const association = - "author_association" in entityInput.data - ? entityInput.data.author_association - : "UNKNOWN"; - core.info( - `Skipping OctoGuide rules for ${association} created ${entityType}: ${url}`, - ); - return; - } + // TODO: can we still get fast-fail? + // what if the user hasn't provided any options? + + // // TODO: move down to rule-level + + // if (!settings.baseOptions.includeBots && isEntityFromBot(entityInput)) { + // core.info(`Skipping OctoGuide rules for bot-created ${entityType}: ${url}`); + // return; + // } + + // // TODO: move associations down to rule-level + + // if ( + // !isEntityAssociationIncluded( + // entityInput, + // settings.baseOptions.includeAssociations, + // ) + // ) { + // const association = + // "author_association" in entityInput.data + // ? entityInput.data.author_association + // : "UNKNOWN"; + // core.info( + // `Skipping OctoGuide rules for ${association} created ${entityType}: ${url}`, + // ); + // return; + // } const { actor, entity, reports } = await runOctoGuideRules({ auth, @@ -430,15 +405,3 @@ function isIssueLikeData( typeof data.html_url === "string" ); } - -function parseRulesConfig(input: string) { - if (input === "") { - return {}; - } - - try { - return JSON.parse(input) as Record; - } catch (error) { - return error as Error; - } -} diff --git a/src/execution/isRuleSkippedForEntity.ts b/src/execution/isRuleSkippedForEntity.ts new file mode 100644 index 00000000..3047d880 --- /dev/null +++ b/src/execution/isRuleSkippedForEntity.ts @@ -0,0 +1,21 @@ +import { isEntityFromBot } from "../action/isEntityFromBot"; +import { Entity } from "../types/entities"; +import { Rule, RuleOptions } from "../types/rules"; + +export function isRuleSkippedForEntity( + entity: Entity, + options: RuleOptions, + rule: Rule, +) { + const includeAssociations = options["include-associations"]?.split(","); + if (!includeAssociations) { + // return false; + } + + const includeBots = options["include-bots"]; + if (isEntityFromBot(entity) && !includeBots) { + return true; + } + + return true; +} diff --git a/src/execution/mergeRuleOptions.ts b/src/execution/mergeRuleOptions.ts new file mode 100644 index 00000000..a7fc4510 --- /dev/null +++ b/src/execution/mergeRuleOptions.ts @@ -0,0 +1,23 @@ +import { RuleOptions, RuleOptionsRaw } from "../types/rules"; +import { BaseOptions } from "../types/settings"; + +export function mergeRuleOptions( + baseOptions: BaseOptions, + overrides: boolean | RuleOptionsRaw | undefined, +): RuleOptions { + if (!overrides || typeof overrides === "boolean") { + return { + includeAssociations: baseOptions.includeAssociations, + includeBots: baseOptions.includeBots, + }; + } + + return { + ...baseOptions, + ...overrides, + "include-associations": overrides["include-associations"] + ? new Set(overrides["include-associations"]) + : baseOptions.includeAssociations, + "include-bots": overrides["include-bots"] ?? baseOptions.includeBots, + }; +} diff --git a/src/execution/parseIncludeAssociations.ts b/src/execution/parseIncludeAssociations.ts new file mode 100644 index 00000000..4c65a7f5 --- /dev/null +++ b/src/execution/parseIncludeAssociations.ts @@ -0,0 +1,20 @@ +const defaultIncludeAssociations = new Set([ + "CONTRIBUTOR", + "FIRST_TIME_CONTRIBUTOR", + "FIRST_TIMER", + "NONE", +]); + +export function parseIncludeAssociations(raw: string) { + if (!raw) { + return defaultIncludeAssociations; + } + + return new Set([ + "NONE", + ...raw + .split(",") + .map((a) => a.trim()) + .filter((a) => a.length > 0), + ]); +} diff --git a/src/execution/parseRulesConfig.ts b/src/execution/parseRulesConfig.ts new file mode 100644 index 00000000..b541359e --- /dev/null +++ b/src/execution/parseRulesConfig.ts @@ -0,0 +1,27 @@ +import { RuleOptions, RuleOptionsRaw } from "../types/rules"; +import { parseIncludeAssociations } from "./parseIncludeAssociations"; + +export function parseRulesConfig(input: string) { + if (input === "") { + return {}; + } + + try { + return processRulesConfig(JSON.parse(input) as RuleOptionsRaw); + } catch (error) { + return error as Error; + } +} + +function processRulesConfig(raw: RuleOptionsRaw): RuleOptions { + return { + ...raw, + // TODO: this is wrong + // these shouldn't be at root + // these are for each rule + "include-associations": raw["include-associations"] + ? parseIncludeAssociations(raw["include-associations"]) + : undefined, + "include-bots": !raw["include-bots"], + }; +} diff --git a/src/runOctoGuideRules.ts b/src/runOctoGuideRules.ts index bb453058..558bff3d 100644 --- a/src/runOctoGuideRules.ts +++ b/src/runOctoGuideRules.ts @@ -8,6 +8,8 @@ import type { RuleContext } from "./types/rules.js"; import type { Settings } from "./types/settings.js"; import { createActor } from "./actors/createActor.js"; +import { isRuleSkippedForEntity } from "./execution/isRuleSkippedForEntity.js"; +import { mergeRuleOptions } from "./execution/mergeRuleOptions.js"; import { runRuleOnEntity } from "./execution/runRuleOnEntity.js"; import { allRules } from "./rules/all.js"; import { configs } from "./rules/configs.js"; @@ -31,7 +33,7 @@ export interface RunOctoGuideRulesOptions { /** * Settings for the run, including rules to enable. */ - settings?: Settings; + settings: Settings; } /** @@ -102,11 +104,11 @@ export async function runOctoGuideRules({ const reports: RuleReport[] = []; - const config = settings?.config ?? "recommended"; + const config = settings.config ?? "recommended"; const configRuleNames = Object.values(configs[config]).map( (rule) => rule.about.name, ); - const ruleOverrides = settings?.rules ?? {}; + const ruleOverrides = settings.rules ?? {}; const enabledRules = allRules.filter((rule) => { const ruleName = rule.about.name; @@ -120,9 +122,16 @@ export async function runOctoGuideRules({ await Promise.all( enabledRules.map(async (rule) => { + // TODO: merge with parent options + const options = mergeRuleOptions( + settings.baseOptions, + ruleOverrides[rule.about.name], + ); + const context: RuleContext = { locator, octokit, + options: typeof options === "object" ? options : undefined, report(data) { reports.push({ about: rule.about, @@ -131,7 +140,9 @@ export async function runOctoGuideRules({ }, }; - await runRuleOnEntity(context, rule, entity); + if (!isRuleSkippedForEntity(entity, options, rule)) { + await runRuleOnEntity(context, rule, entity); + } }), ); diff --git a/src/types/rules.ts b/src/types/rules.ts index 636028cd..d25709ac 100644 --- a/src/types/rules.ts +++ b/src/types/rules.ts @@ -89,6 +89,29 @@ export interface RuleContext { * Registers a new violation. */ report: RuleReporter; + + /** + * Processed options for any rule that may be provided by the user. + */ + options?: RuleOptions; +} + +/** + * Options for any rule that as provided by the user. + */ +export interface RuleOptionsRaw { + [i: string]: unknown; + "include-associations"?: string; + "include-bots"?: boolean; +} + +/** + * Processed options for any rule that may be provided by the user. + */ +export interface RuleOptions { + [i: string]: unknown; + "include-associations"?: Set; + "include-bots"?: boolean; } /** diff --git a/src/types/settings.ts b/src/types/settings.ts index c111e70a..b15949d3 100644 --- a/src/types/settings.ts +++ b/src/types/settings.ts @@ -1,12 +1,19 @@ import type { ConfigName } from "./core.js"; +import type { RuleOptions, RuleOptionsRaw } from "./rules.js"; -export interface Settings { - comments?: Comments; - config?: ConfigName; - rules?: Record; +export interface BaseOptions { + includeAssociations?: Set; + includeBots?: boolean; } -interface Comments { +export interface Comments { footer: string; header: string; } + +export interface Settings { + baseOptions: BaseOptions; + comments?: Comments; + config?: ConfigName; + rules?: Record; +}