style: pr review nitpicks

Author: jrentlez

libs/execution/src/lib/constraints/constraint-executor.ts

@@ -36,8 +36,8 @@ export class ConstraintExecutor<
     if (properties === undefined) {
       context.evaluationContext.setValueForValueKeyword(value);
     } else {
-      const assignment_for_type_system: ValueTypeProperty[] = properties;
-      for (const property of assignment_for_type_system) {
+      const assignmentForTypeSystem: ValueTypeProperty[] = properties;
+      for (const property of assignmentForTypeSystem) {
         context.evaluationContext.setValueForReference(property.name, value);
       }
     }

libs/execution/src/lib/types/value-types/internal-representation-parsing.ts

@@ -17,7 +17,7 @@ import {
   type TextValuetype,
   type ValueType,
   ValueTypeVisitor,
-  collapseArray,
+  onlyElementOrUndefined,
   internalValueToString,
   isCellRangeLiteral,
 } from '@jvalue/jayvee-language-server';
@@ -107,7 +107,7 @@ class InternalRepresentationParserVisitor extends ValueTypeVisitor<
   ): InternalValidValueRepresentation | InvalidValue {
     const containedTypes = valueType.getContainedTypes();
     assert(containedTypes !== undefined);
-    const containedType = collapseArray(containedTypes);
+    const containedType = onlyElementOrUndefined(containedTypes);
     if (containedType === undefined) {
       return new InvalidValue(
         `Cannot parse value types with multiple properties`,

libs/execution/src/lib/types/value-types/visitors/sql-column-type-visitor.ts

@@ -7,7 +7,7 @@ import { strict as assert } from 'assert';
 
 import {
   type AtomicValueType,
-  collapseArray,
+  onlyElementOrUndefined,
   ValueTypeVisitor,
 } from '@jvalue/jayvee-language-server';
 
@@ -31,7 +31,7 @@ export class SQLColumnTypeVisitor extends ValueTypeVisitor<string> {
   override visitAtomicValueType(valueType: AtomicValueType): string {
     const containedTypes = valueType.getContainedTypes();
     assert(containedTypes !== undefined);
-    const containedType = collapseArray(containedTypes);
+    const containedType = onlyElementOrUndefined(containedTypes);
     if (containedType === undefined) {
       throw new Error(
         'Can only determine sql column type for value types with one property',

libs/execution/src/lib/types/value-types/visitors/sql-value-representation-visitor.ts

@@ -8,7 +8,7 @@ import { strict as assert } from 'assert';
 import {
   type AtomicValueType,
   type BooleanValuetype,
-  collapseArray,
+  onlyElementOrUndefined,
   type DecimalValuetype,
   ERROR_TYPEGUARD,
   type IntegerValuetype,
@@ -72,7 +72,7 @@ export class SQLValueRepresentationVisitor extends ValueTypeVisitor<
   ) => string {
     const containedTypes = valueType.getContainedTypes();
     assert(containedTypes !== undefined);
-    const containedType = collapseArray(containedTypes);
+    const containedType = onlyElementOrUndefined(containedTypes);
     if (containedType === undefined) {
       throw new Error(
         'Can only determine sql value representation for value types with one' +

libs/language-server/src/lib/ast/expressions/type-inference.ts

@@ -47,7 +47,7 @@ import {
   pickCommonAtomicValueType,
   pickCommonPrimitiveValuetype,
 } from '../wrappers/util/value-type-util';
-import { collapseArray } from '../../util';
+import { onlyElementOrUndefined } from '../../util';
 
 /**
  * @returns The inferred ValueType. `undefined` means that any type could be
@@ -240,7 +240,7 @@ function inferCollectionType(
     return valueTypeProvider.EmptyCollection;
   }
   if (elementValuetypes.length === 1) {
-    const elementValueType = collapseArray(elementValuetypes);
+    const elementValueType = onlyElementOrUndefined(elementValuetypes);
     assert(elementValueType !== undefined);
     return valueTypeProvider.createCollectionValueTypeOf(elementValueType);
   }

libs/language-server/src/lib/ast/wrappers/util/value-type-util.ts

@@ -14,27 +14,36 @@ import {
   isPrimitiveValueType,
 } from '../value-type/primitive';
 import { type ValueType } from '../value-type/value-type';
-import { collapseArray } from '../../../util';
+import { onlyElementOrUndefined } from '../../../util';
+
+function getBasePrimitiveValueType(
+  valueType: ValueType,
+): PrimitiveValueType | undefined {
+  while (!isPrimitiveValueType(valueType)) {
+    const containedTypes = valueType.getContainedTypes();
+    assert(
+      containedTypes !== undefined,
+      'non-primitive value types always have at least one contained type',
+    );
+    const containedType = onlyElementOrUndefined(containedTypes);
+    if (containedType === undefined) {
+      return undefined;
+    }
+    valueType = containedType;
+  }
+  return valueType;
+}
 
 export function pickCommonPrimitiveValuetype(
   valueTypes: ValueType[],
 ): PrimitiveValueType | undefined {
-  const primitiveValueTypes = valueTypes.flatMap((valueType) => {
-    while (!isPrimitiveValueType(valueType)) {
-      const containedTypes = valueType.getContainedTypes();
-      if (containedTypes === undefined) {
-        return [];
-      }
-      const containedType = collapseArray(containedTypes);
-      if (containedType === undefined) {
-        return [];
-      }
-      valueType = containedType;
+  const primitiveValueTypes: PrimitiveValueType[] = [];
+  for (const valueType of valueTypes) {
+    const primitiveValueType = getBasePrimitiveValueType(valueType);
+    if (primitiveValueType === undefined) {
+      return undefined;
     }
-    return [valueType];
-  });
-  if (primitiveValueTypes.length !== valueTypes.length) {
-    return undefined;
+    primitiveValueTypes.push(primitiveValueType);
   }
 
   let resultingType = primitiveValueTypes.pop();
@@ -86,7 +95,7 @@ export function pickCommonAtomicValueType(
       } else if (isAtomicValueType(valueType)) {
         const containedTypes = valueType.getContainedTypes();
         assert(containedTypes !== undefined);
-        const containedType = collapseArray(containedTypes);
+        const containedType = onlyElementOrUndefined(containedTypes);
         if (containedType === undefined) {
           return [];
         }

libs/language-server/src/lib/ast/wrappers/value-type/abstract-value-type.ts

@@ -16,7 +16,7 @@ export abstract class AbstractValueType<
   abstract acceptVisitor<R>(visitor: ValueTypeVisitor<R>): R;
 
   getContainedTypes(): ValueType[] | undefined {
-    if (this.typeCycleIndex() !== undefined) {
+    if (this.getIndexOfFirstPropertyInATypeCycle() !== undefined) {
       return undefined;
     }
     return this.doGetContainedTypes();
@@ -42,15 +42,19 @@ export abstract class AbstractValueType<
 
   abstract getName(): string;
 
-  typeCycleIndex(visited: ValueType[] = []): number | undefined {
+  getIndexOfFirstPropertyInATypeCycle(
+    visited: ValueType[] = [],
+  ): number | undefined {
     const cycleDetected = visited.some((v) => v.equals(this));
     if (cycleDetected) {
       return -1;
     }
     visited.push(this);
 
     const idx = this.doGetContainedTypes().findIndex(
-      (containedType) => containedType.typeCycleIndex(visited) !== undefined,
+      (containedType) =>
+        containedType.getIndexOfFirstPropertyInATypeCycle(visited) !==
+        undefined,
     );
     return idx !== -1 ? idx : undefined;
   }

libs/language-server/src/lib/ast/wrappers/value-type/atomic-value-type.ts

@@ -20,7 +20,7 @@ import { type WrapperFactoryProvider } from '../wrapper-factory-provider';
 import { AbstractValueType } from './abstract-value-type';
 import { type ValueTypeProvider } from './primitive';
 import { type ValueType, type ValueTypeVisitor } from './value-type';
-import { collapseArray } from '../../../util';
+import { onlyElementOrUndefined } from '../../../util';
 
 export class AtomicValueType
   extends AbstractValueType<InternalValidValueRepresentation>
@@ -88,7 +88,7 @@ export class AtomicValueType
     if (containedTypes === undefined) {
       return false;
     }
-    const containedType = collapseArray(containedTypes);
+    const containedType = onlyElementOrUndefined(containedTypes);
     if (containedType === undefined) {
       return false;
     }
@@ -138,7 +138,7 @@ export class AtomicValueType
     if (containedTypes === undefined) {
       return false;
     }
-    const containedType = collapseArray(containedTypes);
+    const containedType = onlyElementOrUndefined(containedTypes);
     if (containedType === undefined) {
       return false;
     }

libs/language-server/src/lib/ast/wrappers/value-type/value-type.ts

@@ -65,7 +65,9 @@ export interface ValueType<
   /**
    * Returns the index of the first contained type that is part of a type cycle
    */
-  typeCycleIndex(visited?: ValueType[]): number | undefined;
+  getIndexOfFirstPropertyInATypeCycle(
+    visited?: ValueType[],
+  ): number | undefined;
 
   isAllowedAsRuntimeParameter(): boolean;
   getName(): string;

libs/language-server/src/lib/util/array.ts

@@ -2,8 +2,10 @@
 //
 // SPDX-License-Identifier: AGPL-3.0-only
 
-/// If the array has exactly one element, that element is returned. Otherwise
-/// undefined
-export function collapseArray<T>(array: T[]): T | undefined {
+/**
+ * If the array has exactly one element, that element is returned. Otherwise
+ * undefined
+ */
+export function onlyElementOrUndefined<T>(array: T[]): T | undefined {
   return array.length == 1 ? array[0] : undefined;
 }