Skip to content

eslint-factory: extract nonStringKind to shared module; align setOutput rule with CORE_ALIASES #45973

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.

Sign up for GitHub

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

Merged
pelikhan merged 2 commits into from
Jul 16, 2026
+110 −84
Merged

eslint-factory: extract nonStringKind to shared module; align setOutput rule with CORE_ALIASES #45973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d508d54
Initial plan
Copilot Jul 16, 2026
6a3c491
feat(eslint-factory): extract nonStringKind to shared module; align s…
Copilot Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 2 additions & 41 deletions eslint-factory/src/rules/no-core-exportvariable-non-string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,9 @@
import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils";
import { AST_NODE_TYPES, ESLintUtils, TSESLint } from "@typescript-eslint/utils";
import { CORE_ALIASES } from "./core-aliases";
import { nonStringKind, NULL_KIND, UNDEFINED_KIND } from "./non-string-kind";

const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`);

/**
* Returns a description of the non-string value kind if the node is one of the
* low-false-positive forms targeted by this rule, or null if the value may already
* be a string or cannot be determined without type information.
*
* Targeted forms (low false-positive risk):
* - Numeric literal: 0, 42, 3.14
* - Boolean literal: true, false
* - Null literal
* - Identifier `undefined`
* - .length member access: commonly numeric in practice
*/
const NUMERIC_LITERAL_KIND = "numeric literal" as const;
const BOOLEAN_LITERAL_KIND = "boolean literal" as const;
const NULL_KIND = "null" as const;
const UNDEFINED_KIND = "undefined" as const;
const LENGTH_KIND = ".length (number)" as const;

type NonStringKind = typeof NUMERIC_LITERAL_KIND | typeof BOOLEAN_LITERAL_KIND | typeof NULL_KIND | typeof UNDEFINED_KIND | typeof LENGTH_KIND;

function nonStringKind(node: TSESTree.Node): NonStringKind | null {
if (node.type === AST_NODE_TYPES.Literal) {
if (typeof node.value === "number") return NUMERIC_LITERAL_KIND;
if (typeof node.value === "boolean") return BOOLEAN_LITERAL_KIND;
if (node.value === null) return NULL_KIND;
}

if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") {
return UNDEFINED_KIND;
}

// expr.length — commonly numeric; computed access (expr["length"]) is intentionally
// excluded because it is far less common and raises the FP risk slightly.
if (node.type === AST_NODE_TYPES.MemberExpression && !node.computed && node.property.type === AST_NODE_TYPES.Identifier && node.property.name === "length") {
return LENGTH_KIND;
}

return null;
}

export const noCoreExportVariableNonStringRule = createRule({
name: "no-core-exportvariable-non-string",
meta: {
Expand Down
62 changes: 62 additions & 0 deletions eslint-factory/src/rules/no-core-setoutput-non-string.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ describe("no-core-setoutput-non-string", () => {
});
});

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

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
valid: [`coreObj.setOutput("aic", roundedAIC);`, `coreObj.setOutput("result", "hello");`, `coreObj.setOutput("count", String(items.length));`],
invalid: [],
});
});

it("valid: computed string-literal setOutput with string value is accepted", () => {
cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
valid: [`core["setOutput"]("count", "42");`],
Expand Down Expand Up @@ -185,4 +192,59 @@ describe("no-core-setoutput-non-string", () => {
],
});
});

it("invalid: coreObj alias with numeric value is flagged", () => {
cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
valid: [],
invalid: [
{

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

code: `coreObj.setOutput("aic", 0);`,
errors: [
{
messageId: "nonStringValue",
data: { kind: "numeric literal", valueText: "0" },
suggestions: [{ messageId: "wrapWithString", data: { valueText: "0" }, output: `coreObj.setOutput("aic", String(0));` }],
},
],
},
],
});
});

it("invalid: coreObj alias with boolean value is flagged", () => {
cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
valid: [],
invalid: [
{
code: `coreObj.setOutput("success", true);`,
errors: [
{
messageId: "nonStringValue",
suggestions: [{ messageId: "wrapWithString", output: `coreObj.setOutput("success", String(true));` }],
},
],
},
],
});
});

it("invalid: coreObj alias with null value is flagged", () => {
cjsRuleTester.run("no-core-setoutput-non-string", noCoreSetOutputNonStringRule, {
valid: [],
invalid: [
{
code: `coreObj.setOutput("result", null);`,
errors: [
{
messageId: "nonStringValue",
suggestions: [
{ messageId: "useEmptyString", output: `coreObj.setOutput("result", "");` },
{ messageId: "wrapWithString", output: `coreObj.setOutput("result", String(null));` },
],
},
],
},
],
});
});
});
48 changes: 5 additions & 43 deletions eslint-factory/src/rules/no-core-setoutput-non-string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,9 @@
import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils";
import { AST_NODE_TYPES, ESLintUtils, TSESLint } from "@typescript-eslint/utils";
import { CORE_ALIASES } from "./core-aliases";
import { nonStringKind, NULL_KIND, UNDEFINED_KIND } from "./non-string-kind";

const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`);

/**
* Returns a description of the non-string value kind if the node is one of the
* low-false-positive forms targeted by this rule, or null if the value may already
* be a string or cannot be determined without type information.
*
* Targeted forms (low false-positive risk):
* - Numeric literal: 0, 42, 3.14
* - Boolean literal: true, false
* - Null literal
* - Identifier `undefined`
* - .length member access: commonly numeric in practice
*/
const NUMERIC_LITERAL_KIND = "numeric literal" as const;
const BOOLEAN_LITERAL_KIND = "boolean literal" as const;
const NULL_KIND = "null" as const;
const UNDEFINED_KIND = "undefined" as const;
const LENGTH_KIND = ".length (number)" as const;

type NonStringKind = typeof NUMERIC_LITERAL_KIND | typeof BOOLEAN_LITERAL_KIND | typeof NULL_KIND | typeof UNDEFINED_KIND | typeof LENGTH_KIND;

function nonStringKind(node: TSESTree.Node): NonStringKind | null {
if (node.type === AST_NODE_TYPES.Literal) {
if (typeof node.value === "number") return NUMERIC_LITERAL_KIND;
if (typeof node.value === "boolean") return BOOLEAN_LITERAL_KIND;
if (node.value === null) return NULL_KIND;
}

if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") {
return UNDEFINED_KIND;
}

// expr.length — commonly numeric; computed access (expr["length"]) is intentionally
// excluded because it is far less common and raises the FP risk slightly.
if (node.type === AST_NODE_TYPES.MemberExpression && !node.computed && node.property.type === AST_NODE_TYPES.Identifier && node.property.name === "length") {
return LENGTH_KIND;
}

return null;
}

export const noCoreSetOutputNonStringRule = createRule({
name: "no-core-setoutput-non-string",
meta: {
Expand Down Expand Up @@ -70,8 +32,8 @@ export const noCoreSetOutputNonStringRule = createRule({
// Must be a member expression: something.setOutput(...)
if (callee.type !== AST_NODE_TYPES.MemberExpression) return;

// Object must be `core` (the @actions/core import convention in actions/setup/js)
if (callee.object.type !== AST_NODE_TYPES.Identifier || callee.object.name !== "core") return;
// 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;
Comment on lines +35 to +36

// Property must be `setOutput` (direct or computed string-literal access)
const prop = callee.property;
Expand Down
41 changes: 41 additions & 0 deletions eslint-factory/src/rules/non-string-kind.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { AST_NODE_TYPES, TSESTree } from "@typescript-eslint/utils";

/**
* Returns a description of the non-string value kind if the node is one of the
* low-false-positive forms targeted by these rules, or null if the value may already
* be a string or cannot be determined without type information.
*
* Targeted forms (low false-positive risk):
* - Numeric literal: 0, 42, 3.14
* - Boolean literal: true, false
* - Null literal
* - Identifier `undefined`
* - .length member access: commonly numeric in practice
*/
export const NUMERIC_LITERAL_KIND = "numeric literal" as const;
export const BOOLEAN_LITERAL_KIND = "boolean literal" as const;
export const NULL_KIND = "null" as const;
export const UNDEFINED_KIND = "undefined" as const;
export const LENGTH_KIND = ".length (number)" as const;

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

export type NonStringKind = typeof NUMERIC_LITERAL_KIND | typeof BOOLEAN_LITERAL_KIND | typeof NULL_KIND | typeof UNDEFINED_KIND | typeof LENGTH_KIND;

export function nonStringKind(node: TSESTree.Node): NonStringKind | null {
if (node.type === AST_NODE_TYPES.Literal) {
if (typeof node.value === "number") return NUMERIC_LITERAL_KIND;
if (typeof node.value === "boolean") return BOOLEAN_LITERAL_KIND;
if (node.value === null) return NULL_KIND;
}

if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") {
return UNDEFINED_KIND;
}

// expr.length — commonly numeric; computed access (expr["length"]) is intentionally
// excluded because it is far less common and raises the FP risk slightly.
if (node.type === AST_NODE_TYPES.MemberExpression && !node.computed && node.property.type === AST_NODE_TYPES.Identifier && node.property.name === "length") {
return LENGTH_KIND;
}

return null;
}
Loading