Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type args bug fixes #3269

Merged
merged 17 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,116 @@ public void testProfileLoadingWithPackageOffset()
{
testProtocolLoadingModelWithPackageOffset("profileUsedInClassExample.json", null, "update::");
}

@Test
public void testNewConstructorWithMissingTypeArgumentsCompiles()
{
testWithJson("{\n" +
" \"_type\": \"data\",\n" +
" \"elements\": [\n" +
" {\n" +
" \"_type\": \"function\",\n" +
" \"body\": [\n" +
" {\n" +
" \"_type\": \"func\",\n" +
" \"function\": \"new\",\n" +
" \"parameters\": [\n" +
" {\n" +
// ---------- type to construct, no type arguments
" \"_type\": \"packageableElementPtr\",\n" +
" \"fullPath\": \"TdsOlapRank\"\n" +
// ---------- type to construct, no type arguments
" },\n" +
" {\n" +
" \"_type\": \"string\",\n" +
" \"value\": \"\"\n" +
" },\n" +
" {\n" +
" \"_type\": \"collection\",\n" +
" \"multiplicity\": {\n" +
" \"lowerBound\": 1,\n" +
" \"upperBound\": 1\n" +
" },\n" +
" \"values\": [\n" +
" {\n" +
" \"_type\": \"keyExpression\",\n" +
" \"add\": false,\n" +
" \"expression\": {\n" +
" \"_type\": \"lambda\",\n" +
" \"body\": [\n" +
" {\n" +
" \"_type\": \"integer\",\n" +
" \"value\": 1\n" +
" }\n" +
" ],\n" +
" \"parameters\": [\n" +
" {\n" +
" \"_type\": \"var\",\n" +
" \"genericType\": {\n" +
" \"multiplicityArguments\": [],\n" +
" \"rawType\": {\n" +
" \"_type\": \"packageableType\",\n" +
" \"fullPath\": \"TDSRow\"\n" +
" },\n" +
" \"typeArguments\": [],\n" +
" \"typeVariableValues\": []\n" +
" },\n" +
" \"multiplicity\": {\n" +
" \"lowerBound\": 1,\n" +
" \"upperBound\": 1\n" +
" },\n" +
" \"name\": \"r\"\n" +
" }\n" +
" ]\n" +
" },\n" +
" \"key\": {\n" +
" \"_type\": \"string\",\n" +
" \"value\": \"func\"\n" +
" }\n" +
" }\n" +
" ]\n" +
" }\n" +
" ]\n" +
" }\n" +
" ],\n" +
" \"name\": \"new__Any_1_\",\n" +
" \"package\": \"test\",\n" +
" \"parameters\": [],\n" +
" \"postConstraints\": [],\n" +
" \"preConstraints\": [],\n" +
" \"returnGenericType\": {\n" +
" \"multiplicityArguments\": [],\n" +
" \"rawType\": {\n" +
" \"_type\": \"packageableType\",\n" +
" \"fullPath\": \"Any\"\n" +
" },\n" +
" \"typeArguments\": [],\n" +
" \"typeVariableValues\": []\n" +
" },\n" +
" \"returnMultiplicity\": {\n" +
" \"lowerBound\": 1,\n" +
" \"upperBound\": 1\n" +
" },\n" +
" \"stereotypes\": [],\n" +
" \"taggedValues\": [],\n" +
" \"tests\": []\n" +
" },\n" +
" {\n" +
" \"_type\": \"sectionIndex\",\n" +
" \"name\": \"SectionIndex\",\n" +
" \"package\": \"__internal__\",\n" +
" \"sections\": [\n" +
" {\n" +
" \"_type\": \"importAware\",\n" +
" \"elements\": [\n" +
" \"test::new__Any_1_\"\n" +
" ],\n" +
" \"imports\": [],\n" +
" \"parserName\": \"Pure\"\n" +
" }\n" +
" ]\n" +
" }\n" +
" ]\n" +
"}", null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void testProtocolGeneration()
@Test
public void testImportParsing()
{
test(getJsonString("pureImportParsingTest.json"), "{\"isolatedLambdas\":{\"lambdas\":{}},\"modelDataContext\":{\"_type\":\"data\",\"elements\":[{\"_type\":\"class\",\"constraints\":[],\"name\":\"Person\",\"originalMilestonedProperties\":[],\"package\":\"model\",\"properties\":[{\"genericType\":{\"multiplicityArguments\":[],\"rawType\":{\"_type\":\"packageableType\",\"fullPath\":\"TargetA\",\"sourceInformation\":{\"endColumn\":18,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":12,\"startLine\":4}},\"typeArguments\":[],\"typeVariableValues\":[]},\"multiplicity\":{\"lowerBound\":1,\"upperBound\":1},\"name\":\"targetA\",\"sourceInformation\":{\"endColumn\":22,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":3,\"startLine\":4},\"stereotypes\":[],\"taggedValues\":[]}],\"qualifiedProperties\":[],\"sourceInformation\":{\"endColumn\":1,\"endLine\":5,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":2},\"stereotypes\":[],\"superTypes\":[\"VersionClass\"],\"taggedValues\":[{\"sourceInformation\":{\"endColumn\":20,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2},\"tag\":{\"profile\":\"doc\",\"profileSourceInformation\":{\"endColumn\":10,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2},\"sourceInformation\":{\"endColumn\":14,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":12,\"startLine\":2},\"value\":\"doc\"},\"value\":\"a\"}]},{\"_type\":\"sectionIndex\",\"name\":\"SectionIndex\",\"package\":\"__internal__\",\"sections\":[{\"_type\":\"importAware\",\"elements\":[\"model::Person\"],\"imports\":[\"projectA\"],\"parserName\":\"Pure\",\"sourceInformation\":{\"endColumn\":2,\"endLine\":7,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":1}}]}]},\"renderStyle\":\"STANDARD\"}");
test(getJsonString("pureImportParsingTest.json"), "{\"isolatedLambdas\":{\"lambdas\":{}},\"modelDataContext\":{\"_type\":\"data\",\"elements\":[{\"_type\":\"class\",\"constraints\":[],\"name\":\"Person\",\"originalMilestonedProperties\":[],\"package\":\"model\",\"properties\":[{\"genericType\":{\"multiplicityArguments\":[],\"rawType\":{\"_type\":\"packageableType\",\"fullPath\":\"TargetA\",\"sourceInformation\":{\"endColumn\":18,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":12,\"startLine\":4}},\"typeArguments\":[],\"typeVariableValues\":[]},\"multiplicity\":{\"lowerBound\":1,\"upperBound\":1},\"name\":\"targetA\",\"sourceInformation\":{\"endColumn\":22,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":3,\"startLine\":4},\"stereotypes\":[],\"taggedValues\":[]}],\"qualifiedProperties\":[],\"sourceInformation\":{\"endColumn\":1,\"endLine\":5,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":2},\"stereotypes\":[],\"superTypes\":[{\"path\":\"VersionClass\",\"sourceInformation\":{\"endColumn\":56,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":45,\"startLine\":2},\"type\":\"CLASS\"}],\"taggedValues\":[{\"sourceInformation\":{\"endColumn\":20,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2},\"tag\":{\"profile\":\"doc\",\"profileSourceInformation\":{\"endColumn\":10,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2},\"sourceInformation\":{\"endColumn\":14,\"endLine\":2,\"sourceId\":\"\",\"startColumn\":12,\"startLine\":2},\"value\":\"doc\"},\"value\":\"a\"}]},{\"_type\":\"sectionIndex\",\"name\":\"SectionIndex\",\"package\":\"__internal__\",\"sections\":[{\"_type\":\"importAware\",\"elements\":[\"model::Person\"],\"imports\":[\"projectA\"],\"parserName\":\"Pure\",\"sourceInformation\":{\"endColumn\":2,\"endLine\":7,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":1}}]}]},\"renderStyle\":\"STANDARD\"}");
}

@Test
Expand Down Expand Up @@ -96,7 +96,7 @@ public void testMappingIncludeSerialization()
@Test
public void testEnumerationMappingWithStructuredSourceValueSerialization()
{
test("{\"code\": \"###Mapping\\nMapping a::mapping\\n(test::IncType : EnumerationMapping a\\n{ CORP : [1], LLC : [2] })\"}", "{\"isolatedLambdas\":{\"lambdas\":{}},\"modelDataContext\":{\"_type\":\"data\",\"elements\":[{\"_type\":\"mapping\",\"associationMappings\":[],\"classMappings\":[],\"enumerationMappings\":[{\"enumValueMappings\":[{\"enumValue\":\"CORP\",\"sourceValues\":[{\"_type\":\"integerSourceValue\",\"value\":1}]},{\"enumValue\":\"LLC\",\"sourceValues\":[{\"_type\":\"integerSourceValue\",\"value\":2}]}],\"enumeration\":\"test::IncType\",\"id\":\"a\",\"sourceInformation\":{\"endColumn\":25,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":2,\"startLine\":3}}],\"includedMappings\":[],\"name\":\"mapping\",\"package\":\"a\",\"sourceInformation\":{\"endColumn\":26,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":2},\"tests\":[]},{\"_type\":\"sectionIndex\",\"name\":\"SectionIndex\",\"package\":\"__internal__\",\"sections\":[{\"_type\":\"importAware\",\"elements\":[],\"imports\":[],\"parserName\":\"Pure\",\"sourceInformation\":{\"endColumn\":8,\"endLine\":1,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":1}},{\"_type\":\"importAware\",\"elements\":[\"a::mapping\"],\"imports\":[],\"parserName\":\"Mapping\",\"sourceInformation\":{\"endColumn\":26,\"endLine\":6,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2}}]}]},\"renderStyle\":\"STANDARD\"}");
test("{\"code\": \"###Mapping\\nMapping a::mapping\\n(test::IncType : EnumerationMapping a\\n{ CORP : [1], LLC : [2] })\"}", "{\"isolatedLambdas\":{\"lambdas\":{}},\"modelDataContext\":{\"_type\":\"data\",\"elements\":[{\"_type\":\"mapping\",\"associationMappings\":[],\"classMappings\":[],\"enumerationMappings\":[{\"enumValueMappings\":[{\"enumValue\":\"CORP\",\"sourceValues\":[{\"_type\":\"integerSourceValue\",\"value\":1}]},{\"enumValue\":\"LLC\",\"sourceValues\":[{\"_type\":\"integerSourceValue\",\"value\":2}]}],\"enumeration\":{\"path\":\"test::IncType\",\"sourceInformation\":{\"endColumn\":14,\"endLine\":3,\"sourceId\":\"\",\"startColumn\":2,\"startLine\":3},\"type\":\"ENUMERATION\"},\"id\":\"a\",\"sourceInformation\":{\"endColumn\":25,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":2,\"startLine\":3}}],\"includedMappings\":[],\"name\":\"mapping\",\"package\":\"a\",\"sourceInformation\":{\"endColumn\":26,\"endLine\":4,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":2},\"tests\":[]},{\"_type\":\"sectionIndex\",\"name\":\"SectionIndex\",\"package\":\"__internal__\",\"sections\":[{\"_type\":\"importAware\",\"elements\":[],\"imports\":[],\"parserName\":\"Pure\",\"sourceInformation\":{\"endColumn\":8,\"endLine\":1,\"sourceId\":\"\",\"startColumn\":1,\"startLine\":1}},{\"_type\":\"importAware\",\"elements\":[\"a::mapping\"],\"imports\":[],\"parserName\":\"Mapping\",\"sourceInformation\":{\"endColumn\":26,\"endLine\":6,\"sourceId\":\"\",\"startColumn\":8,\"startLine\":2}}]}]},\"renderStyle\":\"STANDARD\"}");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,10 @@
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
<scope>test</scope>
</dependency>
<!-- JACKSON -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.finos.legend.engine.language.pure.grammar.from;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.eclipse.collections.api.factory.Lists;
Expand All @@ -37,6 +38,7 @@
import org.finos.legend.engine.protocol.pure.v1.model.valueSpecification.ValueSpecification;
import org.finos.legend.engine.protocol.pure.v1.model.valueSpecification.raw.Lambda;
import org.finos.legend.engine.protocol.pure.v1.model.valueSpecification.raw.classInstance.graph.RootGraphFetchTree;
import org.finos.legend.engine.shared.core.ObjectMapperFactory;
import org.finos.legend.engine.shared.core.identity.Identity;
import org.finos.legend.engine.shared.core.operational.errorManagement.EngineException;
import org.finos.legend.engine.shared.core.operational.logs.LogInfo;
Expand All @@ -52,6 +54,7 @@ public class PureGrammarParser

private final DEPRECATED_PureGrammarParserLibrary parsers;
private final PureGrammarParserExtensions extensions;
private ObjectMapper converterMapper;

private PureGrammarParser(PureGrammarParserExtensions extensions)
{
Expand All @@ -63,6 +66,7 @@ private PureGrammarParser(PureGrammarParserExtensions extensions)
connectionParser,
RuntimeParser.newInstance(connectionParser)
));
this.converterMapper = ObjectMapperFactory.getNewStandardObjectMapperWithPureProtocolExtensionSupports();
}

public static PureGrammarParser newInstance(PureGrammarParserExtensions extensions)
Expand Down Expand Up @@ -127,7 +131,9 @@ private PureModelContextData parse(String code, DEPRECATED_PureGrammarParserLibr
sectionIndex.name = "SectionIndex";
sectionIndex._package = "__internal__";
sectionIndex.sections = ListIterate.collect(parser.definition().section(), sectionCtx -> this.visitSection(sectionCtx, parserLibrary, walkerSourceInformation, parserContext, builder::addElement, returnSourceInfo));
return builder.withElement(sectionIndex).build();
// tactically run parsed values thru converters to fix old/legacy code
PureModelContextData pmcd = builder.withElement(sectionIndex).build();
return this.converterMapper.convertValue(pmcd, PureModelContextData.class);
}

private Section visitSection(CodeParserGrammar.SectionContext ctx, DEPRECATED_PureGrammarParserLibrary parserLibrary, ParseTreeWalkerSourceInformation walkerSourceInformation, PureGrammarParserContext parserContext, Consumer<PackageableElement> elementConsumer, boolean returnSourceInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ else if ("letFunction".equals(function))
}
else if ("cast".equals(function))
{
return parameters.get(0).accept(this) + "->" + _function + "(@" + parameters.get(1).accept(this) + ")";
return possiblyAddParenthesis(parameters.get(0), this) + "->" + _function + "(@" + parameters.get(1).accept(this) + ")";
}
else if ("subType".equals(function))
{
Expand All @@ -791,7 +791,7 @@ else if ("subType".equals(function))
else if ("new".equals(function))
{
ValueSpecification param = parameters.get(parameters.size() - 1);
List<ValueSpecification> values = param instanceof Collection ? ((Collection) param).values : Arrays.asList(param);
List<ValueSpecification> values = param instanceof Collection ? ((Collection) param).values : Collections.singletonList(param);
String type;
if (parameters.get(0) instanceof GenericTypeInstance)
{
Expand Down Expand Up @@ -842,7 +842,7 @@ else if (parameters.size() == 1)
+ (toCreateNewLine ? this.returnChar() + DEPRECATED_PureGrammarComposerCore.computeIndentationString(this, getTabSize(1)) : " ")
+ possiblyAddParenthesis(function, parameters.get(1), this);
}
else if (function != null && CORE_FUNCTIONS_WITH_PREFIX_RENDERING.contains(function))
else if (CORE_FUNCTIONS_WITH_PREFIX_RENDERING.contains(function))
{
return HelperValueSpecificationGrammarComposer.renderFunctionName(function, this) + "(" +
(this.isRenderingPretty() ? this.returnChar() + DEPRECATED_PureGrammarComposerCore.computeIndentationString(this, getTabSize(1)) : "") +
Expand Down Expand Up @@ -877,12 +877,7 @@ public String visit(AppliedProperty appliedProperty)
@Override
public String visit(Collection collection)
{

return HelperValueSpecificationGrammarComposer.renderCollection(collection.values, v ->
{
String value = ((ValueSpecification) v).accept(this);
return v instanceof AppliedFunction && isInfix((AppliedFunction) v) ? "(" + value + ")" : value;
}, this);
return HelperValueSpecificationGrammarComposer.renderCollection(collection.values, v -> possiblyAddParenthesis((ValueSpecification) v, this), this);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,16 @@ public static String possiblyAddParenthesis(String function, ValueSpecification
{
if ("and".equals(function) || "or".equals(function) || "plus".equals(function) || "minus".equals(function) || "times".equals(function) || "divide".equals(function) || "not".equals(function))
{
if (param instanceof AppliedFunction && isInfix((AppliedFunction) param))
{
return "(" + param.accept(transformer) + ")";
}
return possiblyAddParenthesis(param, transformer);
}
return param.accept(transformer);
}

public static String possiblyAddParenthesis(ValueSpecification param, DEPRECATED_PureGrammarComposerCore transformer)
{
if (param instanceof AppliedFunction && isInfix((AppliedFunction) param))
{
return "(" + param.accept(transformer) + ")";
}
return param.accept(transformer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ private static void testFormat(String code, String unformattedCode, boolean omit
{
parsedModel = PureModelContextData.newPureModelContextData(parsedModel.getSerializer(), parsedModel.getOrigin(), LazyIterate.reject(parsedModel.getElements(), e -> e instanceof SectionIndex));
}

String formatted = grammarTransformer.renderPureModelContextData(parsedModel);
Assert.assertEquals(code, formatted);
// NOTE: do not remove the round-trip test for formatted code as this is a very good way to ensure that grammar <-> >protocol is bijective
Expand Down
Loading
Loading