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

Restrict annotation value to true, map<anydata|readonly> , map<anydata|readonly>[] #23473

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,17 @@ public BObjectType getType() {

@Override
public Object copy(Map<Object, Object> refs) {
throw new UnsupportedOperationException();
return this;
}

@Override
public void freezeDirect() {
return;
}

@Override
public Object frozenCopy(Map<Object, Object> refs) {
throw new UnsupportedOperationException();
return this;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public Object frozenCopy(Map<Object, Object> refs) {
return this;
}

@Override
public void freezeDirect() {
return;
}

@Override
public String toString() {
return BLangConstants.EMPTY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ public BType getType() {

@Override
public Object copy(Map<Object, Object> refs) {
return null;
return this;
}

@Override
public void freezeDirect() {
return;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ public Object copy(Map<Object, Object> refs) {
return this;
}

@Override
public void freezeDirect() {
Copy link
Member

Choose a reason for hiding this comment

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

Please create an issue to track verifying these changes with @irshadnilam's changes.

return;
}

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@

import javax.xml.XMLConstants;

import static org.ballerinalang.model.tree.NodeKind.INVOCATION;
import static org.wso2.ballerinalang.compiler.desugar.AnnotationDesugar.ANNOTATION_DATA;
import static org.wso2.ballerinalang.compiler.util.Constants.DESUGARED_MAPPING_CONSTR_KEY;

Expand All @@ -192,6 +193,7 @@ public class BIRGen extends BLangNodeVisitor {
new CompilerContext.Key<>();

public static final String DEFAULT_WORKER_NAME = "default";
public static final String CLONE_READ_ONLY = "cloneReadOnly";
private BIRGenEnv env;
private Names names;
private final SymbolTable symTable;
Expand Down Expand Up @@ -625,8 +627,10 @@ public void visit(BLangAnnotationAttachment astAnnotAttach) {
this.env.enclAnnotAttachments.add(annotAttachment);
}

private boolean isCompileTimeAnnotationValue(BLangExpression expr) {
// TODO Compile time literal constants
private boolean isCompileTimeAnnotationValue(BLangExpression expression) {

BLangExpression expr = unwrapAnnotationExpressionFromCloneReadOnly(expression);

switch (expr.getKind()) {
case LITERAL:
case NUMERIC_LITERAL:
Expand Down Expand Up @@ -672,7 +676,18 @@ private boolean isCompileTimeAnnotationValue(BLangExpression expr) {
}
}

private BIRAnnotationValue createAnnotationValue(BLangExpression expr) {
private BLangExpression unwrapAnnotationExpressionFromCloneReadOnly(BLangExpression expr) {
if (expr.getKind() == INVOCATION) {
BLangInvocation invocation = (BLangInvocation) expr;
if (invocation.name.getValue().equals(CLONE_READ_ONLY)) {
return invocation.expr;
}
}
return expr;
}

private BIRAnnotationValue createAnnotationValue(BLangExpression expression) {
BLangExpression expr = unwrapAnnotationExpressionFromCloneReadOnly(expression);
// TODO Compile time literal constants
switch (expr.getKind()) {
case LITERAL:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,9 @@ public void visit(BLangAnnotation annotationNode) {

public void visit(BLangAnnotationAttachment annAttachmentNode) {
annAttachmentNode.expr = rewrite(annAttachmentNode.expr, env);
if (annAttachmentNode.expr != null) {
annAttachmentNode.expr = visitCloneReadonly(annAttachmentNode.expr, annAttachmentNode.expr.type);
}
result = annAttachmentNode;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,13 @@
import org.wso2.ballerinalang.compiler.semantics.model.types.BFutureType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BIntersectionType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BInvokableType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BMapType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BObjectType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BRecordType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BServiceType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BStructureType;
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;
Expand Down Expand Up @@ -1360,22 +1363,85 @@ private boolean isValidAnnotationType(BType type) {
}

switch (type.tag) {
case TypeTags.RECORD:
// BRecordType recordType = (BRecordType) type;
// return recordType.fields.stream().allMatch(field -> types.isAnydata(field.type)) &&
// (recordType.sealed || types.isAnydata(recordType.restFieldType));
case TypeTags.MAP:
// return types.isAnydata(((BMapType) type).constraint);
return true;
BType constraintType = ((BMapType) type).constraint;
return isAnyDataOrReadOnlyTypeSkippingObjectType(constraintType);
case TypeTags.RECORD:
BRecordType recordType = (BRecordType) type;
for (BField field : recordType.fields.values()) {
if (!isAnyDataOrReadOnlyTypeSkippingObjectType(field.type)) {
return false;
}
}

BType recordRestType = recordType.restFieldType;
if (recordRestType == null || recordRestType == symTable.noType) {
return true;
}

return isAnyDataOrReadOnlyTypeSkippingObjectType(recordRestType);
case TypeTags.ARRAY:
BType elementType = ((BArrayType) type).eType;
return (elementType.tag == TypeTags.MAP || elementType.tag == TypeTags.RECORD) &&
isValidAnnotationType(elementType);
if ((elementType.tag == TypeTags.MAP) || (elementType.tag == TypeTags.RECORD)) {
return isValidAnnotationType(elementType);
}
return false;
}

return types.isAssignable(type, symTable.trueType);
}

private boolean isAnyDataOrReadOnlyTypeSkippingObjectType(BType type) {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we create an issue to track fixing std lib deviations and add back the object validation to annotations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue #24217

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created an issue to do language changes after StandardLib changes: #24246

if (type == symTable.semanticError) {
return false;
}
switch (type.tag) {
case TypeTags.OBJECT:
return true;
case TypeTags.RECORD:
BRecordType recordType = (BRecordType) type;
for (BField field : recordType.fields.values()) {
if (!isAnyDataOrReadOnlyTypeSkippingObjectType(field.type)) {
return false;
}
}
BType recordRestType = recordType.restFieldType;
if (recordRestType == null || recordRestType == symTable.noType) {
return true;
}
return isAnyDataOrReadOnlyTypeSkippingObjectType(recordRestType);
case TypeTags.MAP:
BType constraintType = ((BMapType) type).constraint;
return isAnyDataOrReadOnlyTypeSkippingObjectType(constraintType);
case TypeTags.UNION:
for (BType memberType : ((BUnionType) type).getMemberTypes()) {
if (!isAnyDataOrReadOnlyTypeSkippingObjectType(memberType)) {
return false;
}
}
return true;
case TypeTags.TUPLE:
BTupleType tupleType = (BTupleType) type;
for (BType tupMemType : tupleType.getTupleTypes()) {
if (!isAnyDataOrReadOnlyTypeSkippingObjectType(tupMemType)) {
return false;
}
}
BType tupRestType = tupleType.restType;
if (tupRestType == null) {
return true;
}
return isAnyDataOrReadOnlyTypeSkippingObjectType(tupRestType);
case TypeTags.TABLE:
return isAnyDataOrReadOnlyTypeSkippingObjectType(((BTableType) type).constraint);
case TypeTags.ARRAY:
return isAnyDataOrReadOnlyTypeSkippingObjectType(((BArrayType) type).getElementType());
}

return types.isAssignable(type, symTable.anydataOrReadOnlyType);
}


/**
* Visit each compilation unit (.bal file) and add each top-level node
* in the compilation unit to the package node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public class SymbolTable {

public final BType semanticError = new BType(TypeTags.SEMANTIC_ERROR, null);
public final BType nullSet = new BType(TypeTags.NULL_SET, null);
public final BUnionType anydataOrReadOnlyType = BUnionType.create(null, anydataType, readonlyType);

public BType streamType = new BStreamType(TypeTags.STREAM, anydataType, null, null);
public BType tableType = new BTableType(TypeTags.TABLE, anydataType, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,8 @@ error.annotation.attachment.cannot.specify.multiple.values=\
cannot specify more than one annotation value for annotation ''{0}''

error.annotation.invalid.type=\
annotation declaration requires a subtype of ''true'', ''map<anydata>'' or ''map<anydata>[]'', but found ''{0}''
annotation declaration requires a subtype of ''true'', ''map<anydata|readonly>'' or ''map<anydata|readonly>[]'', but \
found ''{0}''

error.annotation.invalid.const.type=\
invalid type ''{0}'' for ''const'' annotation declaration, expected ''anydata''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ public Object[][] dataProvider() {
// {"annotationAccessExpression4.json", "annotation"},
{"annotationAccessExpression5.json", "annotation"},
{"annotationAccessExpression6.json", "annotation"},
{"annotationBodyCompletion1.json", "annotation"},
{"annotationBodyCompletion2.json", "annotation"},
// {"annotationBodyCompletion1.json", "annotation"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracking issue: #24131

// {"annotationBodyCompletion2.json", "annotation"},
{"annotationBodyCompletion3.json", "annotation"},
{"annotationBodyCompletion4.json", "annotation"},
{"annotationBodyCompletion5.json", "annotation"},
{"annotationBodyCompletion6.json", "annotation"},
// {"annotationBodyCompletion6.json", "annotation"},
// {"annotationInSameModule1.json", "annotation"},
// {"annotationInSameModule2.json", "annotation"},
// {"annotationInSameModule3.json", "annotation"},
Expand Down
2 changes: 1 addition & 1 deletion stdlib/grpc/src/main/ballerina/src/grpc/annotation.bal
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public annotation GrpcServiceConfig ServiceConfig on service;
# + descMap - Service dependent descriptor map, which should be set at the compile time
public type ServiceDescriptorData record {|
string descriptor = "";
map<any> descMap = {};
map<anydata> descMap = {};
KRVPerera marked this conversation as resolved.
Show resolved Hide resolved
|};

# Service descriptor annotation. This is for internal use.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public type QueueConfiguration record {|
boolean durable = false;
boolean exclusive = false;
boolean autoDelete = true;
map<any>? arguments = ();
map<anydata>? arguments = ();
aashikam marked this conversation as resolved.
Show resolved Hide resolved
|};

# Configurations used to declare an exchange.
Expand All @@ -83,7 +83,7 @@ public type ExchangeConfiguration record {|
ExchangeType exchangeType = DIRECT_EXCHANGE;
boolean durable = false;
boolean autoDelete = false;
map<any>? arguments = ();
map<anydata>? arguments = ();
|};

# Configurations used to create a `rabbitmq:Connection`.
Expand Down
Loading