Merge pull request #505 from jvalue/simplify-validation-incorrect-ext…

…ractor-loader

WIP: Simplify checking for incorrect extractor and loader blocks

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

@@ -76,21 +76,6 @@ describe('Validation of BlockDefinition', () => {
     );
   });
 
-  it('should diagnose error on block as output without having an output', async () => {
-    const text = readJvTestAsset(
-      'block-definition/invalid-output-block-as-input.jv',
-    );
-
-    await parseAndValidateBlock(text);
-
-    expect(validationAcceptorMock).toHaveBeenCalledTimes(1);
-    expect(validationAcceptorMock).toHaveBeenCalledWith(
-      'error',
-      'Blocks of type TestTableLoader do not have an output',
-      expect.any(Object),
-    );
-  });
-
   it('should diagnose error on block as input for multiple pipes', async () => {
     const text = readJvTestAsset(
       'block-definition/invalid-block-as-multiple-pipe-inputs.jv',

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

@@ -40,24 +40,6 @@ function checkPipesOfBlock(
   const blockType = new BlockTypeWrapper(block?.type);
   const pipes = collectPipes(block, whatToCheck);
 
-  const isStartOrEnd =
-    (whatToCheck === 'input' && !blockType.hasInput()) ||
-    (whatToCheck === 'output' && !blockType.hasOutput());
-  if (isStartOrEnd) {
-    if (pipes.length > 0) {
-      for (const pipe of pipes) {
-        context.accept(
-          'error',
-          `Blocks of type ${blockType.type} do not have an ${whatToCheck}`,
-          whatToCheck === 'input'
-            ? pipe.getToDiagnostic()
-            : pipe.getFromDiagnostic(),
-        );
-      }
-    }
-    return;
-  }
-
   const hasMultipleInputPorts = pipes.length > 1 && whatToCheck === 'input';
   if (hasMultipleInputPorts) {
     for (const pipe of pipes) {
@@ -70,8 +52,6 @@ function checkPipesOfBlock(
     return;
   }
 
-  // above: there should be pipes (defined by blocktype)
-  // but there aren't any
   if (pipes.length === 0) {
     const isLastBlockOfCompositeBlocktype =
       isCompositeBlocktypeDefinition(block.$container) &&
@@ -81,9 +61,18 @@ function checkPipesOfBlock(
       isCompositeBlocktypeDefinition(block.$container) &&
       block.$container.blocks.at(0)?.name === block.name;
 
+    const isExtractorBlock = !blockType.hasInput() && whatToCheck === 'input';
+    const isLoaderBlock = !blockType.hasOutput() && whatToCheck === 'output';
+
     // exception: the first block in a composite block is connected to the input, not another block
     // exception: The last block in a composite block is connected to the output, not another block
-    if (!isLastBlockOfCompositeBlocktype && !isFirstBlockOfCompositeBlocktype) {
+    // exception: the extractor / loader block in a pipeline
+    if (
+      !isLastBlockOfCompositeBlocktype &&
+      !isFirstBlockOfCompositeBlocktype &&
+      !isExtractorBlock &&
+      !isLoaderBlock
+    ) {
       context.accept(
         'warning',
         `A pipe should be connected to the ${whatToCheck} of this block`,

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

@@ -91,7 +91,29 @@ describe('Validation of PipeDefinition', () => {
       expect(validationAcceptorMock).toHaveBeenNthCalledWith(
         2,
         'error',
-        `The output type "File" of TestFileExtractor is incompatible with the input type "Table" of TestTableLoader`,
+        'The output type "File" of block "TestExtractor" (of type "TestFileExtractor") is not compatible with the input type "Table" of block "TestLoader" (of type "TestTableLoader")',
+        expect.any(Object),
+      );
+    });
+
+    it('should diagnose error on connecting loader block to extractor block with a pipe', async () => {
+      const text = readJvTestAsset(
+        'pipe-definition/invalid-output-block-as-input.jv',
+      );
+
+      await parseAndValidatePipe(text);
+
+      expect(validationAcceptorMock).toHaveBeenCalledTimes(2);
+      expect(validationAcceptorMock).toHaveBeenNthCalledWith(
+        1,
+        'error',
+        'Block "BlockTo" cannot be connected to other blocks. Its blocktype "TestFileLoader" has output type "None".',
+        expect.any(Object),
+      );
+      expect(validationAcceptorMock).toHaveBeenNthCalledWith(
+        2,
+        'error',
+        'Block "BlockFrom" cannot be connected to from other blocks. Its blocktype "TestFileExtractor" has input type "None".',
         expect.any(Object),
       );
     });

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

@@ -37,12 +37,23 @@ function checkBlockCompatibility(
     const fromBlockType = new BlockTypeWrapper(fromBlockTypeDefinition);
     const toBlockType = new BlockTypeWrapper(toBlockTypeDefinition);
 
-    if (fromBlockType.hasOutput() && toBlockType.hasInput()) {
-      if (!fromBlockType.canBeConnectedTo(toBlockType)) {
-        const errorMessage = `The output type "${fromBlockType.outputType}" of ${fromBlockType.type} is incompatible with the input type "${toBlockType.inputType}" of ${toBlockType.type}`;
-        context.accept('error', errorMessage, pipeWrapper.getFromDiagnostic());
-        context.accept('error', errorMessage, pipeWrapper.getToDiagnostic());
-      }
+    const isFromBlockLoader = !fromBlockType.hasOutput();
+    const isToBlockExtractor = !toBlockType.hasInput();
+
+    if (isFromBlockLoader) {
+      const errorMessage = `Block "${pipeWrapper.from?.name}" cannot be connected to other blocks. Its blocktype "${fromBlockType.astNode.name}" has output type "${fromBlockType.outputType}".`;
+      context.accept('error', errorMessage, pipeWrapper.getFromDiagnostic());
+    }
+
+    if (isToBlockExtractor) {
+      const errorMessage = `Block "${pipeWrapper.to?.name}" cannot be connected to from other blocks. Its blocktype "${toBlockType.astNode.name}" has input type "${toBlockType.inputType}".`;
+      context.accept('error', errorMessage, pipeWrapper.getToDiagnostic());
+    }
+
+    if (!fromBlockType.canBeConnectedTo(toBlockType)) {
+      const errorMessage = `The output type "${fromBlockType.outputType}" of block "${pipeWrapper.from?.name}" (of type "${fromBlockType.astNode.name}") is not compatible with the input type "${toBlockType.inputType}" of block "${pipeWrapper.to?.name}" (of type "${toBlockType.astNode.name}")`;
+      context.accept('error', errorMessage, pipeWrapper.getFromDiagnostic());
+      context.accept('error', errorMessage, pipeWrapper.getToDiagnostic());
     }
   }
 }

libs/language-server/src/test/assets/block-definition/invalid-output-block-as-input.jv → libs/language-server/src/test/assets/pipe-definition/invalid-output-block-as-input.jv

@@ -3,14 +3,12 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 pipeline Pipeline {
-  block BlockTo oftype TestTableLoader {
+  block BlockTo oftype TestFileLoader {
   }
 
   block BlockFrom oftype TestFileExtractor {
   }
 
-  BlockFrom -> BlockTo;
-
   BlockTo -> BlockFrom;
 }
 
@@ -19,7 +17,7 @@ builtin blocktype TestFileExtractor {
   output outPort oftype File;
 }
 
-builtin blocktype TestTableLoader {
+builtin blocktype TestFileLoader {
   input inPort oftype File;
   output outPort oftype None;
 }