Skip to content

Commit

Permalink
Function activator to use pointer to capture source information (#2496)
Browse files Browse the repository at this point in the history 
* Function activator to use pointer to capture source information

* Capture parsing error for better error handler on VS code plugin.
  • Loading branch information
@rafaelbey
rafaelbey authored Dec 11, 2023
1 parent 554665f commit 75a7d62
Show file tree
Hide file tree
Showing 22 changed files with 100 additions and 41 deletions.
5 changes: 5 additions & 0 deletions ...engine-core/legend-engine-core-language-pure/legend-engine-language-pure-compiler/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,11 @@
<artifactId>legend-pure-runtime-java-extension-functions-json</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<scope>test</scope>
</dependency>
<!-- TEST -->
</dependencies>
</project>
17 changes: 9 additions & 8 deletions .../java/org/finos/legend/engine/language/pure/compiler/test/TestCompilationFromGrammar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
package org.finos.legend.engine.language.pure.compiler.test;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.collections.api.tuple.Pair;
import org.eclipse.collections.impl.tuple.Tuples;
import org.finos.legend.engine.language.pure.compiler.Compiler;
Expand All @@ -25,13 +28,11 @@
import org.finos.legend.engine.shared.core.ObjectMapperFactory;
import org.finos.legend.engine.shared.core.deployment.DeploymentMode;
import org.finos.legend.engine.shared.core.operational.errorManagement.EngineException;
import org.hamcrest.CoreMatchers;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
import org.junit.Test;

import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

public class TestCompilationFromGrammar
{
public abstract static class TestCompilationFromGrammarTestSuite
Expand Down Expand Up @@ -78,8 +79,7 @@ public static Pair<PureModelContextData, PureModel> test(String str, String expe

if (expectedWarnings != null)
{
List<String> warnings = pureModel.getWarnings().stream().map(Warning::buildPrettyWarningMessage).collect(Collectors.toList());
Collections.sort(warnings);
List<String> warnings = pureModel.getWarnings().stream().map(Warning::buildPrettyWarningMessage).sorted().collect(Collectors.toList());
Collections.sort(expectedWarnings);
Assert.assertEquals(expectedWarnings, warnings);
}
Expand All @@ -93,7 +93,8 @@ public static Pair<PureModelContextData, PureModel> test(String str, String expe
throw e;
}
Assert.assertNotNull("No source information provided in error", e.getSourceInformation());
Assert.assertEquals(expectedErrorMsg, EngineException.buildPrettyErrorMessage(e.getMessage(), e.getSourceInformation(), e.getErrorType()));
MatcherAssert.assertThat(EngineException.buildPrettyErrorMessage(e.getMessage(), e.getSourceInformation(),
e.getErrorType()), CoreMatchers.startsWith(expectedErrorMsg));
return null;
}
catch (Exception e)
Expand Down Expand Up @@ -312,7 +313,7 @@ public void testCompilationCme()
"['includedDate', 'calendarAgg'])" +
"}");
}

@Test
public void testCompilationCw()
{
Expand Down
2 changes: 1 addition & 1 deletion ...est/java/org/finos/legend/engine/language/pure/grammar/api/test/TestGrammarToJsonApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void testLambdaParsingError()
@Test
public void testMixedParsingErrors()
{
test("{\"code\": \"Class A {,\", \"isolatedLambdas\": {\"good\": \"|'good'\", \"bad\": \"|,\"}}", "{\"codeError\":{\"message\":\"Unexpected token ','\",\"sourceInformation\":{\"endColumn\":10,\"endLine\":1,\"sourceId\":\"\",\"startColumn\":10,\"startLine\":1}},\"isolatedLambdas\":{\"lambdaErrors\":{\"bad\":{\"message\":\"Unexpected token ','\",\"sourceInformation\":{\"endColumn\":2,\"endLine\":1,\"sourceId\":\"bad\",\"startColumn\":2,\"startLine\":1}}},\"lambdas\":{\"good\":{\"_type\":\"lambda\",\"body\":[{\"_type\":\"string\",\"sourceInformation\":{\"endColumn\":7,\"endLine\":1,\"sourceId\":\"good\",\"startColumn\":2,\"startLine\":1},\"value\":\"good\"}],\"parameters\":[],\"sourceInformation\":{\"endColumn\":7,\"endLine\":1,\"sourceId\":\"good\",\"startColumn\":1,\"startLine\":1}}}},\"renderStyle\":\"STANDARD\"}");
test("{\"code\": \"Class A {,\", \"isolatedLambdas\": {\"good\": \"|'good'\", \"bad\": \"|,\"}}", "{\"codeError\":{\"message\":\"Unexpected token ','. Valid alternatives: ['import', 'Class', 'Association', 'Profile', 'Enum', 'Measure', 'function', 'extends', 'stereotypes', 'tags', 'Error', 'Warn', 'native', 'projects', 'as', 'composite', 'shared', 'none', 'all', 'let', 'allVersions', 'allVersionsInRange', 'toBytes', '(', '<']\",\"sourceInformation\":{\"endColumn\":10,\"endLine\":1,\"sourceId\":\"\",\"startColumn\":10,\"startLine\":1}},\"isolatedLambdas\":{\"lambdaErrors\":{\"bad\":{\"message\":\"Unexpected token ','. Valid alternatives: ['import', 'Class', 'Association', 'Profile', 'Enum', 'Measure', 'function', 'extends', 'stereotypes', 'tags', 'Error', 'Warn', 'native', 'projects', 'as', 'composite', 'shared', 'none', 'all', 'let', 'allVersions', 'allVersionsInRange', 'toBytes', '!', '[', '(', '$', '^', '|', '@', '+', '-']\",\"sourceInformation\":{\"endColumn\":2,\"endLine\":1,\"sourceId\":\"bad\",\"startColumn\":2,\"startLine\":1}}},\"lambdas\":{\"good\":{\"_type\":\"lambda\",\"body\":[{\"_type\":\"string\",\"sourceInformation\":{\"endColumn\":7,\"endLine\":1,\"sourceId\":\"good\",\"startColumn\":2,\"startLine\":1},\"value\":\"good\"}],\"parameters\":[],\"sourceInformation\":{\"endColumn\":7,\"endLine\":1,\"sourceId\":\"good\",\"startColumn\":1,\"startLine\":1}}}},\"renderStyle\":\"STANDARD\"}");
}

@Test
Expand Down
5 changes: 5 additions & 0 deletions ...-engine-core/legend-engine-core-language-pure/legend-engine-language-pure-grammar/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<scope>test</scope>
</dependency>
<!-- TEST -->
</dependencies>
</project>
10 changes: 4 additions & 6 deletions ...src/main/java/org/finos/legend/engine/language/pure/grammar/from/ParserErrorListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void syntaxError(
{
if (e != null && e.getOffendingToken() != null && e instanceof InputMismatchException)
{
List<String> expectedSymbols = dereferenceTokens(e.getExpectedTokens().toList());
List<String> expectedSymbols = dereferenceTokens(e.getExpectedTokens().toList(), recognizer.getVocabulary());
if (expectedSymbols.isEmpty())
{
msg = "Unexpected token '" + e.getOffendingToken().getText() + "'";
Expand Down Expand Up @@ -103,13 +103,11 @@ else if (e == null || e.getOffendingToken() == null)
charPositionInLine + 1 + (line == 1 ? this.walkerSourceInformation.getColumnOffset() : 0),
offendingToken.getLine() + this.walkerSourceInformation.getLineOffset(),
charPositionInLine + offendingToken.getText().length() + (line == 1 ? this.walkerSourceInformation.getColumnOffset() : 0));
throw new EngineException(msg, sourceInformation, EngineErrorType.PARSER);
throw new EngineException(msg, sourceInformation, EngineErrorType.PARSER, e);
}

protected List<String> dereferenceTokens(List<Integer> expectedTokens)
protected List<String> dereferenceTokens(List<Integer> expectedTokens, Vocabulary vocabulary)
{
return vocabulary
.<List<String>>map(v -> ListIterate.collect(expectedTokens, v::getLiteralName).select(Objects::nonNull))
.orElse(Collections.emptyList());
return ListIterate.collect(expectedTokens, vocabulary::getLiteralName).select(Objects::nonNull);
}
}
5 changes: 4 additions & 1 deletion ...r/src/test/java/org/finos/legend/engine/language/pure/grammar/test/TestGrammarParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.finos.legend.engine.shared.core.operational.errorManagement.EngineException;
import org.finos.legend.engine.shared.core.operational.logs.LogInfo;
import org.finos.legend.engine.shared.core.operational.logs.LoggingEventType;
import org.hamcrest.CoreMatchers;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -101,7 +103,8 @@ protected static void test(String val, String expectedErrorMsg)
{
LogInfo errorResponse = new LogInfo(null, LoggingEventType.PARSE_ERROR, e);
Assert.assertNotNull("No source information provided in error", errorResponse.sourceInformation);
Assert.assertEquals(expectedErrorMsg, EngineException.buildPrettyErrorMessage(errorResponse.message, errorResponse.sourceInformation, EngineErrorType.PARSER));
MatcherAssert.assertThat(EngineException.buildPrettyErrorMessage(errorResponse.message, errorResponse.sourceInformation,
EngineErrorType.PARSER), CoreMatchers.startsWith(expectedErrorMsg));
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions ...ava/org/finos/legend/engine/protocol/pure/v1/model/context/PackageableElementPointer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

package org.finos.legend.engine.protocol.pure.v1.model.context;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.finos.legend.engine.protocol.pure.v1.model.SourceInformation;

import java.util.Objects;
Expand All @@ -36,11 +38,20 @@ public PackageableElementPointer(PackageableElementType type, String path)
this.path = path;
}

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)
public PackageableElementPointer(String path)
{
this.path = path;
}

@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
public PackageableElementPointer(@JsonProperty("type") PackageableElementType type, @JsonProperty("path") String path, @JsonProperty("sourceInformation") SourceInformation sourceInformation)
{
this.type = type;
this.path = path;
this.sourceInformation = sourceInformation;
}

@Override
public String toString()
{
Expand Down
1 change: 1 addition & 0 deletions ...n/java/org/finos/legend/engine/protocol/pure/v1/model/context/PackageableElementType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public enum PackageableElementType
CLASS,
ASSOCIATION,
ENUMERATION,
FUNCTION,
STORE,
RUNTIME,
MAPPING,
Expand Down
17 changes: 15 additions & 2 deletions ...re/src/test/java/org/finos/legend/engine/protocol/test/TestCompatibilityAndMigration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import net.javacrumbs.jsonunit.JsonAssert;
import org.finos.legend.engine.protocol.pure.v1.PureProtocolObjectMapperFactory;
import org.finos.legend.engine.protocol.pure.v1.model.context.PackageableElementPointer;
import org.finos.legend.engine.protocol.pure.v1.model.context.PureModelContextData;
import org.junit.Assert;
import org.junit.Test;

public class TestCompatibilityAndMigration
{
private static final ObjectMapper objectMapper = PureProtocolObjectMapperFactory.getNewObjectMapper();
private static final ObjectMapper objectMapper = PureProtocolObjectMapperFactory.getNewObjectMapper()
.setSerializationInclusion(JsonInclude.Include.NON_EMPTY);

@Test
public void testStringValueSpecification() throws Exception
Expand Down Expand Up @@ -1201,10 +1203,21 @@ public void testModelStoreData() throws Exception
"}\n");
}

@Test
public void testPackageableElementPointerCompatibility() throws Exception
{
String asString = "\"abc::myPath::MyName\"";
String expected = "{\"path\":\"abc::myPath::MyName\"}";
PackageableElementPointer pointerFromStringConstructor = objectMapper.readValue(asString, PackageableElementPointer.class);
String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(pointerFromStringConstructor);
JsonAssert.assertJsonEquals(expected, json);
PackageableElementPointer expectedPointerFromObjectConstructor = objectMapper.readValue(expected, PackageableElementPointer.class);
Assert.assertEquals(expectedPointerFromObjectConstructor, pointerFromStringConstructor);
}

private void check(String input, String output) throws Exception
{
PureModelContextData context = objectMapper.readValue(input, PureModelContextData.class);
objectMapper.setSerializationInclusion(JsonInclude.Include.NON_EMPTY);
String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(context);
JsonAssert.assertJsonEquals(output, json);
}
Expand Down
2 changes: 1 addition & 1 deletion ...ine/language/bigqueryFunction/compiler/toPureGraph/BigQueryFunctionCompilerExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public Root_meta_external_function_activator_bigQueryFunction_BigQueryFunction b
{
try
{
PackageableFunction<?> func = (PackageableFunction<?>) context.resolvePackageableElement(FunctionDescriptor.functionDescriptorToId(bigQueryFunction.function), bigQueryFunction.sourceInformation);
PackageableFunction<?> func = (PackageableFunction<?>) context.resolvePackageableElement(FunctionDescriptor.functionDescriptorToId(bigQueryFunction.function.path), bigQueryFunction.sourceInformation);
return new Root_meta_external_function_activator_bigQueryFunction_BigQueryFunction_Impl(
bigQueryFunction.name,
null,
Expand Down
10 changes: 8 additions & 2 deletions ...inos/legend/engine/language/bigqueryFunction/grammar/from/BigQueryFunctionTreeWalker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
import org.finos.legend.engine.language.pure.grammar.from.PureGrammarParserUtility;
import org.finos.legend.engine.language.pure.grammar.from.antlr4.BigQueryFunctionParserGrammar;
import org.finos.legend.engine.protocol.bigqueryFunction.metamodel.BigQueryFunctionDeploymentConfiguration;
import org.finos.legend.engine.protocol.functionActivator.metamodel.DeploymentConfiguration;
import org.finos.legend.engine.protocol.pure.v1.model.context.PackageableElementPointer;
import org.finos.legend.engine.protocol.pure.v1.model.context.PackageableElementType;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.PackageableElement;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.connection.ConnectionPointer;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.domain.StereotypePtr;
Expand Down Expand Up @@ -84,7 +85,12 @@ private BigQueryFunction visitBigQueryFunction(BigQueryFunctionParserGrammar.Big
BigQueryFunctionParserGrammar.FunctionNameContext functionNameContext = PureGrammarParserUtility.validateAndExtractRequiredField(ctx.functionName(), "functionName", bigQueryFunction.sourceInformation);
bigQueryFunction.functionName = PureGrammarParserUtility.fromGrammarString(functionNameContext.STRING().getText(), true);
BigQueryFunctionParserGrammar.FunctionContext functionContext = PureGrammarParserUtility.validateAndExtractRequiredField(ctx.function(), "function", bigQueryFunction.sourceInformation);
bigQueryFunction.function = functionContext.functionIdentifier().getText();
bigQueryFunction.function = new PackageableElementPointer(
PackageableElementType.FUNCTION,
functionContext.functionIdentifier().getText(),
walkerSourceInformation.getSourceInformation(functionContext.functionIdentifier())
);

BigQueryFunctionParserGrammar.OwnerContext ownerContext = PureGrammarParserUtility.validateAndExtractOptionalField(ctx.owner(), "owner", bigQueryFunction.sourceInformation);
if (ownerContext != null)
{
Expand Down
2 changes: 1 addition & 1 deletion ...s/legend/engine/language/bigqueryFunction/grammar/to/BigQueryFunctionGrammarComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private static String renderBigQueryFunction(BigQueryFunction app)
return "BigQueryFunction " + renderAnnotations(app.stereotypes, app.taggedValues) + packageName + "\n" +
"{\n" +
" functionName : '" + app.functionName + "';\n" +
" function : " + app.function + ";\n" +
" function : " + app.function.path + ";\n" +
(app.owner == null ? "" : " owner : '" + app.owner + "';\n") +
(app.description == null ? "" : " description : '" + app.description + "';\n") +
(app.activationConfiguration == null ? "" : " activationConfiguration : " + ((BigQueryFunctionDeploymentConfiguration) app.activationConfiguration).activationConnection.connection + ";\n") +
Expand Down
3 changes: 2 additions & 1 deletion .../java/org/finos/legend/engine/protocol/functionActivator/metamodel/FunctionActivator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.finos.legend.engine.protocol.functionActivator.metamodel;

import org.finos.legend.engine.protocol.pure.v1.model.context.PackageableElementPointer;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.PackageableElement;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.domain.StereotypePtr;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.domain.TaggedValue;
Expand All @@ -28,6 +29,6 @@ public abstract class FunctionActivator extends PackageableElement
{
public List<StereotypePtr> stereotypes = Collections.emptyList();
public List<TaggedValue> taggedValues = Collections.emptyList();
public String function;
public PackageableElementPointer function;
public DeploymentConfiguration activationConfiguration;
}
2 changes: 1 addition & 1 deletion ...nd/engine/language/hostedService/compiler/toPureGraph/HostedServiceCompilerExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public Root_meta_external_function_activator_hostedService_HostedService buildHo
{
try
{
PackageableFunction<?> func = (PackageableFunction<?>) context.resolvePackageableElement(FunctionDescriptor.functionDescriptorToId(app.function), app.sourceInformation);
PackageableFunction<?> func = (PackageableFunction<?>) context.resolvePackageableElement(FunctionDescriptor.functionDescriptorToId(app.function.path), app.sourceInformation);
return new Root_meta_external_function_activator_hostedService_HostedService_Impl(
app.name,
null,
Expand Down
Loading

0 comments on commit 75a7d62

Please sign in to comment.