eslint-factory: extract nonStringKind to shared module; align setOutput rule with CORE_ALIASES by Copilot · Pull Request #45973 · github/gh-aw

Copilot · 2026-07-16T09:56:41Z

no-core-setoutput-non-string hardcoded callee.object.name !== "core", so coreObj.setOutput("x", 0) escaped detection while the identical call on core was flagged. Both rules also embedded a verbatim-duplicated nonStringKind() implementation, creating drift risk.

Changes

New non-string-kind.ts — shared module exporting nonStringKind(), the five kind constants, and NonStringKind type; single source of truth for both rules.
no-core-setoutput-non-string — replaces hardcoded name !== "core" with !CORE_ALIASES.has(name), matching the exportVariable twin. Removes local duplicate, imports from shared module.
no-core-exportvariable-non-string — removes local duplicate, imports from shared module.
Tests — adds valid-case coverage for coreObj string values and invalid cases for coreObj.setOutput with numeric, boolean, and null args:

// Now flagged (was silently ignored before)
coreObj.setOutput("aic", 0);          // → String(0)
coreObj.setOutput("success", true);   // → String(true)
coreObj.setOutput("result", null);    // → "" or String(null)

…etOutput rule with CORE_ALIASES - Create `non-string-kind.ts` with shared `nonStringKind()`, kind constants, and `NonStringKind` type - Update `no-core-setoutput-non-string` to use `CORE_ALIASES` (adds `coreObj` coverage) and import from shared module - Update `no-core-exportvariable-non-string` to import from shared module (removes duplicate) - Add `coreObj.setOutput` valid and invalid test cases Closes #45929 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot

Pull request overview

Centralizes non-string value detection and extends setOutput linting to the coreObj alias.

Changes:

Extracts shared non-string classification logic.
Aligns setOutput alias handling with exportVariable.
Adds coreObj.setOutput coverage.

Show a summary per file

File	Description
`non-string-kind.ts`	Adds shared value classification.
`no-core-setoutput-non-string.ts`	Supports all core aliases.
`no-core-setoutput-non-string.test.ts`	Tests `coreObj` behavior.
`no-core-exportvariable-non-string.ts`	Uses the shared classifier.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Medium

+        // Object must be a known @actions/core alias (`core` or `coreObj`)
+        if (callee.object.type !== AST_NODE_TYPES.Identifier || !CORE_ALIASES.has(callee.object.name)) return;


github-actions · 2026-07-16T11:29:22Z

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

github-actions · 2026-07-16T11:29:22Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-16T11:29:23Z

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

github-actions · 2026-07-16T11:29:41Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

github-actions

{"pull_request_number":45973,"event":"COMMENT","body":"### Review Summary

The refactoring is clean and correct. The shared non-string-kind.ts module eliminates drift risk, and the CORE_ALIASES fix closes the detection gap for coreObj.setOutput. New tests cover the previously-missed cases.

The one outstanding item is already captured in the inline comment: meta.docs.description (and likely README) still references only core.setOutput, which understates the rule scope after the CORE_ALIASES expansion."}

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 18.4 AIC · ⌖ 4.29 AIC · ⊞ 5K

github-actions

Skills-Based Review 🧠

Applied /codebase-design and /tdd — commenting with minor improvement suggestions; no blocking issues.

📋 Key Themes & Highlights

Key Themes

Test constant coupling: new coreObj test cases use raw string literals (e.g. "numeric literal") that duplicate values from the freshly-extracted constants — a constant rename would silently leave tests passing.
Surface width: three of the five exported constants (NUMERIC_LITERAL_KIND, BOOLEAN_LITERAL_KIND, LENGTH_KIND) are not imported by any consumer today; exporting them without a stated reason widens the API unnecessarily.
Coverage gap: the new valid-case test for coreObj could add a .length invalid case to match the parity the core alias tests already have.

Positive Highlights

✅ Clean DRY extraction — 78 lines of duplicated logic removed across two rule files
✅ Bug fix is well-motivated and the PR description is thorough
✅ Good test additions covering numeric, boolean, and null cases for the new alias
✅ Symmetric fix: setOutput rule now uses CORE_ALIASES consistent with exportVariable

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 32.3 AIC · ⌖ 4.63 AIC · ⊞ 6.7K
_{Comment /matt to run again}

github-actions · 2026-07-16T11:37:39Z

+    cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
+      valid: [],
+      invalid: [
+        {


[/tdd] Test assertions use raw string literals for kind data instead of the exported constants — if a constant value changes, the tests will silently pass against stale strings.

💡 Suggested fix

Import and use the constants:

import { NUMERIC_LITERAL_KIND, BOOLEAN_LITERAL_KIND, NULL_KIND } from "./non-string-kind"; // then: data: { kind: NUMERIC_LITERAL_KIND, valueText: "0" },

This makes the test a true regression guard against constant renames.

@copilot please address this.

github-actions · 2026-07-16T11:37:39Z

+export const NULL_KIND = "null" as const;
+export const UNDEFINED_KIND = "undefined" as const;
+export const LENGTH_KIND = ".length (number)" as const;
+


[/codebase-design] NUMERIC_LITERAL_KIND, BOOLEAN_LITERAL_KIND, and LENGTH_KIND are exported but no rule file imports them — only NULL_KIND and UNDEFINED_KIND are imported externally. Exporting unused symbols unnecessarily widens the module's public surface.

💡 Suggestion

Keep only NULL_KIND, UNDEFINED_KIND, NonStringKind, and nonStringKind exported. Either remove the export keyword from the other three constants, or document the intent to make them a public API.

@copilot please address this.

github-actions · 2026-07-16T11:37:39Z

    });
  });

+  it("valid: coreObj alias with string value is accepted", () => {


[/tdd] The new valid-case test covers roundedAIC (an identifier) and "hello" (a string literal), but not expr.length — the .length form is a supported valid passthrough in the shared nonStringKind() (returns non-null, so it should be flagged, not accepted). This could be a spec question worth clarifying with a test.

💡 Context

nonStringKind returns LENGTH_KIND for expr.length, meaning coreObj.setOutput("x", arr.length) is flagged. A test that explicitly asserts this for the coreObj alias (like the core alias tests already do) would complete the coverage.

@copilot please address this.

github-actions

Review: Non-blocking observations

The core change is correct — extracting nonStringKind() to a shared module and aligning setOutput to use CORE_ALIASES fixes the real bug. Two issues with the new tests are worth addressing before this is considered complete.

📋 Findings

1. Boolean test omits data field (no-core-setoutput-non-string.test.ts line ~104)
The numeric coreObj test verifies data: { kind, valueText } but the boolean test does not, so a broken message interpolation for booleans goes undetected.

2. Hardcoded kind string literals in test assertions (no-core-setoutput-non-string.test.ts line ~89)
Tests use "numeric literal" etc. as raw strings instead of importing the exported constants from non-string-kind.ts. If a constant value is renamed, tests silently pass while the rule emits the new name — defeating the shared-module centralisation.

(An existing comment on line 36 of no-core-setoutput-non-string.ts also flags that meta.docs.description still says core.setOutput only and should be updated.)

🔎 Code quality review by PR Code Quality Reviewer · 36.8 AIC · ⌖ 4.58 AIC · ⊞ 5.6K
_{Comment /review to run again}

Comments that could not be inline-anchored

eslint-factory/src/rules/no-core-setoutput-non-string.test.ts:104

Boolean error assertion omits data field, leaving message interpolation unverified. The numeric coreObj test (a few lines above) explicitly asserts data: { kind: "numeric literal", valueText: "0" }, but this boolean case does not, so a broken kind or valueText in the boolean branch would go undetected.

<details>
<summary>💡 Suggested fix</summary>

Add the data field to match the numeric test's pattern:

errors: [
  {
    messageId: &quot;nonStringValue&quot;,
    data: { kind: &quot;bool…

</details>

<details><summary>eslint-factory/src/rules/no-core-setoutput-non-string.test.ts:89</summary>

**Test assertions hardcode kind string literals instead of importing the exported constants**, defeating the centralisation benefit of the new shared module.

&lt;details&gt;
&lt;summary&gt;💡 Suggested fix&lt;/summary&gt;

Import the constants and use them in `data` fields:

```ts
import { NUMERIC_LITERAL_KIND, BOOLEAN_LITERAL_KIND, NULL_KIND } from &quot;./non-string-kind&quot;;

// then in errors:
data: { kind: NUMERIC_LITERAL_KIND, valueText: &quot;0&quot; },

If NUMERIC_LITERAL_KIND is renamed from "numeric literal" to…

github-actions · 2026-07-16T19:58:12Z

🎉 This pull request is included in a new release.

Release: v0.82.11

Initial plan

d508d54

Copilot AI assigned Copilot and pelikhan Jul 16, 2026

Copilot started work on behalf of pelikhan July 16, 2026 09:57 View session

Copilot AI linked an issue Jul 16, 2026 that may be closed by this pull request

[ESLint Factory] no-core-setoutput-non-string: align with CORE_ALIASES (misses grounded coreObj.setOutput) and de-duplicate nonS [Content truncated due to length] #45929

Closed

Copilot AI changed the title ~~[WIP] Fix no-core-setoutput-non-string to align with CORE_ALIASES~~ eslint-factory: extract nonStringKind to shared module; align setOutput rule with CORE_ALIASES Jul 16, 2026

Copilot AI requested a review from pelikhan July 16, 2026 10:05

Copilot finished work on behalf of pelikhan July 16, 2026 10:05

pelikhan marked this pull request as ready for review July 16, 2026 10:17

Copilot AI review requested due to automatic review settings July 16, 2026 10:17

Copilot started reviewing on behalf of pelikhan July 16, 2026 10:17 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

Comment thread eslint-factory/src/rules/no-core-setoutput-non-string.ts

Comment on lines +35 to +36

// Object must be a known @actions/core alias (`core` or `coreObj`)

if (callee.object.type !== AST_NODE_TYPES.Identifier || !CORE_ALIASES.has(callee.object.name)) return;

github-actions Bot reviewed Jul 16, 2026

View reviewed changes

pelikhan merged commit 0b4c8aa into main Jul 16, 2026
35 checks passed

pelikhan deleted the copilot/fix-no-core-setoutput-non-string-issue branch July 16, 2026 11:40

github-actions Bot mentioned this pull request Jul 16, 2026

Smoke Test: MAI-Code-1-Flash - 29538613340 #46092

Closed

		// Object must be a known @actions/core alias (`core` or `coreObj`)
		if (callee.object.type !== AST_NODE_TYPES.Identifier \|\| !CORE_ALIASES.has(callee.object.name)) return;

Uh oh!

Conversation

Copilot AI commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review: Non-blocking observations

Comments that could not be inline-anchored

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading