Code review: centralize types, add caching, improve documentation

src/core/contains.ts

@@ -1,9 +1,11 @@
-export default function contains<T>(array: T[], obj: T): boolean {
-  for (let i = 0; i < array.length; i++) {
-    if (array[i] === obj) {
-      return true;
-    }
-  }
-
-  return false;
+/**
+ * Checks if an array contains a specific value
+ * Uses strict equality (===) for comparison
+ *
+ * @param array - The array to search in
+ * @param obj - The value to search for
+ * @returns true if the array contains the value, false otherwise
+ */
+export default function contains<T>(array: readonly T[], obj: T): boolean {
+  return array.includes(obj);
 }

src/core/evaluate.ts

@@ -1,7 +1,14 @@
+/**
+ * Expression evaluation module
+ *
+ * This module contains the core evaluation logic for executing parsed expressions.
+ * It uses a stack-based interpreter to evaluate instruction sequences produced by the parser.
+ */
+
 import { INUMBER, IOP1, IOP2, IOP3, IVAR, IVARNAME, IFUNCALL, IFUNDEF, IEXPR, IEXPREVAL, IMEMBER, IENDSTATEMENT, IARRAY, IUNDEFINED, ICASEMATCH, IWHENMATCH, ICASEELSE, ICASECOND, IWHENCOND, IOBJECT, IPROPERTY, IOBJECTEND } from '../parsing/instruction.js';
 import type { Instruction } from '../parsing/instruction.js';
 import type { Expression } from './expression.js';
-import type { Value, Values } from '../types/values.js';
+import type { Value, Values, VariableResolveResult, VariableAlias, VariableValue } from '../types/values.js';
 import { VariableError } from '../types/errors.js';
 import { ExpressionValidator } from '../validation/expression-validator.js';
 
@@ -10,23 +17,19 @@ import { ExpressionValidator } from '../validation/expression-validator.js';
 // cSpell:words IOBJECT IOBJECTEND
 // cSpell:words nstack
 
-// Resolver result types (matching parser definitions)
-interface VariableAlias {
-  alias: string;
-}
-
-interface VariableValue {
-  value: Value;
-}
-
-type VariableResolveResult = VariableAlias | VariableValue | Value | undefined;
-
+/**
+ * Wrapper for lazy expression evaluation
+ * Used for short-circuit evaluation of logical operators and conditionals
+ */
 interface ExpressionEvaluator {
   type: typeof IEXPREVAL;
   value: (scope: Values) => Value | Promise<Value>;
 }
 
+/** Type alias for evaluation context values */
 type EvaluationValues = Values;
+
+/** Type alias for the evaluation stack (stores intermediate results) */
 type EvaluationStack = any[];
 
 /**

src/core/simplify.ts

@@ -1,7 +1,5 @@
 import { Instruction, INUMBER, IOP1, IOP2, IOP3, IVAR, IVARNAME, IEXPR, IMEMBER, IARRAY } from '../parsing/instruction.js';
-
-// Type for operator functions
-type OperatorFunction = (...args: any[]) => any;
+import type { OperatorFunction } from '../types/parser.js';
 
 export default function simplify(
   tokens: Instruction[],

src/language-service/language-service.ts

@@ -46,19 +46,59 @@ export function createLanguageService(options: LanguageServiceOptions | undefine
     ...DEFAULT_CONSTANT_DOCS
   } as Record<string, string>;
 
+  // Instance-level cache for function details and names
+  // Each language service instance maintains its own cache, making this thread-safe
+  // as concurrent uses will operate on separate instances
+  let cachedFunctions: FunctionDetails[] | null = null;
+  let cachedFunctionNames: Set<string> | null = null;
+  let cachedConstants: string[] | null = null;
+
+  /**
+   * Returns all available functions with their details
+   * Results are cached for performance within this instance
+   */
   function allFunctions(): FunctionDetails[] {
+    if (cachedFunctions !== null) {
+      return cachedFunctions;
+    }
+
     // Parser exposes built-in functions on parser.functions
     const definedFunctions = parser.functions ? Object.keys(parser.functions) : [];
     // Unary operators can also be used like functions with parens: sin(x), abs(x), ...
     const unary = parser.unaryOps ? Object.keys(parser.unaryOps) : [];
     // Merge, prefer functions map descriptions where available
     const rawFunctions = Array.from(new Set([...definedFunctions, ...unary]));
 
-    return rawFunctions.map(name => new FunctionDetails(parser, name));
+    cachedFunctions = rawFunctions.map(name => new FunctionDetails(parser, name));
+    cachedFunctionNames = new Set(rawFunctions);
+    return cachedFunctions;
+  }
+
+  /**
+   * Returns a set of function names for fast lookup
+   * This ensures the cache is populated before returning
+   */
+  function functionNamesSet(): Set<string> {
+    if (cachedFunctionNames !== null) {
+      return cachedFunctionNames;
+    }
+    // Calling allFunctions() ensures cachedFunctionNames is populated
+    allFunctions();
+    // After allFunctions(), cachedFunctionNames is guaranteed to be non-null
+    // We return a fallback empty set only as a defensive measure
+    return cachedFunctionNames ?? new Set<string>();
   }
 
+  /**
+   * Returns all available constants
+   * Results are cached for performance within this instance
+   */
   function allConstants(): string[] {
-    return parser.consts ? Object.keys(parser.consts) : [];
+    if (cachedConstants !== null) {
+      return cachedConstants;
+    }
+    cachedConstants = parser.consts ? Object.keys(parser.consts) : [];
+    return cachedConstants;
   }
 
   function tokenKindToHighlight(t: Token): HighlightToken['type'] {
@@ -79,9 +119,8 @@ export function createLanguageService(options: LanguageServiceOptions | undefine
         return 'punctuation';
       case TNAME:
       default: {
-        // If not matches, check if it's a function or an identifier
-        const functions = allFunctions();
-        if (t.type === TNAME && functions.find((f: FunctionDetails) => f.name == String(t.value))) {
+        // Use cached set for fast function name lookup
+        if (t.type === TNAME && functionNamesSet().has(String(t.value))) {
           return 'function';
         }
 

src/parsing/instruction.ts

@@ -2,31 +2,69 @@
 // cSpell:words IFUNDEF IUNDEFINED ICASEMATCH ICASECOND IWHENCOND IWHENMATCH ICASEELSE IPROPERTY
 // cSpell:words IOBJECT IOBJECTEND
 
-// Instruction type constants
+/**
+ * Instruction types for the expression evaluator's bytecode-style representation.
+ *
+ * The parser converts expressions into a sequence of instructions that are
+ * executed by the evaluator in a stack-based manner (reverse polish notation).
+ *
+ * Instruction type naming convention:
+ * - I = Instruction prefix
+ * - NUMBER = numeric literal
+ * - OP1/OP2/OP3 = unary/binary/ternary operators
+ * - VAR = variable reference
+ * - FUNCALL = function call
+ * - etc.
+ */
+
+/** Numeric literal instruction */
 export const INUMBER = 'INUMBER' as const;
+/** Unary operator instruction (e.g., negation, factorial) */
 export const IOP1 = 'IOP1' as const;
+/** Binary operator instruction (e.g., +, -, *, /) */
 export const IOP2 = 'IOP2' as const;
+/** Ternary operator instruction (e.g., conditional ?) */
 export const IOP3 = 'IOP3' as const;
+/** Variable reference instruction */
 export const IVAR = 'IVAR' as const;
+/** Variable name instruction (used in assignments) */
 export const IVARNAME = 'IVARNAME' as const;
+/** Function call instruction */
 export const IFUNCALL = 'IFUNCALL' as const;
+/** Function definition instruction */
 export const IFUNDEF = 'IFUNDEF' as const;
+/** Expression instruction (for lazy evaluation) */
 export const IEXPR = 'IEXPR' as const;
+/** Expression evaluator instruction (compiled expression) */
 export const IEXPREVAL = 'IEXPREVAL' as const;
+/** Member access instruction (e.g., obj.property) */
 export const IMEMBER = 'IMEMBER' as const;
+/** End of statement instruction (for multi-statement expressions) */
 export const IENDSTATEMENT = 'IENDSTATEMENT' as const;
+/** Array literal instruction */
 export const IARRAY = 'IARRAY' as const;
+/** Undefined value instruction */
 export const IUNDEFINED = 'IUNDEFINED' as const;
+/** CASE condition instruction (for CASE WHEN without input) */
 export const ICASECOND = 'ICASECOND' as const;
+/** CASE match instruction (for CASE $input WHEN) */
 export const ICASEMATCH = 'ICASEMATCH' as const;
+/** WHEN condition instruction */
 export const IWHENCOND = 'IWHENCOND' as const;
+/** WHEN match instruction */
 export const IWHENMATCH = 'IWHENMATCH' as const;
+/** CASE ELSE instruction */
 export const ICASEELSE = 'ICASEELSE' as const;
+/** Object property instruction */
 export const IPROPERTY = 'IPROPERTY' as const;
+/** Object start instruction */
 export const IOBJECT = 'IOBJECT' as const;
+/** Object end instruction */
 export const IOBJECTEND = 'IOBJECTEND' as const;
 
-// Union type for all instruction types
+/**
+ * Union type for all instruction types
+ */
 export type InstructionType =
   | typeof INUMBER
   | typeof IOP1
@@ -51,7 +89,9 @@ export type InstructionType =
   | typeof IOBJECT
   | typeof IOBJECTEND;
 
-// Discriminated union types for better type safety
+/**
+ * Discriminated union types for better type safety
+ */
 export interface NumberInstruction {
   type: typeof INUMBER;
   value: number;

src/parsing/parser.ts

@@ -3,8 +3,9 @@ import { TEOF } from './token.js';
 import { TokenStream } from './token-stream.js';
 import { ParserState } from './parser-state.js';
 import { Expression } from '../core/expression.js';
-import type { Value } from '../types/values.js';
+import type { Value, VariableResolveResult, Values } from '../types/values.js';
 import type { Instruction } from './instruction.js';
+import type { OperatorFunction } from '../types/parser.js';
 import { atan2, condition, fac, filter, fold, gamma, hypot, indexOf, join, map, max, min, random, roundTo, sum, json, stringLength, isEmpty, stringContains, startsWith, endsWith, searchCount, trim, toUpper, toLower, toTitle, stringJoin, split, repeat, reverse, left, right, replace, replaceFirst, naturalSort, toNumber, toBoolean, padLeft, padRight } from '../functions/index.js';
 import {
   add,
@@ -62,32 +63,19 @@ import {
   log2
 } from '../operators/unary/index.js';
 
-// Type for operator functions - they accept arrays of values and return a value
-type OperatorFunction = (...args: any[]) => any;
-
-// Parser options interface
+/**
+ * Parser options configuration
+ */
 interface ParserOptions {
   allowMemberAccess?: boolean;
   operators?: Record<string, boolean>;
 }
 
-// Variable resolution result types
-interface VariableAlias {
-  alias: string;
-}
-
-interface VariableValue {
-  value: Value;
-}
-
-type VariableResolveResult = VariableAlias | VariableValue | Value | undefined;
-
-// Variable resolver function type
+/**
+ * Variable resolver function type for custom variable resolution
+ */
 type VariableResolver = (token: string) => VariableResolveResult;
 
-// Values object for evaluation
-type Values = Record<string, Value>;
-
 export class Parser {
   public options: ParserOptions;
   public keywords: string[];