ljmotta commented on code in PR #2490: URL: https://github.com/apache/incubator-kie-tools/pull/2490#discussion_r1696728113
########## packages/feel-input-component/tests/semanticTokensProvider.test.ts: ########## @@ -0,0 +1,192 @@ +/* + * Copyright 2024 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { SemanticTokensProvider } from "../src/semanticTokensProvider"; +import { FeelVariables, DmnDefinitions } from "@kie-tools/dmn-feel-antlr4-parser"; + +import * as Monaco from "@kie-tools-core/monaco-editor"; + +describe("Semantic Tokens Provider", () => { + const cancellationTokenMock = { + isCancellationRequested: false, + onCancellationRequested: jest.fn().mockImplementation(), + }; + + const knownVariable = + "This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight"; + + /** + * The 'parsedTokens' are the tokens that parser should found in the provided 'expression'. + * The 'expected' are the Monaco Semantic Tokens that we are expecting to pass to Monaco to paint it on the screen. + * + * Each Monaco Semantic Tokens is an array of 5 positions + * 0 = The start line of the token RELATIVE TO THE PREVIOUS LINE + * 1 = The start index of the token relative to the START of the previous token + * 2 = The length of the token + * 3 = The type of the token (GlobalVariable, Unknown, Function Parameter, etc.). It determines the color of the token + * 4 = Token modifier. It's always zero since we don't have this feature. + */ + test.each([ + { + expression: + 'This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight + "bar"', + expected: [[0, 0, 102, 5, 0]], + }, + { + expression: `This is a variable with a very long +name to reproduce the issue thousand +one hundred and seventy-eight + "bar"`, + expected: [ + [0, 0, 36, 5, 0], + [1, 0, 37, 5, 0], + [1, 0, 29, 5, 0], + ], + }, + { + expression: `"aaaaaa" + +This is a variable with a very +long name to + reproduce the issue thousand + one hundred and seventy-eight + "bar" + "NICE" + This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight`, + expected: [ + [1, 0, 31, 5, 0], + [1, 0, 12, 5, 0], + [1, 0, 30, 5, 0], + [1, 0, 30, 5, 0], + [0, 50, 102, 5, 0], + ], + }, + { + expression: `This is a variable with a very long name to +reproduce +the issue +thousand +one hundred +and +seventy-eight + "bar`, + expected: [ + [0, 0, 44, 5, 0], + [1, 0, 10, 5, 0], + [1, 0, 10, 5, 0], + [1, 0, 9, 5, 0], + [1, 0, 12, 5, 0], + [1, 0, 4, 5, 0], + [1, 0, 13, 5, 0], + ], + }, + { + expression: `"My " + This is a variable with a very long name to reproduce + the issue thousand one hundred and seventy-eight + "bar"`, + expected: [ + [0, 8, 89, 5, 0], + [1, 0, 104, 5, 0], + ], + }, + { + expression: `This is a variable with a very long name to +reproduce the issue thousand one hundred and seventy-eight + "bar"`, + expected: [ + [0, 0, 43, 5, 0], + [1, 0, 58, 5, 0], + ], + }, + { + expression: `VeryLongVariableWithoutSpaces +ThatShouldFailWhenBreakLine`, + expected: [ + [0, 0, 29, 7, 0], + [1, 0, 27, 7, 0], + ], + }, + ])("multiline variables", async ({ expression, expected }) => { + const modelMock = { + getValue: jest.fn().mockReturnValue(expression), + getLinesContent: jest.fn().mockReturnValue(expression.split("\n")), + }; + + const id = "expressionId"; + const dmnDefinitions = getDmnModel({ knownVariable: knownVariable, expressionId: id, expression: expression }); + + const feelVariables = new FeelVariables(dmnDefinitions, new Map()); + const semanticTokensProvider = new SemanticTokensProvider(feelVariables, id, () => {}); + + const semanticMonacoTokens = await semanticTokensProvider.provideDocumentSemanticTokens( + modelMock as unknown as Monaco.editor.ITextModel, + null, + cancellationTokenMock + ); + + const expectedSemanticMonacoTokens = expected.reduce((accumulator, value) => accumulator.concat(value), []); + + for (let i = 0; i < expectedSemanticMonacoTokens.length; i++) { + expect(semanticMonacoTokens?.data[i]).toEqual(expectedSemanticMonacoTokens[i]); + } + }); +}); + +function getDmnModel({ + knownVariable, + expressionId, + expression, +}: { + knownVariable: string; + expressionId: string; + expression: string; +}) { Review Comment: Overall I don't think the name and signature tells what this function do. It will get a DMN model with a context expression with one known entry (variable + expression). Also, you could add the return type to the function removing the necessity of creating a varible. ########## packages/feel-input-component/tests/semanticTokensProvider.test.ts: ########## @@ -0,0 +1,192 @@ +/* + * Copyright 2024 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { SemanticTokensProvider } from "../src/semanticTokensProvider"; +import { FeelVariables, DmnDefinitions } from "@kie-tools/dmn-feel-antlr4-parser"; + +import * as Monaco from "@kie-tools-core/monaco-editor"; + +describe("Semantic Tokens Provider", () => { + const cancellationTokenMock = { + isCancellationRequested: false, + onCancellationRequested: jest.fn().mockImplementation(), + }; + + const knownVariable = + "This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight"; + + /** + * The 'parsedTokens' are the tokens that parser should found in the provided 'expression'. + * The 'expected' are the Monaco Semantic Tokens that we are expecting to pass to Monaco to paint it on the screen. + * + * Each Monaco Semantic Tokens is an array of 5 positions + * 0 = The start line of the token RELATIVE TO THE PREVIOUS LINE + * 1 = The start index of the token relative to the START of the previous token + * 2 = The length of the token + * 3 = The type of the token (GlobalVariable, Unknown, Function Parameter, etc.). It determines the color of the token + * 4 = Token modifier. It's always zero since we don't have this feature. + */ + test.each([ Review Comment: Maybe this and tests should be inside another test.describe block? I mean, the variable is specific to a very long variable name. ########## packages/feel-input-component/tests/semanticTokensProvider.test.ts: ########## @@ -0,0 +1,192 @@ +/* + * Copyright 2024 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { SemanticTokensProvider } from "../src/semanticTokensProvider"; +import { FeelVariables, DmnDefinitions } from "@kie-tools/dmn-feel-antlr4-parser"; + +import * as Monaco from "@kie-tools-core/monaco-editor"; + +describe("Semantic Tokens Provider", () => { + const cancellationTokenMock = { + isCancellationRequested: false, + onCancellationRequested: jest.fn().mockImplementation(), + }; + + const knownVariable = + "This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight"; + + /** + * The 'parsedTokens' are the tokens that parser should found in the provided 'expression'. + * The 'expected' are the Monaco Semantic Tokens that we are expecting to pass to Monaco to paint it on the screen. + * + * Each Monaco Semantic Tokens is an array of 5 positions + * 0 = The start line of the token RELATIVE TO THE PREVIOUS LINE + * 1 = The start index of the token relative to the START of the previous token + * 2 = The length of the token + * 3 = The type of the token (GlobalVariable, Unknown, Function Parameter, etc.). It determines the color of the token + * 4 = Token modifier. It's always zero since we don't have this feature. + */ + test.each([ + { + expression: + 'This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight + "bar"', + expected: [[0, 0, 102, 5, 0]], + }, + { + expression: `This is a variable with a very long +name to reproduce the issue thousand +one hundred and seventy-eight + "bar"`, + expected: [ + [0, 0, 36, 5, 0], + [1, 0, 37, 5, 0], + [1, 0, 29, 5, 0], + ], + }, + { + expression: `"aaaaaa" + +This is a variable with a very +long name to + reproduce the issue thousand + one hundred and seventy-eight + "bar" + "NICE" + This is a variable with a very long name to reproduce the issue thousand one hundred and seventy-eight`, + expected: [ + [1, 0, 31, 5, 0], + [1, 0, 12, 5, 0], + [1, 0, 30, 5, 0], + [1, 0, 30, 5, 0], + [0, 50, 102, 5, 0], + ], + }, + { + expression: `This is a variable with a very long name to +reproduce +the issue +thousand +one hundred +and +seventy-eight + "bar`, + expected: [ + [0, 0, 44, 5, 0], + [1, 0, 10, 5, 0], + [1, 0, 10, 5, 0], + [1, 0, 9, 5, 0], + [1, 0, 12, 5, 0], + [1, 0, 4, 5, 0], + [1, 0, 13, 5, 0], + ], + }, + { + expression: `"My " + This is a variable with a very long name to reproduce + the issue thousand one hundred and seventy-eight + "bar"`, + expected: [ + [0, 8, 89, 5, 0], + [1, 0, 104, 5, 0], + ], + }, + { + expression: `This is a variable with a very long name to +reproduce the issue thousand one hundred and seventy-eight + "bar"`, + expected: [ + [0, 0, 43, 5, 0], + [1, 0, 58, 5, 0], + ], + }, + { + expression: `VeryLongVariableWithoutSpaces +ThatShouldFailWhenBreakLine`, + expected: [ + [0, 0, 29, 7, 0], + [1, 0, 27, 7, 0], + ], + }, + ])("multiline variables", async ({ expression, expected }) => { + const modelMock = { + getValue: jest.fn().mockReturnValue(expression), + getLinesContent: jest.fn().mockReturnValue(expression.split("\n")), + }; + + const id = "expressionId"; + const dmnDefinitions = getDmnModel({ knownVariable: knownVariable, expressionId: id, expression: expression }); + + const feelVariables = new FeelVariables(dmnDefinitions, new Map()); + const semanticTokensProvider = new SemanticTokensProvider(feelVariables, id, () => {}); + + const semanticMonacoTokens = await semanticTokensProvider.provideDocumentSemanticTokens( + modelMock as unknown as Monaco.editor.ITextModel, + null, + cancellationTokenMock + ); + + const expectedSemanticMonacoTokens = expected.reduce((accumulator, value) => accumulator.concat(value), []); Review Comment: You could use `expected.flat()` to archieve the same behavior. ########## packages/feel-input-component/tests/semanticTokensProvider.test.ts: ########## @@ -0,0 +1,192 @@ +/* + * Copyright 2024 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ Review Comment: Shouldn't it be an Apache header? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
