From 88ffa42c8b1957d06bd8fa9d1e2a6f888d44a134 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Fri, 15 Dec 2023 18:59:37 +0100 Subject: [PATCH 1/4] Remove the single pipe syntax to reduce complexity --- .../dev/04-guides/02-working-with-the-ast.md | 2 +- apps/docs/docs/user/core-concepts.md | 22 ----- example/cars.jv | 41 ++++---- .../src/grammar/pipeline.langium | 9 -- .../src/lib/ast/wrappers/pipe-wrapper.ts | 96 ++++--------------- .../validation/checks/pipe-definition.spec.ts | 49 +--------- .../invalid-pipe-between-blocktypes.jv | 0 .../single/invalid-pipe-between-blocktypes.jv | 26 ----- .../single/valid-undefined-block.jv | 10 -- .../single/valid-unknown-blocktype.jv | 16 ---- .../{chained => }/valid-undefined-block.jv | 0 .../{chained => }/valid-unknown-blocktype.jv | 0 12 files changed, 41 insertions(+), 230 deletions(-) rename libs/language-server/src/test/assets/pipe-definition/{chained => }/invalid-pipe-between-blocktypes.jv (100%) delete mode 100644 libs/language-server/src/test/assets/pipe-definition/single/invalid-pipe-between-blocktypes.jv delete mode 100644 libs/language-server/src/test/assets/pipe-definition/single/valid-undefined-block.jv delete mode 100644 libs/language-server/src/test/assets/pipe-definition/single/valid-unknown-blocktype.jv rename libs/language-server/src/test/assets/pipe-definition/{chained => }/valid-undefined-block.jv (100%) rename libs/language-server/src/test/assets/pipe-definition/{chained => }/valid-unknown-blocktype.jv (100%) diff --git a/apps/docs/docs/dev/04-guides/02-working-with-the-ast.md b/apps/docs/docs/dev/04-guides/02-working-with-the-ast.md index ca02dc335..895cae12c 100644 --- a/apps/docs/docs/dev/04-guides/02-working-with-the-ast.md +++ b/apps/docs/docs/dev/04-guides/02-working-with-the-ast.md @@ -141,7 +141,7 @@ assert(referenced !== undefined); ## AST wrapper classes The generated interfaces for AST nodes in `ast.ts` are only meant to represent the AST structurally, they don't define any behavior. -Also, in case of syntactic sugar, there may be different kinds of AST nodes representing the same semantic language concept (e.g. single pipes with a verbose syntax or chained pipes). +Also, in case of syntactic sugar, there may be different kinds of AST nodes representing the same semantic language concept. To cope with this problem, there is the concept of an `AstNodeWrapper`. An AST node wrapper is capable of wrapping AST nodes that represent the same semantic language concept and adding behavior to them via custom methods. diff --git a/apps/docs/docs/user/core-concepts.md b/apps/docs/docs/user/core-concepts.md index eba148319..add0a01f5 100644 --- a/apps/docs/docs/user/core-concepts.md +++ b/apps/docs/docs/user/core-concepts.md @@ -24,28 +24,6 @@ pipeline CarsPipeline { } ``` -Alternatively, you can use a slightly longer syntax for pipes: - -```jayvee -pipeline CarsPipeline { - // Assumption: blocks "GasReserveHttpExtractor", "GasReserveCSVInterpreter", "GasReserveTableInterpreter", and "GasReserveLoader" are defined - - pipe { - from: GasReserveHttpExtractor; - to: GasReserveTextFileInterpreter; - - } - - pipe { - from: GasReserveTextFileInterpreter; - to: GasReserveCSVInterpreter; - - } - - // etc. -} -``` - ## Blocks A `Block` is a processing step within a `Pipeline`. diff --git a/example/cars.jv b/example/cars.jv index 9ada5cb32..2100e65e7 100644 --- a/example/cars.jv +++ b/example/cars.jv @@ -16,64 +16,59 @@ pipeline CarsPipeline { // usually at the top of the pipeline. // by connecting blocks via pipes. - // 3. Verbose syntax of a pipe + // 3. Syntax of a pipe // connecting the block CarsExtractor // with the block CarsTextFileInterpreter. - pipe { - from: CarsExtractor; - to: CarsTextFileInterpreter; - } - - // 4. The output of the "from" block is hereby used - // as input for the "to" block. + CarsExtractor -> CarsTextFileInterpreter; - // 5. More convenient syntax of a pipe - CarsTextFileInterpreter -> CarsCSVInterpreter; + // 4. The output of the preceding block is hereby used + // as input for the succeeding block. - // 6. Pipes can be further chained, + // 5. Pipes can be further chained, // leading to an overview of the pipeline. - CarsCSVInterpreter + CarsTextFileInterpreter + -> CarsCSVInterpreter -> NameHeaderWriter -> CarsTableInterpreter -> CarsLoader; - // 7. Below the pipes, we usually define the blocks + // 6. Below the pipes, we usually define the blocks // that are connected by the pipes. - // 8. Blocks instantiate a blocktype by using the oftype keyword. + // 7. Blocks instantiate a blocktype by using the oftype keyword. // The blocktype defines the available properties that the block // can use to specify the intended behavior of the block block CarsExtractor oftype HttpExtractor { - // 9. Properties are assigned to concrete values. + // 8. Properties are assigned to concrete values. // Here, we specify the URL where the file shall be downloaded from. url: "https://gist.githubusercontent.com/noamross/e5d3e859aa0c794be10b/raw/b999fb4425b54c63cab088c0ce2c0d6ce961a563/cars.csv"; } - // 10. The HttpExtractor requires no input and produces a binary file as output. + // 9. The HttpExtractor requires no input and produces a binary file as output. // This file has to be interpreted, e.g., as text file. block CarsTextFileInterpreter oftype TextFileInterpreter { } - // 11. Next, we interpret the text file as sheet. + // 10. Next, we interpret the text file as sheet. // A sheet only contains text cells and is useful for manipulating the shape of data before assigning more strict value types to cells. block CarsCSVInterpreter oftype CSVInterpreter { enclosing: '"'; } - // 12. We can write into cells of a sheet using the CellWriter blocktype. + // 11. We can write into cells of a sheet using the CellWriter blocktype. block NameHeaderWriter oftype CellWriter { - // 13. We utilize a syntax similar to spreadsheet programs. + // 12. We utilize a syntax similar to spreadsheet programs. // Cell ranges can be described using the keywords "cell", "row", "column", or "range" that indicate which // cells are selected for the write action. at: cell A1; - // 14. For each cell we selected with the "at" property above, + // 13. For each cell we selected with the "at" property above, // we can specify what value shall be written into the cell. write: ["name"]; } - // 15. As a next step, we interpret the sheet as a table by adding structure. + // 14. As a next step, we interpret the sheet as a table by adding structure. // We define a valuetype per column that specifies the data type of the column. // Rows that include values that are not valid according to the their valuetypes are dropped automatically. block CarsTableInterpreter oftype TableInterpreter { @@ -94,7 +89,7 @@ pipeline CarsPipeline { ]; } - // 16. As a last step, we load the table into a sink, + // 15. As a last step, we load the table into a sink, // here into a sqlite file. // The structural information of the table is used // to generate the correct table. @@ -103,7 +98,7 @@ pipeline CarsPipeline { file: "./cars.sqlite"; } - // 17. Congratulations! + // 16. Congratulations! // You can now use the sink for your data analysis, app, // or whatever you want to do with the cleaned data. } \ No newline at end of file diff --git a/libs/language-server/src/grammar/pipeline.langium b/libs/language-server/src/grammar/pipeline.langium index 609b286cd..8e6a3e3d8 100644 --- a/libs/language-server/src/grammar/pipeline.langium +++ b/libs/language-server/src/grammar/pipeline.langium @@ -20,13 +20,4 @@ PipelineDefinition: '}'; PipeDefinition: - SinglePipeDefinition | ChainedPipeDefinition; - -SinglePipeDefinition: - 'pipe' '{' - 'from' ':' from=[BlockDefinition] ';' - 'to' ':' to=[BlockDefinition] ';' - '}'; - -ChainedPipeDefinition: blocks+=[BlockDefinition] ('->' blocks+=[BlockDefinition])+ ';'; \ No newline at end of file diff --git a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts index b69d6c643..4162cdf0f 100644 --- a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts +++ b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts @@ -9,59 +9,33 @@ import { DiagnosticInfo } from 'langium'; import { BlockDefinition, BlocktypePipeline, - ChainedPipeDefinition, PipeDefinition, - SinglePipeDefinition, - isSinglePipeDefinition, } from '../generated/ast'; import { AstNodeWrapper } from './ast-node-wrapper'; -export class PipeWrapper - implements AstNodeWrapper +export class PipeWrapper + implements AstNodeWrapper { - public readonly astNode: N; - private readonly chainIndex?: number; + public readonly astNode: PipeDefinition | BlocktypePipeline; + private readonly chainIndex: number; public readonly from: BlockDefinition; public readonly to: BlockDefinition; - constructor( - pipe: ChainedPipeDefinition | BlocktypePipeline, - chainIndex: number, - ); - constructor(pipe: SinglePipeDefinition); - constructor(pipe: N, chainIndex?: number) { + constructor(pipe: BlocktypePipeline | PipeDefinition, chainIndex: number) { this.astNode = pipe; - if (isSinglePipeDefinition(pipe)) { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - assert(pipe.from?.ref !== undefined); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - assert(pipe.to?.ref !== undefined); - this.from = pipe.from.ref; - this.to = pipe.to.ref; - } else { - assert(chainIndex !== undefined); - assert(0 <= chainIndex && chainIndex + 1 < pipe.blocks.length); - assert(pipe.blocks[chainIndex]?.ref !== undefined); - assert(pipe.blocks[chainIndex + 1]?.ref !== undefined); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.from = pipe.blocks[chainIndex]!.ref!; - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.to = pipe.blocks[chainIndex + 1]!.ref!; - this.chainIndex = chainIndex; - } + assert(0 <= chainIndex && chainIndex + 1 < pipe.blocks.length); + assert(pipe.blocks[chainIndex]?.ref !== undefined); + assert(pipe.blocks[chainIndex + 1]?.ref !== undefined); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.from = pipe.blocks[chainIndex]!.ref!; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.to = pipe.blocks[chainIndex + 1]!.ref!; + this.chainIndex = chainIndex; } - getFromDiagnostic(): DiagnosticInfo { - if (isSinglePipeDefinition(this.astNode)) { - const result: DiagnosticInfo = { - node: this.astNode, - property: 'from', - }; - return result; - } - assert(this.chainIndex !== undefined); - const result: DiagnosticInfo = { + getFromDiagnostic(): DiagnosticInfo { + const result: DiagnosticInfo = { node: this.astNode, property: 'blocks', index: this.chainIndex, @@ -69,16 +43,8 @@ export class PipeWrapper return result; } - getToDiagnostic(): DiagnosticInfo { - if (isSinglePipeDefinition(this.astNode)) { - const result: DiagnosticInfo = { - node: this.astNode, - property: 'to', - }; - return result; - } - assert(this.chainIndex !== undefined); - const result: DiagnosticInfo = { + getToDiagnostic(): DiagnosticInfo { + const result: DiagnosticInfo = { node: this.astNode, property: 'blocks', index: this.chainIndex + 1, @@ -91,20 +57,10 @@ export class PipeWrapper } static canBeWrapped( - pipe: ChainedPipeDefinition | BlocktypePipeline, + pipe: PipeDefinition | BlocktypePipeline, chainIndex: number, - ): boolean; - static canBeWrapped(pipe: SinglePipeDefinition): boolean; - static canBeWrapped( - pipe: N, - chainIndex?: number, ): boolean { - if (isSinglePipeDefinition(pipe)) { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - return pipe.from?.ref !== undefined && pipe.to?.ref !== undefined; - } return ( - chainIndex !== undefined && 0 <= chainIndex && chainIndex + 1 < pipe.blocks.length && pipe.blocks[chainIndex]?.ref !== undefined && @@ -115,22 +71,6 @@ export class PipeWrapper export function createSemanticPipes( pipe: PipeDefinition | BlocktypePipeline, -): PipeWrapper[] { - if (isSinglePipeDefinition(pipe)) { - return createFromSinglePipe(pipe); - } - return createFromChainedPipe(pipe); -} - -function createFromSinglePipe(pipe: SinglePipeDefinition): PipeWrapper[] { - if (PipeWrapper.canBeWrapped(pipe)) { - return [new PipeWrapper(pipe)]; - } - return []; -} - -function createFromChainedPipe( - pipe: ChainedPipeDefinition | BlocktypePipeline, ): PipeWrapper[] { const result: PipeWrapper[] = []; for (let chainIndex = 0; chainIndex < pipe.blocks.length - 1; ++chainIndex) { diff --git a/libs/language-server/src/lib/validation/checks/pipe-definition.spec.ts b/libs/language-server/src/lib/validation/checks/pipe-definition.spec.ts index 9aa8ff203..fb6c43a9a 100644 --- a/libs/language-server/src/lib/validation/checks/pipe-definition.spec.ts +++ b/libs/language-server/src/lib/validation/checks/pipe-definition.spec.ts @@ -60,51 +60,10 @@ describe('Validation of PipeDefinition', () => { validationAcceptorMock.mockReset(); }); - describe('SinglePipeDefinition syntax', () => { + describe('PipeDefinition syntax', () => { // This test should succeed, because the error is thrown by langium during linking, not during validation! it('should have no error even if pipe references non existing block', async () => { - const text = readJvTestAsset( - 'pipe-definition/single/valid-undefined-block.jv', - ); - - await parseAndValidatePipe(text); - - expect(validationAcceptorMock).toHaveBeenCalledTimes(0); - }); - - it('should have no error even if pipe references block of non existing type', async () => { - const text = readJvTestAsset( - 'pipe-definition/single/valid-unknown-blocktype.jv', - ); - - await parseAndValidatePipe(text); - - expect(validationAcceptorMock).toHaveBeenCalledTimes(0); - }); - - it('should diagnose error on unsupported pipe between Blocktypes', async () => { - const text = readJvTestAsset( - 'pipe-definition/single/invalid-pipe-between-blocktypes.jv', - ); - - await parseAndValidatePipe(text); - - expect(validationAcceptorMock).toHaveBeenCalledTimes(2); - expect(validationAcceptorMock).toHaveBeenNthCalledWith( - 2, - 'error', - `The output type "File" of TestFileExtractor is incompatible with the input type "Table" of TestTableLoader`, - expect.any(Object), - ); - }); - }); - - describe('ChainedPipeDefinition syntax', () => { - // This test should succeed, because the error is thrown by langium during linking, not during validation! - it('should have no error even if pipe references non existing block', async () => { - const text = readJvTestAsset( - 'pipe-definition/chained/valid-undefined-block.jv', - ); + const text = readJvTestAsset('pipe-definition/valid-undefined-block.jv'); await parseAndValidatePipe(text); @@ -113,7 +72,7 @@ describe('Validation of PipeDefinition', () => { it('should have no error even if pipe references block of non existing type', async () => { const text = readJvTestAsset( - 'pipe-definition/chained/valid-unknown-blocktype.jv', + 'pipe-definition/valid-unknown-blocktype.jv', ); await parseAndValidatePipe(text); @@ -123,7 +82,7 @@ describe('Validation of PipeDefinition', () => { it('should diagnose error on unsupported pipe between Blocktypes', async () => { const text = readJvTestAsset( - 'pipe-definition/chained/invalid-pipe-between-blocktypes.jv', + 'pipe-definition/invalid-pipe-between-blocktypes.jv', ); await parseAndValidatePipe(text); diff --git a/libs/language-server/src/test/assets/pipe-definition/chained/invalid-pipe-between-blocktypes.jv b/libs/language-server/src/test/assets/pipe-definition/invalid-pipe-between-blocktypes.jv similarity index 100% rename from libs/language-server/src/test/assets/pipe-definition/chained/invalid-pipe-between-blocktypes.jv rename to libs/language-server/src/test/assets/pipe-definition/invalid-pipe-between-blocktypes.jv diff --git a/libs/language-server/src/test/assets/pipe-definition/single/invalid-pipe-between-blocktypes.jv b/libs/language-server/src/test/assets/pipe-definition/single/invalid-pipe-between-blocktypes.jv deleted file mode 100644 index 2494fb23a..000000000 --- a/libs/language-server/src/test/assets/pipe-definition/single/invalid-pipe-between-blocktypes.jv +++ /dev/null @@ -1,26 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg -// -// SPDX-License-Identifier: AGPL-3.0-only - -pipeline Pipeline { - block TestExtractor oftype TestFileExtractor { - } - - block TestLoader oftype TestTableLoader { - } - - pipe { - from: TestExtractor; - to: TestLoader; - } -} - -builtin blocktype TestFileExtractor { - input inPort oftype None; - output outPort oftype File; -} - -builtin blocktype TestTableLoader { - input inPort oftype Table; - output outPort oftype None; -} \ No newline at end of file diff --git a/libs/language-server/src/test/assets/pipe-definition/single/valid-undefined-block.jv b/libs/language-server/src/test/assets/pipe-definition/single/valid-undefined-block.jv deleted file mode 100644 index 56bf4b50d..000000000 --- a/libs/language-server/src/test/assets/pipe-definition/single/valid-undefined-block.jv +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg -// -// SPDX-License-Identifier: AGPL-3.0-only - -pipeline Pipeline { - pipe { - from: TestExtractor; - to: TestLoader; - } -} diff --git a/libs/language-server/src/test/assets/pipe-definition/single/valid-unknown-blocktype.jv b/libs/language-server/src/test/assets/pipe-definition/single/valid-unknown-blocktype.jv deleted file mode 100644 index 786638dec..000000000 --- a/libs/language-server/src/test/assets/pipe-definition/single/valid-unknown-blocktype.jv +++ /dev/null @@ -1,16 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg -// -// SPDX-License-Identifier: AGPL-3.0-only - -pipeline Pipeline { - block UnknownOutput oftype UnknownOutputType { - } - - block UnknownInput oftype UnknownInputType { - } - - pipe { - from: UnknownOutput; - to: UnknownInput; - } -} diff --git a/libs/language-server/src/test/assets/pipe-definition/chained/valid-undefined-block.jv b/libs/language-server/src/test/assets/pipe-definition/valid-undefined-block.jv similarity index 100% rename from libs/language-server/src/test/assets/pipe-definition/chained/valid-undefined-block.jv rename to libs/language-server/src/test/assets/pipe-definition/valid-undefined-block.jv diff --git a/libs/language-server/src/test/assets/pipe-definition/chained/valid-unknown-blocktype.jv b/libs/language-server/src/test/assets/pipe-definition/valid-unknown-blocktype.jv similarity index 100% rename from libs/language-server/src/test/assets/pipe-definition/chained/valid-unknown-blocktype.jv rename to libs/language-server/src/test/assets/pipe-definition/valid-unknown-blocktype.jv From 155b3f696bacde7e865d71c265d23f2d8cbaca1a Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Fri, 15 Dec 2023 19:13:52 +0100 Subject: [PATCH 2/4] Refactor naming of semantic pipes to pipe wrappers --- libs/language-server/src/lib/ast/model-util.ts | 13 ++++++++----- .../src/lib/ast/wrappers/pipe-wrapper.ts | 6 +++--- .../src/lib/validation/checks/pipe-definition.ts | 14 +++++++------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/libs/language-server/src/lib/ast/model-util.ts b/libs/language-server/src/lib/ast/model-util.ts index 25606bc9b..06bfcd544 100644 --- a/libs/language-server/src/lib/ast/model-util.ts +++ b/libs/language-server/src/lib/ast/model-util.ts @@ -25,7 +25,10 @@ import { } from './generated/ast'; // eslint-disable-next-line import/no-cycle import { BlockTypeWrapper, ConstraintTypeWrapper } from './wrappers'; -import { PipeWrapper, createSemanticPipes } from './wrappers/pipe-wrapper'; +import { + PipeWrapper, + createWrappersFromPipeChain, +} from './wrappers/pipe-wrapper'; export function collectStartingBlocks( container: PipelineDefinition | CompositeBlocktypeDefinition, @@ -90,12 +93,12 @@ function collectPipes( const pipeline = block.$container; const allPipes = collectAllPipes(pipeline); - return allPipes.filter((semanticPipe) => { + return allPipes.filter((pipeWrapper) => { switch (kind) { case 'outgoing': - return semanticPipe.from === block; + return pipeWrapper.from === block; case 'ingoing': - return semanticPipe.to === block; + return pipeWrapper.to === block; case undefined: return false; } @@ -108,7 +111,7 @@ export function collectAllPipes( ): PipeWrapper[] { const result: PipeWrapper[] = []; for (const pipe of container.pipes) { - result.push(...createSemanticPipes(pipe)); + result.push(...createWrappersFromPipeChain(pipe)); } return result; } diff --git a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts index 4162cdf0f..7ec9539b6 100644 --- a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts +++ b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts @@ -69,7 +69,7 @@ export class PipeWrapper } } -export function createSemanticPipes( +export function createWrappersFromPipeChain( pipe: PipeDefinition | BlocktypePipeline, ): PipeWrapper[] { const result: PipeWrapper[] = []; @@ -77,8 +77,8 @@ export function createSemanticPipes( if (!PipeWrapper.canBeWrapped(pipe, chainIndex)) { continue; } - const semanticPipe = new PipeWrapper(pipe, chainIndex); - result.push(semanticPipe); + const pipeWrapper = new PipeWrapper(pipe, chainIndex); + result.push(pipeWrapper); } return result; } diff --git a/libs/language-server/src/lib/validation/checks/pipe-definition.ts b/libs/language-server/src/lib/validation/checks/pipe-definition.ts index fbb24e5d5..76f24acbe 100644 --- a/libs/language-server/src/lib/validation/checks/pipe-definition.ts +++ b/libs/language-server/src/lib/validation/checks/pipe-definition.ts @@ -8,7 +8,7 @@ /* eslint-disable @typescript-eslint/no-unnecessary-condition */ import { PipeDefinition } from '../../ast/generated/ast'; -import { createSemanticPipes } from '../../ast/wrappers/pipe-wrapper'; +import { createWrappersFromPipeChain } from '../../ast/wrappers/pipe-wrapper'; import { BlockTypeWrapper } from '../../ast/wrappers/typed-object/blocktype-wrapper'; import { ValidationContext } from '../validation-context'; @@ -23,10 +23,10 @@ function checkBlockCompatibility( pipe: PipeDefinition, context: ValidationContext, ): void { - const semanticPipes = createSemanticPipes(pipe); - for (const semanticPipe of semanticPipes) { - const fromBlockTypeDefinition = semanticPipe.from?.type; - const toBlockTypeDefinition = semanticPipe.to?.type; + const pipeWrappers = createWrappersFromPipeChain(pipe); + for (const pipeWrapper of pipeWrappers) { + const fromBlockTypeDefinition = pipeWrapper.from?.type; + const toBlockTypeDefinition = pipeWrapper.to?.type; if ( !BlockTypeWrapper.canBeWrapped(fromBlockTypeDefinition) || @@ -40,8 +40,8 @@ function checkBlockCompatibility( 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, semanticPipe.getFromDiagnostic()); - context.accept('error', errorMessage, semanticPipe.getToDiagnostic()); + context.accept('error', errorMessage, pipeWrapper.getFromDiagnostic()); + context.accept('error', errorMessage, pipeWrapper.getToDiagnostic()); } } } From a4018b4e84ca0cf6a542676e7d3004a40aafa0dc Mon Sep 17 00:00:00 2001 From: joluj Date: Mon, 18 Dec 2023 13:01:04 +0100 Subject: [PATCH 3/4] Refactor PipeWrapper to make use of generics --- .../src/lib/ast/wrappers/pipe-wrapper.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts index 7ec9539b6..244b9d821 100644 --- a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts +++ b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts @@ -14,15 +14,15 @@ import { import { AstNodeWrapper } from './ast-node-wrapper'; -export class PipeWrapper - implements AstNodeWrapper +export class PipeWrapper + implements AstNodeWrapper { - public readonly astNode: PipeDefinition | BlocktypePipeline; + public readonly astNode: T; private readonly chainIndex: number; public readonly from: BlockDefinition; public readonly to: BlockDefinition; - constructor(pipe: BlocktypePipeline | PipeDefinition, chainIndex: number) { + constructor(pipe: T, chainIndex: number) { this.astNode = pipe; assert(0 <= chainIndex && chainIndex + 1 < pipe.blocks.length); assert(pipe.blocks[chainIndex]?.ref !== undefined); @@ -34,25 +34,23 @@ export class PipeWrapper this.chainIndex = chainIndex; } - getFromDiagnostic(): DiagnosticInfo { - const result: DiagnosticInfo = { + getFromDiagnostic(): DiagnosticInfo { + return { node: this.astNode, property: 'blocks', index: this.chainIndex, }; - return result; } - getToDiagnostic(): DiagnosticInfo { - const result: DiagnosticInfo = { + getToDiagnostic(): DiagnosticInfo { + return { node: this.astNode, property: 'blocks', index: this.chainIndex + 1, }; - return result; } - equals(pipe: PipeWrapper): boolean { + equals(pipe: PipeWrapper): boolean { return this.from === pipe.from && this.to === pipe.to; } @@ -69,10 +67,10 @@ export class PipeWrapper } } -export function createWrappersFromPipeChain( - pipe: PipeDefinition | BlocktypePipeline, -): PipeWrapper[] { - const result: PipeWrapper[] = []; +export function createWrappersFromPipeChain< + A extends PipeDefinition | BlocktypePipeline, +>(pipe: A): PipeWrapper[] { + const result: PipeWrapper[] = []; for (let chainIndex = 0; chainIndex < pipe.blocks.length - 1; ++chainIndex) { if (!PipeWrapper.canBeWrapped(pipe, chainIndex)) { continue; From f8e20230902149a6f86eba98ec2de289b46b27bf Mon Sep 17 00:00:00 2001 From: joluj Date: Mon, 18 Dec 2023 13:34:05 +0100 Subject: [PATCH 4/4] Add default to generic --- .../src/lib/ast/wrappers/pipe-wrapper.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts index 244b9d821..70715eb43 100644 --- a/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts +++ b/libs/language-server/src/lib/ast/wrappers/pipe-wrapper.ts @@ -14,8 +14,11 @@ import { import { AstNodeWrapper } from './ast-node-wrapper'; -export class PipeWrapper - implements AstNodeWrapper +export class PipeWrapper< + T extends PipeDefinition | BlocktypePipeline = + | PipeDefinition + | BlocktypePipeline, +> implements AstNodeWrapper { public readonly astNode: T; private readonly chainIndex: number; @@ -34,7 +37,7 @@ export class PipeWrapper this.chainIndex = chainIndex; } - getFromDiagnostic(): DiagnosticInfo { + getFromDiagnostic(): DiagnosticInfo { return { node: this.astNode, property: 'blocks', @@ -42,7 +45,7 @@ export class PipeWrapper }; } - getToDiagnostic(): DiagnosticInfo { + getToDiagnostic(): DiagnosticInfo { return { node: this.astNode, property: 'blocks',