Applied review feedback

libs/execution/src/lib/blocks/block-execution-util.ts

@@ -20,12 +20,21 @@ export interface ExecutionOrderItem {
   value: IOTypeImplementation | null;
 }
 
+/**
+ * Executes an ordered list of blocks in sequence, using outputs from previous blocks as inputs for downstream blocks.
+ *
+ * @param executionContext The context the blocks are executed in, e.g., a pipeline or composite block
+ * @param executionOrder An ordered list of blocks so that blocks that need inputs are after blocks that produce these inputs
+ * @param initialInputValue An initial input that was produced outside of this block chain, e.g., as input to a composite block
+ *
+ * @returns The ordered blocks and their produced outputs or an error on failure
+ */
 export async function executeBlocks(
   executionContext: ExecutionContext,
   executionOrder: ExecutionOrderItem[],
   initialInputValue: IOTypeImplementation | undefined = undefined,
 ): Promise<R.Result<ExecutionOrderItem[]>> {
-  let isFirstblock = true;
+  let isFirstBlock = true;
 
   for (const blockData of executionOrder) {
     const block = blockData.block;
@@ -35,11 +44,10 @@ export async function executeBlocks(
     let inputValue =
       parentData[0]?.value === undefined ? NONE : parentData[0]?.value;
 
-    if (
-      isFirstblock &&
-      inputValue === NONE &&
-      initialInputValue !== undefined
-    ) {
+    const useExternalInputValueForFirstBlock =
+      isFirstBlock && inputValue === NONE && initialInputValue !== undefined;
+
+    if (useExternalInputValueForFirstBlock) {
       inputValue = initialInputValue;
     }
 
@@ -57,7 +65,7 @@ export async function executeBlocks(
     blockData.value = blockResultData;
 
     executionContext.exitNode(block);
-    isFirstblock = false;
+    isFirstBlock = false;
   }
   return R.ok(executionOrder);
 }

libs/execution/src/lib/blocks/block-executor-registry.ts

@@ -30,12 +30,11 @@ export function getRegisteredBlockExecutors(): BlockExecutorClass[] {
 }
 
 export function createBlockExecutor(block: BlockDefinition): BlockExecutor {
-  const blockType = block.type.ref?.name;
+  const blockType = block.type.ref;
   assert(blockType !== undefined);
 
   if (
-    !blockExecutorRegistry.get(blockType) &&
-    block.type.ref &&
+    !blockExecutorRegistry.get(blockType.name) &&
     isCompositeBlocktypeDefinition(block.type.ref)
   ) {
     const executorClass = createCompositeBlockExecutor(
@@ -47,11 +46,11 @@ export function createBlockExecutor(block: BlockDefinition): BlockExecutor {
     blockExecutorRegistry.register(block.type.ref.name, executorClass);
   }
 
-  const blockExecutor = blockExecutorRegistry.get(blockType);
+  const blockExecutor = blockExecutorRegistry.get(blockType.name);
 
   assert(
     blockExecutor !== undefined,
-    `No executor was registered for block type ${blockType}`,
+    `No executor was registered for block type ${blockType.name}`,
   );
 
   return new blockExecutor();

libs/execution/src/lib/blocks/composite-block-executor.ts

@@ -91,10 +91,9 @@ export function createCompositeBlockExecutor(
 
       this.removeVariablesFromContext(blockTypeReference.properties, context);
 
-      const pipeline = getPipeline(blockTypeReference);
-
       if (R.isOk(executionResult)) {
         // The last block always pipes into the output if it exists
+        const pipeline = getPipeline(blockTypeReference);
         const lastBlock = pipeline.blocks.at(-1);
 
         const blockExecutionResult = R.okData(executionResult).find(
@@ -129,7 +128,7 @@ export function createCompositeBlockExecutor(
       context: ExecutionContext,
     ) {
       properties.forEach((blocktypeProperty) => {
-        const valueType = createValuetype(blocktypeProperty.valuetype);
+        const valueType = createValuetype(blocktypeProperty.valueType);
 
         assert(
           valueType,
@@ -167,7 +166,7 @@ export function createCompositeBlockExecutor(
         (property) => property.name === name,
       );
 
-      if (propertyFromBlock) {
+      if (propertyFromBlock !== undefined) {
         const value = evaluatePropertyValue(
           propertyFromBlock,
           evaluationContext,
@@ -183,10 +182,7 @@ export function createCompositeBlockExecutor(
         (property) => property.name === name,
       );
 
-      if (
-        !propertyFromBlockType ||
-        propertyFromBlockType.defaultValue === undefined
-      ) {
+      if (propertyFromBlockType?.defaultValue === undefined) {
         return;
       }
 

libs/language-server/src/grammar/blocktype.langium

@@ -36,7 +36,7 @@ BlocktypeOutput:
     'output' name=ID 'oftype' iotype=[IotypeDefinition] ';';
 
 BlocktypeProperty:
-    'property' name=ID 'oftype' valuetype=ValuetypeReference (':' defaultValue=Expression)? ';';
+    'property' name=ID 'oftype' valueType=ValuetypeReference (':' defaultValue=Expression)? ';';
 
 BlocktypePipeline:
     input=[BlocktypeInput] '->' (blocks+=[BlockDefinition] '->')+ output=[BlocktypeOutput] ';';

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

@@ -331,9 +331,7 @@ function inferTypeFromReferenceLiteral(
     isTransformPortDefinition(referenced) ||
     isBlocktypeProperty(referenced)
   ) {
-    const valueType = isTransformPortDefinition(referenced)
-      ? referenced.valueType
-      : referenced.valuetype;
+    const valueType = referenced.valueType;
     // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
     if (valueType === undefined) {
       return undefined;

libs/language-server/src/lib/ast/io-type.ts

@@ -16,12 +16,16 @@ export enum IOType {
 }
 
 export function getIOType(blockIO: BlocktypeInput | BlocktypeOutput): IOType {
-  const ioName = blockIO.iotype.ref?.name as string;
+  const ioTypeName = blockIO.iotype.ref?.name;
+  assert(
+    ioTypeName !== undefined,
+    `Unknown IOType name for block input/output ${blockIO.name}.`,
+  );
 
   assert(
-    Object.values(IOType).some((type) => type === ioName),
-    `IOType ${ioName} does not exist.`,
+    Object.values(IOType).some((type) => type === ioTypeName),
+    `IOType ${ioTypeName} does not exist.`,
   );
 
-  return ioName as IOType;
+  return ioTypeName as IOType;
 }

libs/language-server/src/lib/ast/model-util.ts

@@ -28,15 +28,14 @@ export function collectStartingBlocks(
       .map((pipe) => pipe.blocks[0])
       .map((blockRef: Reference<BlockDefinition> | undefined) => {
         if (
-          blockRef &&
-          blockRef.ref &&
+          blockRef?.ref !== undefined &&
           getMetaInformation(blockRef.ref.type) !== undefined
         ) {
           return blockRef.ref;
         }
         return undefined;
       })
-      .filter(Boolean) as unknown as BlockDefinition[];
+      .filter((x): x is BlockDefinition => x !== undefined);
 
     return startingBlocks;
   }

libs/language-server/src/lib/meta-information/composite-blocktype-meta-inf.ts

@@ -21,7 +21,7 @@ export class CompositeBlocktypeMetaInformation extends BlockMetaInformation {
     const properties: Record<string, PropertySpecification> = {};
 
     for (const property of blockTypeDefinition.properties) {
-      const valuetype = createValuetype(property.valuetype);
+      const valuetype = createValuetype(property.valueType);
       assert(valuetype !== undefined);
 
       properties[property.name] = {
@@ -50,7 +50,7 @@ export class CompositeBlocktypeMetaInformation extends BlockMetaInformation {
         (blocktypeProperty) => blocktypeProperty.name === propertyName,
       );
 
-      return !blocktypeProperty?.defaultValue;
+      return blocktypeProperty?.defaultValue === undefined;
     });
   }
 }

libs/language-server/src/lib/validation/checks/block-definition.ts

@@ -76,13 +76,12 @@ function checkPipesOfBlock(
       );
     }
   } else if (pipes.length === 0) {
+    const isLastBlockOfCompositeBlocktype =
+      isCompositeBlocktypeDefinition(block.$container) &&
+      block.$container.blocks.at(-1)?.name === block.name;
+
     // The last block in a composite block is connected to the output, not another block
-    if (
-      !(
-        isCompositeBlocktypeDefinition(block.$container) &&
-        block.$container.blocks.at(-1)?.name === block.name
-      )
-    ) {
+    if (!isLastBlockOfCompositeBlocktype) {
       context.accept(
         'warning',
         `A pipe should be connected to the ${whatToCheck} of this block`,

libs/language-server/src/lib/validation/checks/blocktype-definition.ts

@@ -191,7 +191,7 @@ function checkPropertyDefaultValuesHasCorrectType(
   }
 
   // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-  const expectedValuetype = createValuetype(property.valuetype?.reference?.ref);
+  const expectedValuetype = createValuetype(property.valueType?.reference?.ref);
   assert(expectedValuetype !== undefined);
 
   if (!expectedValuetype.isInternalValueRepresentation(evaluatedExpression)) {

libs/language-server/src/lib/validation/checks/valuetype-reference.spec.ts

@@ -282,7 +282,7 @@ describe('Validation of ValuetypeReference', () => {
     expectNoParserAndLexerErrors(document);
     const valuetypeRef = locator.getAstNode<ValuetypeReference>(
       document.parseResult.value,
-      `blocktypes@0/properties@0/valuetype`,
+      `blocktypes@0/properties@0/valueType`,
     );
     assert(valuetypeRef !== undefined);