diff --git a/.github/workflows/agentic-auto-upgrade.yml b/.github/workflows/agentic-auto-upgrade.yml index e169cea32b6..7035101ee35 100644 --- a/.github/workflows/agentic-auto-upgrade.yml +++ b/.github/workflows/agentic-auto-upgrade.yml @@ -34,7 +34,7 @@ name: Agentic Auto-Upgrade on: schedule: - - cron: "11 4 * * 6" # Weekly (auto-upgrade) + - cron: "21 3 * * 5" # Weekly (auto-upgrade) workflow_dispatch: permissions: diff --git a/Makefile b/Makefile index d2da47f282c..77023c2f193 100644 --- a/Makefile +++ b/Makefile @@ -260,13 +260,14 @@ test-impacted-js: build-js fi; \ CHANGED_SETUP_JS_FILES=$$(printf '%s\n' "$$CHANGED_JS_FILES" | grep '^actions/setup/js/' || true); \ CHANGED_ESLINT_FACTORY_FILES=$$(printf '%s\n' "$$CHANGED_JS_FILES" | grep '^eslint-factory/' || true); \ + ROOT=$$(pwd); \ if [ -n "$$CHANGED_SETUP_JS_FILES" ]; then \ echo "Running impacted JavaScript unit tests in actions/setup/js for changed files: $$CHANGED_SETUP_JS_FILES"; \ - cd actions/setup/js && printf '%s\n' "$$CHANGED_SETUP_JS_FILES" | sed 's|^actions/setup/js/||' | tr '\n' '\0' | xargs -0 -r npm run test:js -- --no-file-parallelism --passWithNoTests $(JS_IMPACTED_TEST_EXCLUDES); \ + cd "$$ROOT/actions/setup/js" && printf '%s\n' "$$CHANGED_SETUP_JS_FILES" | sed 's|^actions/setup/js/||' | tr '\n' '\0' | xargs -0 -r npm run test:js -- --no-file-parallelism --passWithNoTests $(JS_IMPACTED_TEST_EXCLUDES); \ fi; \ if [ -n "$$CHANGED_ESLINT_FACTORY_FILES" ]; then \ echo "Running eslint-factory tests for changed files: $$CHANGED_ESLINT_FACTORY_FILES"; \ - cd eslint-factory && npm test; \ + cd "$$ROOT/eslint-factory" && npm test; \ fi # Test impacted Go unit tests only (excluding integration tests) diff --git a/actions/setup/js/generate_usage_activity_summary.cjs b/actions/setup/js/generate_usage_activity_summary.cjs index b10a2a0e0b6..7298bd84392 100644 --- a/actions/setup/js/generate_usage_activity_summary.cjs +++ b/actions/setup/js/generate_usage_activity_summary.cjs @@ -12,6 +12,8 @@ const fs = require("fs"); const { globSync } = require("node:fs"); const path = require("path"); +require("./shim.cjs"); + const SQUID_STATUS_INDEX = 6; const SQUID_DECISION_INDEX = 7; const SQUID_DOMAIN_INDEX = 2; @@ -468,7 +470,7 @@ function main() { } catch (err) { throw new Error(`Failed to write file ${outputPath}: ${String(err)}`, { cause: err }); } - console.log(outputPath); + core.info(outputPath); } // Run main function diff --git a/actions/setup/js/generate_workflow_overview.cjs b/actions/setup/js/generate_workflow_overview.cjs index be0346d20a1..62886973d09 100644 --- a/actions/setup/js/generate_workflow_overview.cjs +++ b/actions/setup/js/generate_workflow_overview.cjs @@ -35,7 +35,7 @@ async function generateWorkflowOverview(core) { const summary = "
\n" + `${summaryLabel}\n\n` + details + "\n" + "
"; await core.summary.addRaw(summary).write(); - console.log("Generated workflow overview in step summary"); + core.info("Generated workflow overview in step summary"); } module.exports = { diff --git a/actions/setup/js/generate_workflow_overview.test.cjs b/actions/setup/js/generate_workflow_overview.test.cjs index c87e666af38..01d8cf30e07 100644 --- a/actions/setup/js/generate_workflow_overview.test.cjs +++ b/actions/setup/js/generate_workflow_overview.test.cjs @@ -135,13 +135,8 @@ describe("generate_workflow_overview.cjs", () => { }; fs.writeFileSync(awInfoPath, JSON.stringify(awInfo)); - // Capture console.log - const consoleSpy = vi.spyOn(console, "log"); - await generateWorkflowOverview(mockCore); - expect(consoleSpy).toHaveBeenCalledWith("Generated workflow overview in step summary"); - - consoleSpy.mockRestore(); + expect(mockCore.info).toHaveBeenCalledWith("Generated workflow overview in step summary"); }); }); diff --git a/actions/setup/js/merge_remote_agent_github_folder.cjs b/actions/setup/js/merge_remote_agent_github_folder.cjs index c28b3b66bb0..c316a32d907 100644 --- a/actions/setup/js/merge_remote_agent_github_folder.cjs +++ b/actions/setup/js/merge_remote_agent_github_folder.cjs @@ -9,7 +9,8 @@ * and merges it into the current repository, failing on conflicts. * * This script runs in a github-script context where core object - * is available globally. Do NOT use npm packages from the actions org. + * is available globally. When run as a plain Node.js process, shim.cjs + * provides global.core. Do NOT use npm packages from the actions org. * * Environment Variables: * - GH_AW_REPOSITORY_IMPORTS: JSON array of repository imports (e.g., '["owner/repo@ref"]') @@ -22,23 +23,11 @@ const fs = require("fs"); const path = require("path"); const { execFileSync } = require("child_process"); +require("./shim.cjs"); + const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_CONFIG, ERR_PARSE, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); -// Get the core object - in github-script context it's global, for testing we create a minimal version -const coreObj = - typeof core !== "undefined" - ? core - : { - info: msg => console.log(msg), - warning: msg => console.warn(msg), - error: msg => console.error(msg), - setFailed: msg => { - console.error(msg); - process.exitCode = 1; - }, - }; - /** * Parse the agent import specification to extract repository details * Format: owner/repo/path@ref or owner/repo@ref or owner/repo/path @@ -50,7 +39,7 @@ function parseAgentImportSpec(importSpec) { return null; } - coreObj.info(`Parsing import spec: ${importSpec}`); + core.info(`Parsing import spec: ${importSpec}`); // Remove section reference if present (file.md#Section) let cleanSpec = importSpec; @@ -66,7 +55,7 @@ function parseAgentImportSpec(importSpec) { // Parse path: owner/repo or owner/repo/path/to/file.md const slashParts = pathPart.split("/"); if (slashParts.length < 2) { - coreObj.warning(`Invalid import spec format: ${importSpec}`); + core.warning(`Invalid import spec format: ${importSpec}`); return null; } @@ -75,11 +64,11 @@ function parseAgentImportSpec(importSpec) { // Check if this is a local import (starts with . or doesn't have owner/repo format) if (owner.startsWith(".") || owner.includes("github/workflows")) { - coreObj.info("Import is local, skipping remote .github folder merge"); + core.info("Import is local, skipping remote .github folder merge"); return null; } - coreObj.info(`Parsed: owner=${owner}, repo=${repo}, ref=${ref}`); + core.info(`Parsed: owner=${owner}, repo=${repo}, ref=${ref}`); return { owner, repo, ref }; } @@ -177,7 +166,7 @@ function validateSafePath(userPath, basePath, name) { * @param {string} tempDir - Temporary directory for checkout */ function sparseCheckoutGithubFolder(owner, repo, ref, tempDir) { - coreObj.info(`Performing sparse checkout of .github folder from ${owner}/${repo}@${ref}`); + core.info(`Performing sparse checkout of .github folder from ${owner}/${repo}@${ref}`); // Validate inputs to prevent command injection validateGitParameter(owner, "owner"); @@ -190,29 +179,29 @@ function sparseCheckoutGithubFolder(owner, repo, ref, tempDir) { try { // Initialize git repository execFileSync("git", ["init"], { cwd: tempDir, stdio: "pipe" }); - coreObj.info("Initialized temporary git repository"); + core.info("Initialized temporary git repository"); // Configure sparse checkout execFileSync("git", ["config", "core.sparseCheckout", "true"], { cwd: tempDir, stdio: "pipe" }); - coreObj.info("Enabled sparse checkout"); + core.info("Enabled sparse checkout"); // Set sparse checkout pattern to only include .github folder const sparseCheckoutFile = path.join(tempDir, ".git", "info", "sparse-checkout"); fs.writeFileSync(sparseCheckoutFile, ".github/\n"); - coreObj.info("Configured sparse checkout pattern: .github/"); + core.info("Configured sparse checkout pattern: .github/"); // Add remote - using execFileSync prevents shell injection execFileSync("git", ["remote", "add", "origin", repoUrl], { cwd: tempDir, stdio: "pipe" }); - coreObj.info(`Added remote: ${repoUrl}`); + core.info(`Added remote: ${repoUrl}`); // Fetch and checkout - using execFileSync with validated ref - coreObj.info(`Fetching ref: ${ref}`); + core.info(`Fetching ref: ${ref}`); execFileSync("git", ["fetch", "--depth", "1", "origin", ref], { cwd: tempDir, stdio: "pipe" }); - coreObj.info("Checking out .github folder"); + core.info("Checking out .github folder"); execFileSync("git", ["checkout", "FETCH_HEAD"], { cwd: tempDir, stdio: "pipe" }); - coreObj.info("Sparse checkout completed successfully"); + core.info("Sparse checkout completed successfully"); } catch (error) { throw new Error(`${ERR_PARSE}: Sparse checkout failed: ${getErrorMessage(error)}`, { cause: error }); } @@ -226,7 +215,7 @@ function sparseCheckoutGithubFolder(owner, repo, ref, tempDir) { * @returns {{merged: number, conflicts: string[]}} */ function mergeGithubFolder(sourcePath, destPath) { - coreObj.info(`Merging .github folder from ${sourcePath} to ${destPath}`); + core.info(`Merging .github folder from ${sourcePath} to ${destPath}`); const conflicts = []; let mergedCount = 0; @@ -236,7 +225,7 @@ function mergeGithubFolder(sourcePath, destPath) { // Get all files from source .github folder const sourceFiles = getAllFiles(sourcePath); - coreObj.info(`Found ${sourceFiles.length} files in source .github folder`); + core.info(`Found ${sourceFiles.length} files in source .github folder`); for (const relativePath of sourceFiles) { // Validate relative path to prevent path traversal @@ -247,7 +236,7 @@ function mergeGithubFolder(sourcePath, destPath) { const topLevelFolder = pathParts[0]; if (!allowedSubfolders.includes(topLevelFolder)) { - coreObj.info(`Skipping file outside allowed subfolders: ${relativePath}`); + core.info(`Skipping file outside allowed subfolders: ${relativePath}`); continue; } @@ -268,9 +257,9 @@ function mergeGithubFolder(sourcePath, destPath) { if (!sourceContent.equals(destContent)) { conflicts.push(relativePath); - coreObj.error(`Conflict detected: ${relativePath}`); + core.error(`Conflict detected: ${relativePath}`); } else { - coreObj.info(`File already exists with same content: ${relativePath}`); + core.info(`File already exists with same content: ${relativePath}`); } } else { // Copy file to destination @@ -281,12 +270,12 @@ function mergeGithubFolder(sourcePath, destPath) { } catch (err) { throw new Error(`Failed to create directory ${destDir}: ${String(err)}`, { cause: err }); } - coreObj.info(`Created directory: ${path.relative(destPath, destDir)}`); + core.info(`Created directory: ${path.relative(destPath, destDir)}`); } fs.copyFileSync(sourceFile, destFile); mergedCount++; - coreObj.info(`Merged file: ${relativePath}`); + core.info(`Merged file: ${relativePath}`); } } @@ -301,7 +290,7 @@ function mergeGithubFolder(sourcePath, destPath) { * @param {string} workspace - Workspace path */ async function mergeRepositoryGithubFolder(owner, repo, ref, workspace) { - coreObj.info(`Merging .github folder from ${owner}/${repo}@${ref} into workspace`); + core.info(`Merging .github folder from ${owner}/${repo}@${ref} into workspace`); // Calculate the pre-checked-out folder path // This matches the format generated by the compiler: .github/aw/imports/-- @@ -310,7 +299,7 @@ async function mergeRepositoryGithubFolder(owner, repo, ref, workspace) { const relativePath = `.github/aw/imports/${owner}-${repo}-${sanitizedRef}`; const checkoutPath = path.join(workspace, relativePath); - coreObj.info(`Looking for pre-checked-out repository at: ${checkoutPath}`); + core.info(`Looking for pre-checked-out repository at: ${checkoutPath}`); // Check if the pre-checked-out folder exists if (!pathExists(checkoutPath)) { @@ -320,7 +309,7 @@ async function mergeRepositoryGithubFolder(owner, repo, ref, workspace) { // Check if .github folder exists in the checked-out repository const sourceGithubFolder = path.join(checkoutPath, ".github"); if (!pathExists(sourceGithubFolder)) { - coreObj.warning(`Remote repository ${owner}/${repo}@${ref} does not contain a .github folder`); + core.warning(`Remote repository ${owner}/${repo}@${ref} does not contain a .github folder`); return; } @@ -334,24 +323,24 @@ async function mergeRepositoryGithubFolder(owner, repo, ref, workspace) { } catch (err) { throw new Error(`Failed to create directory ${destGithubFolder}: ${String(err)}`, { cause: err }); } - coreObj.info("Created .github folder in workspace"); + core.info("Created .github folder in workspace"); } const { merged, conflicts } = mergeGithubFolder(sourceGithubFolder, destGithubFolder); // Report results if (conflicts.length > 0) { - coreObj.error(`Found ${conflicts.length} file conflicts:`); + core.error(`Found ${conflicts.length} file conflicts:`); for (const conflict of conflicts) { - coreObj.error(` - ${conflict}`); + core.error(` - ${conflict}`); } throw new Error(`${ERR_VALIDATION}: Cannot merge .github folder from ${owner}/${repo}@${ref}: ${conflicts.length} file(s) conflict with existing files`); } if (merged > 0) { - coreObj.info(`Successfully merged ${merged} file(s) from ${owner}/${repo}@${ref}`); + core.info(`Successfully merged ${merged} file(s) from ${owner}/${repo}@${ref}`); } else { - coreObj.info("No new files to merge"); + core.info("No new files to merge"); } } @@ -360,12 +349,12 @@ async function mergeRepositoryGithubFolder(owner, repo, ref, workspace) { */ async function main() { try { - coreObj.info("Starting remote .github folder merge"); + core.info("Starting remote .github folder merge"); // Check for repository imports (owner/repo@ref format - merge entire .github folder) const repositoryImportsEnv = process.env.GH_AW_REPOSITORY_IMPORTS; if (repositoryImportsEnv) { - coreObj.info(`Repository imports detected: ${repositoryImportsEnv}`); + core.info(`Repository imports detected: ${repositoryImportsEnv}`); // Parse the JSON array of repository imports let repositoryImports; @@ -387,11 +376,11 @@ async function main() { // Process each repository import for (const repoImport of repositoryImports) { - coreObj.info(`Processing repository import: ${repoImport}`); + core.info(`Processing repository import: ${repoImport}`); const parsed = parseAgentImportSpec(repoImport); if (!parsed) { - coreObj.warning(`Skipping invalid repository import: ${repoImport}`); + core.warning(`Skipping invalid repository import: ${repoImport}`); continue; } @@ -399,7 +388,7 @@ async function main() { await mergeRepositoryGithubFolder(owner, repo, ref, workspace); } - coreObj.info("All repository imports processed successfully"); + core.info("All repository imports processed successfully"); return; } @@ -407,30 +396,30 @@ async function main() { // Get agent file path from environment const agentFile = process.env.GH_AW_AGENT_FILE; if (!agentFile) { - coreObj.info("No GH_AW_AGENT_FILE or GH_AW_REPOSITORY_IMPORTS specified, skipping .github folder merge"); + core.info("No GH_AW_AGENT_FILE or GH_AW_REPOSITORY_IMPORTS specified, skipping .github folder merge"); return; } - coreObj.info(`Agent file: ${agentFile}`); + core.info(`Agent file: ${agentFile}`); // Get agent import specification const importSpec = process.env.GH_AW_AGENT_IMPORT_SPEC; if (!importSpec) { - coreObj.info("No GH_AW_AGENT_IMPORT_SPEC specified, assuming local agent"); + core.info("No GH_AW_AGENT_IMPORT_SPEC specified, assuming local agent"); return; } - coreObj.info(`Agent import spec: ${importSpec}`); + core.info(`Agent import spec: ${importSpec}`); // Parse import specification const parsed = parseAgentImportSpec(importSpec); if (!parsed) { - coreObj.info("Agent is local or import spec is invalid, skipping remote merge"); + core.info("Agent is local or import spec is invalid, skipping remote merge"); return; } const { owner, repo, ref } = parsed; - coreObj.info(`Remote agent detected: ${owner}/${repo}@${ref}`); + core.info(`Remote agent detected: ${owner}/${repo}@${ref}`); // Get workspace path const workspace = process.env.GITHUB_WORKSPACE; @@ -440,17 +429,17 @@ async function main() { await mergeRepositoryGithubFolder(owner, repo, ref, workspace); - coreObj.info("Remote .github folder merge completed successfully"); + core.info("Remote .github folder merge completed successfully"); } catch (error) { const errorMessage = getErrorMessage(error); - coreObj.setFailed(`Failed to merge remote .github folder: ${errorMessage}`); + core.setFailed(`Failed to merge remote .github folder: ${errorMessage}`); } } // Run if executed directly (not imported) if (require.main === module) { main().catch(err => { - coreObj.setFailed(err && err.stack ? err.stack : String(err)); + core.setFailed(err && err.stack ? err.stack : String(err)); }); } diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index e9ffdfceb5c..6d300ca7e82 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -31,6 +31,7 @@ module.exports = [ "gh-aw-custom/require-return-after-core-setfailed": "warn", "gh-aw-custom/require-spawnsync-error-check": "warn", "gh-aw-custom/require-new-url-try-catch": "warn", + "gh-aw-custom/prefer-core-logging": "warn", "gh-aw-custom/no-core-error-then-process-exit": "warn", }, }, diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index baa2b65d55a..9fee1a0ea63 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -17,6 +17,7 @@ import { requireMkdirSyncTryCatchRule } from "./rules/require-mkdirsync-try-catc import { requireReturnAfterCoreSetFailedRule } from "./rules/require-return-after-core-setfailed"; import { requireSpawnSyncErrorCheckRule } from "./rules/require-spawnsync-error-check"; import { requireNewUrlTryCatchRule } from "./rules/require-new-url-try-catch"; +import { preferCoreLoggingRule } from "./rules/prefer-core-logging"; import { noCoreErrorThenProcessExitRule } from "./rules/no-core-error-then-process-exit"; const plugin = { @@ -44,6 +45,7 @@ const plugin = { "require-return-after-core-setfailed": requireReturnAfterCoreSetFailedRule, "require-spawnsync-error-check": requireSpawnSyncErrorCheckRule, "require-new-url-try-catch": requireNewUrlTryCatchRule, + "prefer-core-logging": preferCoreLoggingRule, "no-core-error-then-process-exit": noCoreErrorThenProcessExitRule, }, }; diff --git a/eslint-factory/src/rules/prefer-core-logging.test.ts b/eslint-factory/src/rules/prefer-core-logging.test.ts new file mode 100644 index 00000000000..1437fd2cd02 --- /dev/null +++ b/eslint-factory/src/rules/prefer-core-logging.test.ts @@ -0,0 +1,192 @@ +import { RuleTester } from "eslint"; +import { describe, expect, it } from "vitest"; +import { preferCoreLoggingRule } from "./prefer-core-logging"; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("prefer-core-logging", () => { + it("uses the correct docs URL", () => { + expect(preferCoreLoggingRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#prefer-core-logging"); + }); + + it("hasSuggestions enabled", () => { + expect(preferCoreLoggingRule.meta.hasSuggestions).toBe(true); + }); + + it("invalid: plain console.log with no core in scope is now flagged", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `console.log("hello");`, + errors: [ + { messageId: "preferCoreLogging", data: { method: "log", replacement: "core.info" }, suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `"hello"` }, output: `core.info("hello");` }] }, + ], + }, + { + code: `console.error("bad thing");`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "error", replacement: "core.error" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.error", args: `"bad thing"` }, output: `core.error("bad thing");` }], + }, + ], + }, + { + code: `const foo = "bar"; console.log(foo);`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "log", replacement: "core.info" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `foo` }, output: `const foo = "bar"; core.info(foo);` }], + }, + ], + }, + ], + }); + }); + + it("valid: core.info calls are fine", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [ + `const core = require("@actions/core"); core.info("hello");`, + `const core = require("@actions/core"); core.error("bad");`, + `const core = require("@actions/core"); core.warning("warn");`, + `const core = require("@actions/core"); core.debug("debug");`, + ], + invalid: [], + }); + }); + + it("invalid: console.log in a function where core is not declared", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `function helper() { console.log("no core here"); }`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "log", replacement: "core.info" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `"no core here"` }, output: `function helper() { core.info("no core here"); }` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.log when core is in scope via require", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `const core = require("@actions/core"); console.log("hello");`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "log", replacement: "core.info" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `"hello"` }, output: `const core = require("@actions/core"); core.info("hello");` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.error when core is in scope", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `const core = require("@actions/core"); console.error("bad thing");`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "error", replacement: "core.error" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.error", args: `"bad thing"` }, output: `const core = require("@actions/core"); core.error("bad thing");` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.warn when core is in scope", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `const core = require("@actions/core"); console.warn("warning");`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "warn", replacement: "core.warning" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.warning", args: `"warning"` }, output: `const core = require("@actions/core"); core.warning("warning");` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.debug when core is in scope", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `const core = require("@actions/core"); console.debug("verbose");`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "debug", replacement: "core.debug" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.debug", args: `"verbose"` }, output: `const core = require("@actions/core"); core.debug("verbose");` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.log when core is a function parameter", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `async function run(core) { console.log("done"); }`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "log", replacement: "core.info" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `"done"` }, output: `async function run(core) { core.info("done"); }` }], + }, + ], + }, + ], + }); + }); + + it("invalid: console.log with multi-arg call when core is in scope", () => { + ruleTester.run("prefer-core-logging", preferCoreLoggingRule, { + valid: [], + invalid: [ + { + code: `const core = require("@actions/core"); const someVar = 1; console.log("value:", someVar);`, + errors: [ + { + messageId: "preferCoreLogging", + data: { method: "log", replacement: "core.info" }, + suggestions: [{ messageId: "replaceWithCoreMethod", data: { replacement: "core.info", args: `"value:", someVar` }, output: `const core = require("@actions/core"); const someVar = 1; core.info("value:", someVar);` }], + }, + ], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/prefer-core-logging.ts b/eslint-factory/src/rules/prefer-core-logging.ts new file mode 100644 index 00000000000..afaf010640f --- /dev/null +++ b/eslint-factory/src/rules/prefer-core-logging.ts @@ -0,0 +1,77 @@ +import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +// Maps console method → recommended core replacement +const CONSOLE_TO_CORE: Record = { + log: "core.info", + info: "core.info", + warn: "core.warning", + error: "core.error", + debug: "core.debug", +}; + +/** + * Returns the `console` method name if the call expression is a `console.*` + * call that has a known core replacement, otherwise null. + */ +function getConsoleMethod(node: TSESTree.CallExpression): string | null { + const callee = node.callee; + if (callee.type !== AST_NODE_TYPES.MemberExpression) return null; + if (callee.computed) return null; + const obj = callee.object; + const prop = callee.property; + if (obj.type !== AST_NODE_TYPES.Identifier || obj.name !== "console") return null; + if (prop.type !== AST_NODE_TYPES.Identifier) return null; + return prop.name in CONSOLE_TO_CORE ? prop.name : null; +} + +export const preferCoreLoggingRule = createRule({ + name: "prefer-core-logging", + meta: { + type: "suggestion", + hasSuggestions: true, + docs: { + description: + "Prefer @actions/core logging methods (core.info, core.error, core.warning, core.debug) over console.* — " + + "global.core is always available via shim.cjs in Node.js context and via github-script in Actions context. " + + "core.* methods integrate with the Actions annotation system (errors and warnings appear as file annotations in the UI) and produce structured log output; console.* does not.", + }, + schema: [], + messages: { + preferCoreLogging: "Use {{replacement}} instead of console.{{method}}() — @actions/core logging integrates with the Actions annotation system and structured output. global.core is always available via shim.cjs.", + replaceWithCoreMethod: "Replace with {{replacement}}({{args}}).", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + + return { + CallExpression(node) { + const method = getConsoleMethod(node); + if (!method) return; + + const replacement = CONSOLE_TO_CORE[method]!; + + // Build replacement argument text from original call + const argsText = node.arguments.map(arg => sourceCode.getText(arg)).join(", "); + + context.report({ + node, + messageId: "preferCoreLogging", + data: { method, replacement }, + suggest: [ + { + messageId: "replaceWithCoreMethod", + data: { replacement, args: argsText }, + fix(fixer) { + return fixer.replaceText(node, `${replacement}(${argsText})`); + }, + }, + ], + }); + }, + }; + }, +});