From 4d600bf3d8edf147b898c309504a689b0fad6f45 Mon Sep 17 00:00:00 2001 From: SimonCockx <47859223+SimonCockx@users.noreply.github.com> Date: Tue, 28 Jan 2025 18:19:33 +0100 Subject: [PATCH] Label change detection fix (#910) * Label change detection fix * Cleaned --- rosetta-ide/pom.xml | 4 + .../ide/server/ChangeDetectionTest.java | 228 ++++++++++++++++++ .../ide/server/ChangeDetectionTest.xtend | 169 ------------- .../AbstractRosettaLanguageServerTest.xtend | 12 +- .../resource/AttributeDescription.java | 10 +- .../RosettaResourceDescriptionStrategy.java | 15 +- 6 files changed, 262 insertions(+), 176 deletions(-) create mode 100644 rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.java delete mode 100644 rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.xtend diff --git a/rosetta-ide/pom.xml b/rosetta-ide/pom.xml index 3c1d4709a..f8109dd3c 100644 --- a/rosetta-ide/pom.xml +++ b/rosetta-ide/pom.xml @@ -33,6 +33,10 @@ - language server features (semantic highlighting, inlay hints, etc) - syntax highlighting + + + 17 + diff --git a/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.java b/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.java new file mode 100644 index 000000000..eed35a2c9 --- /dev/null +++ b/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.java @@ -0,0 +1,228 @@ +package com.regnosys.rosetta.ide.server; + +import org.eclipse.lsp4j.Diagnostic; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import com.regnosys.rosetta.ide.tests.AbstractRosettaLanguageServerValidationTest; + +import java.util.List; + +public class ChangeDetectionTest extends AbstractRosettaLanguageServerValidationTest { + @Test + void testChangeInAttributeTypeIsPropagated() { + String typesURI = createModel("types.rosetta", """ + namespace test + + type A: + attr int (1..1) + """); + String funcsURI = createModel("funcs.rosetta", """ + namespace test + + func Foo: + inputs: input A (1..1) + output: result int (1..1) + + set result: input -> attr + """); + + // Initial: there should be no issue. + assertNoIssues(); + + // Introduce a type error by changing the type of `attr` from `int` to `string`. + makeChange(typesURI, 3, 6, "int", "string"); + + // There should be a type error in func `Foo` + List issues = getDiagnostics().get(funcsURI); + Assertions.assertEquals(1, issues.size()); + Assertions.assertEquals( + "Expected type `int`, but got `string` instead. Cannot assign `string` to output `result`", + issues.get(0).getMessage()); + } + + @Test + void testChangeInAttributeCardinalityIsPropagated() { + String typesURI = createModel("types.rosetta", """ + namespace test + + type A: + attr int (1..1) + """); + String funcsURI = createModel("funcs.rosetta", """ + namespace test + + func Foo: + inputs: input A (1..1) + output: result int (1..1) + + set result: input -> attr + """); + + // Initial: there should be no issue. + assertNoIssues(); + + // Introduce an error by changing the cardinality of `attr` from `(1..1)` to + // `(0..*)`. + makeChange(typesURI, 3, 10, "(1..1)", "(0..*)"); + + // There should be a cardinality error in func `Foo` + List issues = getDiagnostics().get(funcsURI); + Assertions.assertEquals(1, issues.size()); + Assertions.assertEquals("Expecting single cardinality. Cannot assign a list to a single value", + issues.get(0).getMessage()); + } + + @Test + void testChangeInAttributeQualifiedTypeIsPropagated() { + createModel("foo.rosetta", """ + namespace foo + + type MyType: + """); + createModel("bar.rosetta", """ + namespace bar + + type MyType: + """); + String typesURI = createModel("types.rosetta", """ + namespace test + + import foo.MyType + + type A: + attr MyType (1..1) + """); + String funcsURI = createModel("funcs.rosetta", """ + namespace test + + import foo.MyType + + func Foo: + inputs: input A (1..1) + output: result MyType (1..1) + + set result: input -> attr + """); + + // Initial: there should be no issue. + assertNoIssues(); + + // Introduce a type error by changing the type of `attr` from `foo.MyType` to + // `bar.MyType`. + // We do this by changing `import foo.MyType` to `import bar.MyType`. + makeChange(typesURI, 2, 7, "foo", "bar"); + + // There should be a type error in func `Foo` + List issues = getDiagnostics().get(funcsURI); + Assertions.assertEquals(1, issues.size()); + Assertions.assertEquals( + "Expected type `foo.MyType`, but got `bar.MyType` instead. Cannot assign `bar.MyType` to output `result`", + issues.get(0).getMessage()); + } + + @Test + void testChangeInRuleInputTypeIsPropagated() { + String ruleAURI = createModel("ruleA.rosetta", """ + namespace test + + reporting rule A from string: + 42 + """); + String ruleBURI = createModel("ruleB.rosetta", """ + namespace test + + reporting rule B from string: + A + """); + + // Initial: there should be no issue. + assertNoIssues(); + + // Introduce a type error by changing the input type of rule `A` to `int`. + makeChange(ruleAURI, 2, 22, "string", "int"); + + // There should be a type error in rule B + List issues = getDiagnostics().get(ruleBURI); + Assertions.assertEquals(1, issues.size()); + Assertions.assertEquals( + "Expected type `int`, but got `string` instead. Rule `A` cannot be called with type `string`", + issues.get(0).getMessage()); + } + + @Test + void testChangeInRuleExpressionIsPropagated() { + String ruleAURI = createModel("ruleA.rosetta", """ + namespace test + + reporting rule A from string: + 42 + """); + String funcURI = createModel("func.rosetta", """ + namespace test + + func Foo: + output: + result int (1..1) + set result: + A("") + """); + + // Initial: there should be no issue. + assertNoIssues(); + + // Introduce a type error by changing the output of rule `A` to be of type + // `string`. + makeChange(ruleAURI, 3, 1, "42", "\"My string\""); + + // There should be a type error in func Foo + List issues = getDiagnostics().get(funcURI); + Assertions.assertEquals(1, issues.size()); + Assertions.assertEquals( + "Expected type `int`, but got `string` instead. Cannot assign `string` to output `result`", + issues.get(0).getMessage()); + } + + @Test + void testChangeInLabelShouldRegenerateLabelProviderForReport() { + createModel("ruleA.rosetta", """ + namespace test + + body Authority Body + corpus Directive "My corpus" Corpus + + report Body Corpus in T+1 + from string + when FilterEligible + with type MyReport + + eligibility rule FilterEligible from string: + item + """); + String typeURI = createModel("type.rosetta", """ + namespace test + + type MyReport: + attr string (1..1) + [label as "My label"] + """); + + String labelProviderPath = "test/labels/BodyCorpusLabelProvider.java"; + + // There should be no issue. + assertNoIssues(); + // There should be a generated label provider. + String originalLabelProviderCode = readGeneratedFile(labelProviderPath); + Assertions.assertNotNull(originalLabelProviderCode, "Label provider does not exist at " + labelProviderPath); + + // Change label to "My new label". + makeChange(typeURI, 4, 12, "\"My label\"", "\"My new label\""); + + // There should again be no issue. + assertNoIssues(); + // The new label provider should be different. + String newLabelProviderCode = readGeneratedFile(labelProviderPath); + Assertions.assertNotNull(newLabelProviderCode, "Label provider does not exist at " + labelProviderPath); + Assertions.assertNotEquals(originalLabelProviderCode, newLabelProviderCode); + } +} diff --git a/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.xtend b/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.xtend deleted file mode 100644 index 2f2d3809f..000000000 --- a/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/server/ChangeDetectionTest.xtend +++ /dev/null @@ -1,169 +0,0 @@ -package com.regnosys.rosetta.ide.server - -import org.junit.jupiter.api.Test -import static org.junit.jupiter.api.Assertions.* -import com.regnosys.rosetta.ide.tests.AbstractRosettaLanguageServerValidationTest - -class ChangeDetectionTest extends AbstractRosettaLanguageServerValidationTest { - @Test - def void testChangeInAttributeTypeIsPropagated() { - val typesURI = createModel("types.rosetta", ''' - namespace test - - type A: - attr int (1..1) - ''') - val funcsURI = createModel("funcs.rosetta", ''' - namespace test - - func Foo: - inputs: input A (1..1) - output: result int (1..1) - - set result: input -> attr - ''') - - // Initial: there should be no issue. - assertNoIssues - - // Introduce a type error by changing the type of `attr` from `int` to `string`. - makeChange(typesURI, 3, 6, "int", "string") - - // There should be a type error in func `Foo` - val issues = diagnostics.get(funcsURI) - assertEquals(1, issues.size) - assertEquals("Expected type `int`, but got `string` instead. Cannot assign `string` to output `result`", issues.head.message) - } - - @Test - def void testChangeInAttributeCardinalityIsPropagated() { - val typesURI = createModel("types.rosetta", ''' - namespace test - - type A: - attr int (1..1) - ''') - val funcsURI = createModel("funcs.rosetta", ''' - namespace test - - func Foo: - inputs: input A (1..1) - output: result int (1..1) - - set result: input -> attr - ''') - - // Initial: there should be no issue. - assertNoIssues - - // Introduce an error by changing the cardinality of `attr` from `(1..1)` to `(0..*)`. - makeChange(typesURI, 3, 10, "(1..1)", "(0..*)") - - // There should be a cardinality error in func `Foo` - val issues = diagnostics.get(funcsURI) - assertEquals(1, issues.size) - assertEquals("Expecting single cardinality. Cannot assign a list to a single value", issues.head.message) - } - - @Test - def void testChangeInAttributeQualifiedTypeIsPropagated() { - createModel("foo.rosetta", ''' - namespace foo - - type MyType: - ''') - createModel("bar.rosetta", ''' - namespace bar - - type MyType: - ''') - val typesURI = createModel("types.rosetta", ''' - namespace test - - import foo.MyType - - type A: - attr MyType (1..1) - ''') - val funcsURI = createModel("funcs.rosetta", ''' - namespace test - - import foo.MyType - - func Foo: - inputs: input A (1..1) - output: result MyType (1..1) - - set result: input -> attr - ''') - - // Initial: there should be no issue. - assertNoIssues - - // Introduce a type error by changing the type of `attr` from `foo.MyType` to `bar.MyType`. - // We do this by changing `import foo.MyType` to `import bar.MyType`. - makeChange(typesURI, 2, 7, "foo", "bar") - - // There should be a type error in func `Foo` - val issues = diagnostics.get(funcsURI) - assertEquals(1, issues.size) - assertEquals("Expected type `foo.MyType`, but got `bar.MyType` instead. Cannot assign `bar.MyType` to output `result`", issues.head.message) - } - - @Test - def void testChangeInRuleInputTypeIsPropagated() { - val ruleAURI = createModel("ruleA.rosetta", ''' - namespace test - - reporting rule A from string: - 42 - ''') - val ruleBURI = createModel("ruleB.rosetta", ''' - namespace test - - reporting rule B from string: - A - ''') - - // Initial: there should be no issue. - assertNoIssues - - // Introduce a type error by changing the input type of rule `A` to `int`. - makeChange(ruleAURI, 2, 22, "string", "int") - - // There should be a type error in rule B - val issues = diagnostics.get(ruleBURI) - assertEquals(1, issues.size) - assertEquals("Expected type `int`, but got `string` instead. Rule `A` cannot be called with type `string`", issues.head.message) - } - - @Test - def void testChangeInRuleExpressionIsPropagated() { - val ruleAURI = createModel("ruleA.rosetta", ''' - namespace test - - reporting rule A from string: - 42 - ''') - val funcURI = createModel("func.rosetta", ''' - namespace test - - func Foo: - output: - result int (1..1) - set result: - A("") - ''') - - // Initial: there should be no issue. - assertNoIssues - - // Introduce a type error by changing the output of rule `A` to be of type `string`. - makeChange(ruleAURI, 3, 1, "42", "\"My string\"") - - // There should be a type error in func Foo - val issues = diagnostics.get(funcURI) - assertEquals(1, issues.size) - assertEquals("Expected type `int`, but got `string` instead. Cannot assign `string` to output `result`", issues.head.message) - } -} diff --git a/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/tests/AbstractRosettaLanguageServerTest.xtend b/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/tests/AbstractRosettaLanguageServerTest.xtend index de8ea93a6..33f191ce6 100644 --- a/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/tests/AbstractRosettaLanguageServerTest.xtend +++ b/rosetta-ide/src/test/java/com/regnosys/rosetta/ide/tests/AbstractRosettaLanguageServerTest.xtend @@ -25,12 +25,10 @@ import org.eclipse.xtext.testing.FileInfo import org.eclipse.xtext.testing.TextDocumentConfiguration import org.eclipse.xtext.testing.TextDocumentPositionConfiguration import org.junit.jupiter.api.Assertions -import org.eclipse.lsp4j.jsonrpc.messages.Either -import org.eclipse.lsp4j.Command import org.eclipse.lsp4j.CodeAction import org.eclipse.lsp4j.CodeActionParams import org.eclipse.lsp4j.CodeActionContext -import java.util.concurrent.CompletableFuture +import java.nio.file.Files /** * TODO: contribute to Xtext. @@ -57,6 +55,14 @@ abstract class AbstractRosettaLanguageServerTest extends AbstractLanguageServerT Assertions.assertEquals(0, problems.size(), "There were issues found:\n" + problems.join('\n')); } + protected def String readGeneratedFile(String relativePath) { + val path = testRootPath.resolve("src/generated/java").resolve(relativePath) + if (Files.exists(path)) { + return Files.readString(path) + } + return null + } + @Accessors static class TestInlayHintsConfiguration extends TextDocumentPositionConfiguration { String expectedInlayHintItems = '' Integer assertNumberOfInlayHints = null diff --git a/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/AttributeDescription.java b/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/AttributeDescription.java index 70d3308b5..512e3f76c 100644 --- a/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/AttributeDescription.java +++ b/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/AttributeDescription.java @@ -11,13 +11,14 @@ public class AttributeDescription extends EObjectDescription { public static final String TYPE_CALL = "TYPE_CALL"; public static final String CARDINALITY = "CARDINALITY"; + public static final String LABELS = "LABELS"; // TODO: also include metadata - public AttributeDescription(QualifiedName qualifiedName, Attribute attribute, String typeCall, String cardinality) { - super(qualifiedName, attribute, createUserData(typeCall, cardinality)); + public AttributeDescription(QualifiedName qualifiedName, Attribute attribute, String typeCall, String cardinality, String labels) { + super(qualifiedName, attribute, createUserData(typeCall, cardinality, labels)); } - private static Map createUserData(String typeCall, String cardinality) { + private static Map createUserData(String typeCall, String cardinality, String labels) { Map userData = new HashMap<>(); if (typeCall != null) { userData.put(TYPE_CALL, typeCall); @@ -25,6 +26,9 @@ private static Map createUserData(String typeCall, String cardin if (cardinality != null) { userData.put(CARDINALITY, cardinality); } + if (labels != null) { + userData.put(LABELS, labels); + } return userData; } } diff --git a/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/RosettaResourceDescriptionStrategy.java b/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/RosettaResourceDescriptionStrategy.java index 1353780e5..407829c3a 100644 --- a/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/RosettaResourceDescriptionStrategy.java +++ b/rosetta-lang/src/main/java/com/regnosys/rosetta/resource/RosettaResourceDescriptionStrategy.java @@ -1,9 +1,12 @@ package com.regnosys.rosetta.resource; +import java.util.stream.Collectors; + import org.apache.log4j.Logger; import org.eclipse.xtext.naming.QualifiedName; import org.eclipse.xtext.nodemodel.INode; import org.eclipse.xtext.nodemodel.util.NodeModelUtils; +import org.eclipse.emf.common.util.EList; import org.eclipse.emf.ecore.EObject; import org.eclipse.xtext.resource.IEObjectDescription; import org.eclipse.xtext.resource.impl.DefaultResourceDescriptionStrategy; @@ -48,7 +51,8 @@ private boolean createAttributeDescription(Attribute attr, IAcceptor list) { + if (list.isEmpty()) { + return null; + } + return list.stream() + .map(e -> serialize(e)) + .filter(s -> s != null) + .collect(Collectors.joining(",")); + } }