refactor: code review fixes per AGENTS.md rules

- DRY: Extract shared createFileLinesCache in filter-diagnostics.ts
- DRY: Extract resolveFailOnLevel in cli.ts (was duplicated in
  staged and normal paths)
- DRY: Reuse exported getCalleeName from helpers.ts instead of
  duplicating in state-and-effects.ts and performance.ts
- Magic number: Move maxBuffer 10MB to GIT_SHOW_MAX_BUFFER_BYTES constant
- Variable naming: prevLine → previousLine, tagName → fullTagName,
  leafName → leafTagName, configFiles → projectConfigFilenames
- Remove .pop()! non-null assertion in favor of .at(-1) with fallback

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Author: cursoragent

packages/react-doctor/src/cli.ts

@@ -41,6 +41,18 @@ const shouldFailForDiagnostics = (diagnostics: Diagnostic[], failOnLevel: FailOn
   return diagnostics.some((diagnostic) => diagnostic.severity === "error");
 };
 
+const resolveFailOnLevel = (
+  programInstance: Command,
+  flags: CliFlags,
+  userConfig: ReactDoctorConfig | null,
+): FailOnLevel => {
+  const resolvedFailOn =
+    programInstance.getOptionValueSource("failOn") === "cli"
+      ? flags.failOn
+      : (userConfig?.failOn ?? flags.failOn);
+  return isValidFailOnLevel(resolvedFailOn) ? resolvedFailOn : "none";
+};
+
 const printAnnotations = (diagnostics: Diagnostic[]): void => {
   for (const diagnostic of diagnostics) {
     const level = diagnostic.severity === "error" ? "error" : "warning";
@@ -199,15 +211,12 @@ const program = new Command()
             printAnnotations(remappedDiagnostics);
           }
 
-          const resolvedFailOn =
-            program.getOptionValueSource("failOn") === "cli"
-              ? flags.failOn
-              : (userConfig?.failOn ?? flags.failOn);
-          const effectiveFailOn: FailOnLevel = isValidFailOnLevel(resolvedFailOn)
-            ? resolvedFailOn
-            : "none";
-
-          if (shouldFailForDiagnostics(remappedDiagnostics, effectiveFailOn)) {
+          if (
+            shouldFailForDiagnostics(
+              remappedDiagnostics,
+              resolveFailOnLevel(program, flags, userConfig),
+            )
+          ) {
             process.exitCode = 1;
           }
         } finally {
@@ -268,19 +277,13 @@ const program = new Command()
         }
       }
 
-      const resolvedFailOn =
-        program.getOptionValueSource("failOn") === "cli"
-          ? flags.failOn
-          : (userConfig?.failOn ?? flags.failOn);
-      const effectiveFailOn: FailOnLevel = isValidFailOnLevel(resolvedFailOn)
-        ? resolvedFailOn
-        : "none";
-
       if (flags.annotations) {
         printAnnotations(allDiagnostics);
       }
 
-      if (shouldFailForDiagnostics(allDiagnostics, effectiveFailOn)) {
+      if (
+        shouldFailForDiagnostics(allDiagnostics, resolveFailOnLevel(program, flags, userConfig))
+      ) {
         process.exitCode = 1;
       }
     } catch (error) {

packages/react-doctor/src/constants.ts

@@ -48,4 +48,6 @@ export const OXLINT_NODE_REQUIREMENT = "^20.19.0 || >=22.12.0";
 
 export const OXLINT_RECOMMENDED_NODE_MAJOR = 24;
 
+export const GIT_SHOW_MAX_BUFFER_BYTES = 10 * 1024 * 1024;
+
 export const IGNORED_DIRECTORIES = new Set(["node_modules", "dist", "build", "coverage"]);

packages/react-doctor/src/plugin/helpers.ts

@@ -98,7 +98,7 @@ export const isComponentAssignment = (node: EsTreeNode): boolean =>
   Boolean(node.init) &&
   (node.init.type === "ArrowFunctionExpression" || node.init.type === "FunctionExpression");
 
-const getCalleeName = (node: EsTreeNode): string | null => {
+export const getCalleeName = (node: EsTreeNode): string | null => {
   if (node.callee?.type === "Identifier") return node.callee.name;
   if (node.callee?.type === "MemberExpression" && node.callee.property?.type === "Identifier") {
     return node.callee.property.name;

packages/react-doctor/src/plugin/rules/performance.ts

@@ -9,6 +9,7 @@ import {
   SETTER_PATTERN,
 } from "../constants.js";
 import {
+  getCalleeName,
   getEffectCallback,
   isComponentAssignment,
   isHookCall,
@@ -412,16 +413,12 @@ export const renderingHydrationNoFlicker: Rule = {
       if (!bodyStatements || bodyStatements.length !== 1) return;
 
       const soleStatement = bodyStatements[0];
-      const soleCallee = soleStatement?.expression?.callee;
       const soleCalleeName =
-        soleCallee?.type === "Identifier"
-          ? soleCallee.name
-          : soleCallee?.type === "MemberExpression" && soleCallee.property?.type === "Identifier"
-            ? soleCallee.property.name
-            : null;
+        soleStatement?.expression?.type === "CallExpression"
+          ? getCalleeName(soleStatement.expression)
+          : null;
       if (
         soleStatement?.type === "ExpressionStatement" &&
-        soleStatement.expression?.type === "CallExpression" &&
         soleCalleeName &&
         SETTER_PATTERN.test(soleCalleeName)
       ) {

packages/react-doctor/src/plugin/rules/state-and-effects.ts

@@ -10,6 +10,7 @@ import {
   countSetStateCalls,
   extractDestructuredPropNames,
   getCallbackStatements,
+  getCalleeName,
   getEffectCallback,
   isComponentAssignment,
   isHookCall,
@@ -40,21 +41,10 @@ export const noDerivedStateEffect: Rule = {
       const statements = getCallbackStatements(callback);
       if (statements.length === 0) return;
 
-      const getSetterName = (expression: EsTreeNode): string | null => {
-        if (expression.callee?.type === "Identifier") return expression.callee.name;
-        if (
-          expression.callee?.type === "MemberExpression" &&
-          expression.callee.property?.type === "Identifier"
-        ) {
-          return expression.callee.property.name;
-        }
-        return null;
-      };
-
       const containsOnlySetStateCalls = statements.every((statement: EsTreeNode) => {
         if (statement.type !== "ExpressionStatement") return false;
         if (statement.expression?.type !== "CallExpression") return false;
-        const name = getSetterName(statement.expression);
+        const name = getCalleeName(statement.expression);
         return name !== null && isSetterIdentifier(name);
       });
       if (!containsOnlySetStateCalls) return;
@@ -258,15 +248,7 @@ export const rerenderLazyStateInit: Rule = {
 export const rerenderFunctionalSetstate: Rule = {
   create: (context: RuleContext) => ({
     CallExpression(node: EsTreeNode) {
-      let calleeName: string | null = null;
-      if (node.callee?.type === "Identifier") {
-        calleeName = node.callee.name;
-      } else if (
-        node.callee?.type === "MemberExpression" &&
-        node.callee.property?.type === "Identifier"
-      ) {
-        calleeName = node.callee.property.name;
-      }
+      const calleeName = getCalleeName(node);
       if (!calleeName || !isSetterIdentifier(calleeName)) return;
       if (!node.arguments?.length) return;
 

packages/react-doctor/src/utils/filter-diagnostics.ts

@@ -4,6 +4,26 @@ import type { Diagnostic, ReactDoctorConfig } from "../types.js";
 import { compileIgnoredFilePatterns, isFileIgnoredByPatterns } from "./is-ignored-file.js";
 
 const OPENING_TAG_PATTERN = /<([A-Z][\w.]*)/;
+const DISABLE_NEXT_LINE_PATTERN = /\/\/\s*react-doctor-disable-next-line\b(?:\s+(.+))?/;
+const DISABLE_LINE_PATTERN = /\/\/\s*react-doctor-disable-line\b(?:\s+(.+))?/;
+
+const createFileLinesCache = (rootDirectory: string) => {
+  const cache = new Map<string, string[] | null>();
+
+  return (filePath: string): string[] | null => {
+    const cached = cache.get(filePath);
+    if (cached !== undefined) return cached;
+    const absolutePath = path.isAbsolute(filePath) ? filePath : path.join(rootDirectory, filePath);
+    try {
+      const lines = fs.readFileSync(absolutePath, "utf-8").split("\n");
+      cache.set(filePath, lines);
+      return lines;
+    } catch {
+      cache.set(filePath, null);
+      return null;
+    }
+  };
+};
 
 const isInsideTextComponent = (
   lines: string[],
@@ -13,13 +33,20 @@ const isInsideTextComponent = (
   for (let lineIndex = diagnosticLine - 1; lineIndex >= 0; lineIndex--) {
     const match = lines[lineIndex].match(OPENING_TAG_PATTERN);
     if (!match) continue;
-    const tagName = match[1];
-    const leafName = tagName.includes(".") ? tagName.split(".").pop()! : tagName;
-    return textComponentNames.has(tagName) || textComponentNames.has(leafName);
+    const fullTagName = match[1];
+    const leafTagName = fullTagName.includes(".")
+      ? (fullTagName.split(".").at(-1) ?? fullTagName)
+      : fullTagName;
+    return textComponentNames.has(fullTagName) || textComponentNames.has(leafTagName);
   }
   return false;
 };
 
+const isRuleSuppressed = (commentRules: string | undefined, ruleId: string): boolean => {
+  if (!commentRules?.trim()) return true;
+  return commentRules.split(/[,\s]+/).some((rule) => rule.trim() === ruleId);
+};
+
 export const filterIgnoredDiagnostics = (
   diagnostics: Diagnostic[],
   config: ReactDoctorConfig,
@@ -31,21 +58,7 @@ export const filterIgnoredDiagnostics = (
     Array.isArray(config.textComponents) ? config.textComponents : [],
   );
   const hasTextComponents = textComponentNames.size > 0;
-
-  const fileLineCache = new Map<string, string[] | null>();
-  const getFileLines = (filePath: string): string[] | null => {
-    const cached = fileLineCache.get(filePath);
-    if (cached !== undefined) return cached;
-    const absolutePath = path.isAbsolute(filePath) ? filePath : path.join(rootDirectory, filePath);
-    try {
-      const lines = fs.readFileSync(absolutePath, "utf-8").split("\n");
-      fileLineCache.set(filePath, lines);
-      return lines;
-    } catch {
-      fileLineCache.set(filePath, null);
-      return null;
-    }
-  };
+  const getFileLines = createFileLinesCache(rootDirectory);
 
   return diagnostics.filter((diagnostic) => {
     const ruleIdentifier = `${diagnostic.plugin}/${diagnostic.rule}`;
@@ -68,33 +81,11 @@ export const filterIgnoredDiagnostics = (
   });
 };
 
-const DISABLE_NEXT_LINE_PATTERN = /\/\/\s*react-doctor-disable-next-line\b(?:\s+(.+))?/;
-const DISABLE_LINE_PATTERN = /\/\/\s*react-doctor-disable-line\b(?:\s+(.+))?/;
-
-const isRuleSuppressed = (commentRules: string | undefined, ruleId: string): boolean => {
-  if (!commentRules?.trim()) return true;
-  return commentRules.split(/[,\s]+/).some((rule) => rule.trim() === ruleId);
-};
-
 export const filterInlineSuppressions = (
   diagnostics: Diagnostic[],
   rootDirectory: string,
 ): Diagnostic[] => {
-  const fileLineCache = new Map<string, string[] | null>();
-
-  const getFileLines = (filePath: string): string[] | null => {
-    const cached = fileLineCache.get(filePath);
-    if (cached !== undefined) return cached;
-    const absolutePath = path.isAbsolute(filePath) ? filePath : path.join(rootDirectory, filePath);
-    try {
-      const lines = fs.readFileSync(absolutePath, "utf-8").split("\n");
-      fileLineCache.set(filePath, lines);
-      return lines;
-    } catch {
-      fileLineCache.set(filePath, null);
-      return null;
-    }
-  };
+  const getFileLines = createFileLinesCache(rootDirectory);
 
   return diagnostics.filter((diagnostic) => {
     if (diagnostic.line <= 0) return true;
@@ -111,9 +102,9 @@ export const filterInlineSuppressions = (
     }
 
     if (diagnostic.line >= 2) {
-      const prevLine = lines[diagnostic.line - 2];
-      if (prevLine) {
-        const nextLineMatch = prevLine.match(DISABLE_NEXT_LINE_PATTERN);
+      const previousLine = lines[diagnostic.line - 2];
+      if (previousLine) {
+        const nextLineMatch = previousLine.match(DISABLE_NEXT_LINE_PATTERN);
         if (nextLineMatch && isRuleSuppressed(nextLineMatch[1], ruleId)) return false;
       }
     }

packages/react-doctor/src/utils/get-staged-files.ts

@@ -1,7 +1,7 @@
 import { execSync } from "node:child_process";
 import fs from "node:fs";
 import path from "node:path";
-import { SOURCE_FILE_PATTERN } from "../constants.js";
+import { GIT_SHOW_MAX_BUFFER_BYTES, SOURCE_FILE_PATTERN } from "../constants.js";
 
 const getStagedFilePaths = (directory: string): string[] => {
   try {
@@ -23,7 +23,7 @@ const readStagedContent = (directory: string, relativePath: string): string | nu
     return execSync(`git show ":${relativePath}"`, {
       cwd: directory,
       stdio: "pipe",
-      maxBuffer: 10 * 1024 * 1024,
+      maxBuffer: GIT_SHOW_MAX_BUFFER_BYTES,
     }).toString();
   } catch {
     return null;
@@ -56,8 +56,8 @@ export const materializeStagedFiles = (
     materializedFiles.push(relativePath);
   }
 
-  const configFiles = ["tsconfig.json", "package.json"];
-  for (const configFile of configFiles) {
+  const projectConfigFilenames = ["tsconfig.json", "package.json"];
+  for (const configFile of projectConfigFilenames) {
     const sourcePath = path.join(directory, configFile);
     const targetPath = path.join(tempDirectory, configFile);
     if (fs.existsSync(sourcePath) && !fs.existsSync(targetPath)) {