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

Fix service type inference #33842

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 @@ -752,7 +752,9 @@ public enum DiagnosticErrorCode implements DiagnosticCode {

INVALID_NON_ISOLATED_CALL_IN_MATCH_GUARD("BCE4018", "invalid.non.isolated.call.in.match.guard"),
INVALID_CALL_WITH_MUTABLE_ARGS_IN_MATCH_GUARD("BCE4019", "invalid.call.with.mutable.args.in.match.guard"),
ERROR_CONSTRUCTOR_COMPATIBLE_TYPE_NOT_FOUND("BCE4020", "error.constructor.compatible.type.not.found")
ERROR_CONSTRUCTOR_COMPATIBLE_TYPE_NOT_FOUND("BCE4020", "error.constructor.compatible.type.not.found"),
CANNOT_INFER_SERVICE_TYPES_FROM_LISTENERS("BCE4021", "cannot.infer.service.type.from.listeners"),
SERVICE_DOES_NOT_IMPLEMENT_REQUIRED_CONSTRUCTS("BCE4022", "service.decl.does.not.implement.required.constructs")
;

private String diagnosticId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.wso2.ballerinalang.compiler.semantics.model.types.BTableType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BTupleType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BTypeIdSet;
import org.wso2.ballerinalang.compiler.semantics.model.types.BUnionType;
import org.wso2.ballerinalang.compiler.tree.BLangAnnotation;
import org.wso2.ballerinalang.compiler.tree.BLangAnnotationAttachment;
Expand Down Expand Up @@ -3580,6 +3581,7 @@ public void visit(BLangLock lockNode) {

@Override
public void visit(BLangService serviceNode) {
inferServiceTypeFromListeners(serviceNode);
addCheckExprToServiceVariable(serviceNode);
analyzeDef(serviceNode.serviceVariable, env);
if (serviceNode.serviceNameLiteral != null) {
Expand All @@ -3589,6 +3591,7 @@ public void visit(BLangService serviceNode) {
serviceNode.setBType(serviceNode.serviceClass.getBType());
BType serviceType = serviceNode.serviceClass.getBType();
BServiceSymbol serviceSymbol = (BServiceSymbol) serviceNode.symbol;
validateServiceTypeImplementation(serviceNode.pos, serviceType, serviceNode.inferredServiceType);

for (BLangExpression attachExpr : serviceNode.attachedExprs) {
final BType exprType = typeChecker.checkExpr(attachExpr, env);
Expand Down Expand Up @@ -3630,6 +3633,72 @@ public void visit(BLangService serviceNode) {
}
}

private void validateServiceTypeImplementation(Location pos, BType implementedType, BType inferredServiceType) {
if (!types.isAssignableIgnoreObjectTypeIds(implementedType, inferredServiceType)) {
if (inferredServiceType.tag == TypeTags.UNION
&& ((BUnionType) inferredServiceType).getMemberTypes().isEmpty()) {
return;
}
dlog.error(pos, DiagnosticErrorCode.SERVICE_DOES_NOT_IMPLEMENT_REQUIRED_CONSTRUCTS, inferredServiceType);
}
}

private void inferServiceTypeFromListeners(BLangService serviceNode) {
LinkedHashSet<BType> listenerTypes = new LinkedHashSet<>();
for (BLangExpression attachExpr : serviceNode.attachedExprs) {
BType type = typeChecker.checkExpr(attachExpr, env);
flatMapAndGetObjectTypes(listenerTypes, type);
}
BType inferred;
if (listenerTypes.size() == 1) {
inferred = listenerTypes.iterator().next();
} else {
BTypeIdSet typeIdSet = null;
for (BType attachType : listenerTypes) {
BObjectType objectType = (BObjectType) attachType;
// todo: distinct union is only allowed when all types have the same type-ids,
// we need to improve this here.
if (objectType.typeIdSet == null || objectType.typeIdSet.isEmpty()) {
continue;
}
if (typeIdSet == null) {
typeIdSet = objectType.typeIdSet;
} else if (!(typeIdSet.isAssignableFrom(objectType.typeIdSet)
&& objectType.typeIdSet.isAssignableFrom(typeIdSet))) {
dlog.error(serviceNode.attachedExprs.get(0).pos,
DiagnosticErrorCode.CANNOT_INFER_SERVICE_TYPES_FROM_LISTENERS);
}
}
inferred = BUnionType.create(null, listenerTypes);
}

serviceNode.inferredServiceType = inferred;
}

private void flatMapAndGetObjectTypes(Set<BType> result, BType type) {
if (!types.checkListenerCompatibilityAtServiceDecl(type)) {
return;
}
if (type.tag == TypeTags.OBJECT) {
BObjectType objectType = (BObjectType) type;
BObjectTypeSymbol tsymbol = (BObjectTypeSymbol) objectType.tsymbol;
for (BAttachedFunction func : tsymbol.attachedFuncs) {
if (func.funcName.value.equals("attach")) {
BType firstParam = func.type.paramTypes.get(0);
result.add(firstParam);
return;
}
}
} else if (type.tag == TypeTags.UNION) {
for (BType memberType : ((BUnionType) type).getMemberTypes()) {
flatMapAndGetObjectTypes(result, memberType);
}
} else if (type.tag == TypeTags.INTERSECTION) {
BType effectiveType = ((BIntersectionType) type).effectiveType;
flatMapAndGetObjectTypes(result, effectiveType);
}
}

private void addCheckExprToServiceVariable(BLangService serviceNode) {
BLangFunction initFunction = serviceNode.serviceClass.initFunction;
if (initFunction != null && initFunction.returnTypeNode != null
Expand All @@ -3647,7 +3716,8 @@ private void validateServiceAttachmentOnListener(BLangService serviceNode, BLang
if (func.funcName.value.equals("attach")) {
List<BType> paramTypes = func.type.paramTypes;
if (serviceType != null && serviceType != symTable.noType) {
validateServiceTypeAgainstAttachMethod(serviceNode.getBType(), paramTypes.get(0), attachExpr.pos);
validateServiceTypeAgainstAttachMethod(serviceNode.inferredServiceType, paramTypes.get(0),
attachExpr.pos);
}
validateServiceAttachpointAgainstAttachMethod(serviceNode, attachExpr, paramTypes.get(1));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public class Types {
private final BLangAnonymousModelHelper anonymousModelHelper;
private int recordCount = 0;
private SymbolEnv env;
private boolean ignoreObjectTypeIds = false;

public static Types getInstance(CompilerContext context) {
Types types = context.get(TYPES_KEY);
Expand Down Expand Up @@ -648,6 +649,13 @@ public boolean isAssignable(BType source, BType target) {
return isAssignable(source, target, new HashSet<>());
}

public boolean isAssignableIgnoreObjectTypeIds(BType source, BType target) {
this.ignoreObjectTypeIds = true;
boolean result = isAssignable(source, target);
this.ignoreObjectTypeIds = false;
return result;
}

private boolean isAssignable(BType source, BType target, Set<TypePair> unresolvedTypes) {

if (isSameType(source, target)) {
Expand Down Expand Up @@ -1576,7 +1584,7 @@ public boolean checkObjectEquivalency(BObjectType rhsType, BObjectType lhsType,
}
}

return lhsType.typeIdSet.isAssignableFrom(rhsType.typeIdSet);
return lhsType.typeIdSet.isAssignableFrom(rhsType.typeIdSet) || this.ignoreObjectTypeIds;
}

private int getObjectFuncCount(BObjectTypeSymbol sym) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class BLangService extends BLangNode implements ServiceNode {
public BLangSimpleVariable serviceVariable;
public List<IdentifierNode> absoluteResourcePath;
public BLangLiteral serviceNameLiteral;
public BType inferredServiceType;

public BLangService() {
this.flagSet = EnumSet.noneOf(Flag.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,12 @@ error.object.type.required=\
error.service.invalid.object.type=\
given type ''{0}'' does not match with service type interface

error.cannot.infer.service.type.from.listeners=\
cannot infer service type from listener attachments

error.service.decl.does.not.implement.required.constructs=\
service declaration does not implement all required constructs of type: ''{0}''

error.service.function.invalid.invocation=\
service method call is allowed only within the type descriptor

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void testServiceInitNegativeTest() {
Assert.assertTrue(output.errorOutput.contains("error: startError"));
}

@Test(groups = { "disableOnOldParser" })
@Test
public void testServiceWithTransactionalKeyword() {
CompileResult compileResult = BCompileUtil.compile("test-src/endpoint/new/service_transactional_negative.bal");
Assert.assertEquals(compileResult.getErrorCount(), 3);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ public void testServiceDecl() {
BRunUtil.invoke(compileResult, "testServiceDecl");
}

@Test()
public void testServiceDeclDistinctListenerArg() {
CompileResult compileResult = BCompileUtil.compile(
"test-src/services/service_decl_distinct_listener_arg.bal");
BRunUtil.invoke(compileResult, "testServiceDecl");
}

@Test()
public void testServiceDeclWithTypeRefDistinctListenerArg() {
CompileResult compileResult = BCompileUtil.compile(
"test-src/services/service_decl_with_type_ref_distinct_listener_arg.bal");
BRunUtil.invoke(compileResult, "testServiceDecl");
}

@Test()
public void testServiceNameLiteral() {
CompileResult compileResult = BCompileUtil.compile("test-src/services/service_decl_service_name_literal.bal");
Expand Down Expand Up @@ -82,7 +96,8 @@ public void testServiceDeclAndListenerAttachmentsNegative() {
validateError(result, i++, "listener variable incompatible types: 'ue' is not a Listener object", 162, 1);
validateError(result, i++, "listener variable incompatible types: 'ui' is not a Listener object", 165, 1);
validateError(result, i++, "incompatible types: expected 'listener', found 'UnionWithInt'", 167, 14);
validateError(result, i++, "service type is not supported by the listener", 190, 14);
validateError(result, i++,
"service declaration does not implement all required constructs of type: 'ServType'", 190, 1);
validateError(result, i++, "service absolute path or literal is required by listener", 209, 12);
validateError(result, i++, "no implementation found for the method 'exec' of service declaration " +
"'object { function exec () returns ((any|error)); }'", 213, 1);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright (c) 2021 WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
//
// WSO2 Inc. licenses this file to you 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 ballerina/jballerina.java;

public type ServiceType distinct service object {
};

public class Listener {
public isolated function 'start() returns error? {
return externStart(self);
}
public isolated function gracefulStop() returns error? {
}
public isolated function immediateStop() returns error? {
}
public isolated function detach(ServiceType s) returns error? {
}
public isolated function attach(ServiceType s, string[]|string? name = ()) returns error? {
return self.register(s, name);
}
isolated function register(service object {} s, string[]|string? name) returns error? {
return externAttach(s, name);
}

public function init() returns error? {
check externLInit(self);
}
}

isolated function externAttach(service object {} s, string[]|string? name) returns error? = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name: "attach"
} external;

isolated function externStart(object {} o) returns error? = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name: "start"
} external;

isolated function externLInit(object {} o) returns error? = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name: "listenerInit"
} external;

function getService() returns object {} = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name: "getService"
} external;

function reset() = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name: "reset"
} external;

public function callMethod(service object {} s, string name) returns future<any|error> = @java:Method {
'class:"org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue",
name:"callMethod"
} external;

listener lsn = new Listener();

service / on lsn {

function init() returns error? {
}

resource function get processRequest() returns json {
return { output: "Hello" };
}

function createError() returns @tainted error? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, do we still use @tainted ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not needed. Should be a copied code.

return ();
}
}

function testServiceDecl() {
any|error v = wait callMethod(<service object {}> getService(), "$get$processRequest");
reset();
assertEquality(v, <json> { output: "Hello" });
}

function assertEquality(any|error actual, any|error expected) {
if expected is anydata && actual is anydata && expected == actual {
return;
}

if expected === actual {
return;
}

string expectedValAsString = expected is error ? expected.toString() : expected.toString();
string actualValAsString = actual is error ? actual.toString() : actual.toString();
panic error("AssertionError",
message = "expected '" + expectedValAsString + "', found '" + actualValAsString + "'");
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,28 @@ service on x {
service ServType / on new ServTypeListener() {

}

type DServ distinct service object {

};

public class DServListener {
public isolated function 'start() returns error? {
}
public isolated function gracefulStop() returns error? {
}
public isolated function immediateStop() returns error? {
}
public isolated function detach(DServ s) returns error? {
}
public isolated function attach(DServ s, string[]|string name = "") returns error? {
}
}

service DServ / on new DServListener() {

}

service / on new DServListener() {

}
Loading