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.

Sign up for GitHub

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

Jump to bottom

Fix several issues with object type reference #18404

Merged
merged 6 commits into from
Sep 2, 2019
Merged

Fix several issues with object type reference #18404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
27f14ac
Disallow referncing two objects with functions with the same name
MaryamZi Aug 31, 2019
ec8d952
Consider param visibilty for function exact match check
MaryamZi Aug 31, 2019
38e3423
Validate object type refs across modules and copy funcs only for vali…
MaryamZi Sep 1, 2019
b4f7b85
Merge branch 'master' of https://github.com/ballerina-lang/ballerina …
MaryamZi Sep 1, 2019
3356dfe
Disallow fields with default values in abstract objects
MaryamZi Sep 1, 2019
ccf5e3d
Fix tests
MaryamZi Sep 1, 2019
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
2 changes: 2 additions & 0 deletions compiler/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public enum DiagnosticCode {
ABSTRACT_OBJECT_FUNCTION_CANNOT_HAVE_BODY("abstract.object.function.cannot.have.body"),
PRIVATE_OBJECT_CONSTRUCTOR("private.object.constructor"),
PRIVATE_FIELD_ABSTRACT_OBJECT("private.field.abstract.object"),
FIELD_WITH_DEFAULT_VALUE_ABSTRACT_OBJECT("field.with.default.value.abstract.object"),
PRIVATE_FUNC_ABSTRACT_OBJECT("private.function.abstract.object"),
EXTERN_FUNC_ABSTRACT_OBJECT("extern.function.abstract.object"),
RESOURCE_FUNCTION_CANNOT_BE_EXTERN("resource.function.cannot.be.extern"),
Expand Down Expand Up @@ -104,6 +105,7 @@ public enum DiagnosticCode {
EXPLICIT_WORKER_CANNOT_BE_DEFAULT("explicit.worker.cannot.be.default"),
INVALID_MULTIPLE_FORK_JOIN_SEND("worker.multiple.fork.join.send"),
INCOMPATIBLE_TYPE_REFERENCE("incompatible.type.reference"),
INCOMPATIBLE_TYPE_REFERENCE_NON_PUBLIC_MEMBERS("incompatible.type.reference.non.public.members"),
INCOMPATIBLE_RECORD_TYPE_REFERENCE("incompatible.record.type.reference"),
REDECLARED_TYPE_REFERENCE("redeclared.type.reference"),
REDECLARED_FUNCTION_FROM_TYPE_REFERENCE("redeclared.function.from.type.reference"),
Expand Down
10 changes: 8 additions & 2 deletions ...ng/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,14 @@ public void visit(BLangObjectTypeNode objectTypeNode) {
boolean isAbstract = objectTypeNode.flagSet.contains(Flag.ABSTRACT);
objectTypeNode.fields.forEach(field -> {
analyzeDef(field, objectEnv);
if (isAbstract && field.flagSet.contains(Flag.PRIVATE)) {
this.dlog.error(field.pos, DiagnosticCode.PRIVATE_FIELD_ABSTRACT_OBJECT, field.symbol.name);
if (isAbstract) {
if (field.flagSet.contains(Flag.PRIVATE)) {
this.dlog.error(field.pos, DiagnosticCode.PRIVATE_FIELD_ABSTRACT_OBJECT, field.symbol.name);
}

if (field.expr != null) {
this.dlog.error(field.expr.pos, DiagnosticCode.FIELD_WITH_DEFAULT_VALUE_ABSTRACT_OBJECT);
}
}
});

Expand Down
48 changes: 39 additions & 9 deletions ...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SymbolEnter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.wso2.ballerinalang.compiler.semantics.model.types.BField;
import org.wso2.ballerinalang.compiler.semantics.model.types.BFutureType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BInvokableType;
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;
Expand Down Expand Up @@ -1284,6 +1285,7 @@ private void defineMembers(List<BLangTypeDefinition> typeDefNodes, SymbolEnv pkg
defineNode(f, objMethodsEnv);
});

List<String> referencedFunctions = new ArrayList<>();
// Add the attached functions of the referenced types to this object.
// Here it is assumed that all the attached functions of the referred type are
// resolved by the time we reach here. It is achieved by ordering the typeDefs
Expand All @@ -1295,7 +1297,7 @@ private void defineMembers(List<BLangTypeDefinition> typeDefNodes, SymbolEnv pkg

List<BAttachedFunction> functions = ((BObjectTypeSymbol) typeRef.type.tsymbol).attachedFuncs;
for (BAttachedFunction function : functions) {
defineReferencedFunction(typeDef, objMethodsEnv, typeRef, function);
defineReferencedFunction(typeDef, objMethodsEnv, typeRef, function, referencedFunctions);
}
}
} else if (typeDef.symbol.kind == SymbolKind.RECORD) {
Expand Down Expand Up @@ -1688,6 +1690,7 @@ private void createDummyTypeDefSymbol(TypeDefinition typeDefNode, SymbolEnv env)

private void resolveReferencedFields(BLangStructureTypeNode structureTypeNode, SymbolEnv typeDefEnv) {
List<BSymbol> referencedTypes = new ArrayList<>();
List<BLangType> invalidTypeRefs = new ArrayList<>();
// Get the inherited fields from the type references
structureTypeNode.referencedFields = structureTypeNode.typeRefs.stream().flatMap(typeRef -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, This is not a good stream implementation. we should refactor this later.

BType referredType = symResolver.resolveTypeNode(typeRef, typeDefEnv);
Expand All @@ -1698,13 +1701,28 @@ private void resolveReferencedFields(BLangStructureTypeNode structureTypeNode, S
// Check for duplicate type references
if (referencedTypes.contains(referredType.tsymbol)) {
dlog.error(typeRef.pos, DiagnosticCode.REDECLARED_TYPE_REFERENCE, typeRef);
invalidTypeRefs.add(typeRef);
return Stream.empty();
}

if (structureTypeNode.type.tag == TypeTags.OBJECT && (referredType.tag != TypeTags.OBJECT || !Symbols
.isFlagOn(referredType.tsymbol.flags, Flags.ABSTRACT))) {
dlog.error(typeRef.pos, DiagnosticCode.INCOMPATIBLE_TYPE_REFERENCE, typeRef);
return Stream.empty();
if (structureTypeNode.type.tag == TypeTags.OBJECT) {
if (referredType.tag != TypeTags.OBJECT ||
!Symbols.isFlagOn(referredType.tsymbol.flags, Flags.ABSTRACT)) {
dlog.error(typeRef.pos, DiagnosticCode.INCOMPATIBLE_TYPE_REFERENCE, typeRef);
invalidTypeRefs.add(typeRef);
return Stream.empty();
}

BObjectType objectType = (BObjectType) referredType;
if (structureTypeNode.type.tsymbol.owner != referredType.tsymbol.owner) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There can be scenario, this logic fails, Like, local type def. Better to validate pkg, instated.

if (objectType.fields.stream().anyMatch(field -> !Symbols.isPublic(field.symbol)) ||
((BObjectTypeSymbol) objectType.tsymbol).attachedFuncs.stream()
.anyMatch(func -> !Symbols.isPublic(func.symbol))) {
dlog.error(typeRef.pos, DiagnosticCode.INCOMPATIBLE_TYPE_REFERENCE_NON_PUBLIC_MEMBERS, typeRef);
invalidTypeRefs.add(typeRef);
return Stream.empty();
}
}
}

if (structureTypeNode.type.tag == TypeTags.RECORD && referredType.tag != TypeTags.RECORD) {
Expand All @@ -1724,15 +1742,23 @@ private void resolveReferencedFields(BLangStructureTypeNode structureTypeNode, S
return var;
});
}).collect(Collectors.toList());
structureTypeNode.typeRefs.removeAll(invalidTypeRefs);
}

private void defineReferencedFunction(BLangTypeDefinition typeDef, SymbolEnv objEnv, BLangType typeRef,
BAttachedFunction referencedFunc) {
BAttachedFunction referencedFunc, List<String> referencedFunctions) {
String referencedFuncName = referencedFunc.funcName.value;
Name funcName = names.fromString(
Symbols.getAttachedFuncSymbolName(typeDef.symbol.name.value, referencedFunc.funcName.value));
Symbols.getAttachedFuncSymbolName(typeDef.symbol.name.value, referencedFuncName));
BSymbol matchingObjFuncSym = symResolver.lookupSymbol(objEnv, funcName, SymTag.VARIABLE);

if (matchingObjFuncSym != symTable.notFoundSymbol) {
if (referencedFunctions.contains(referencedFuncName)) {
dlog.error(typeRef.pos, DiagnosticCode.REDECLARED_SYMBOL, referencedFuncName);
return;
}
referencedFunctions.add(referencedFuncName);

if (Symbols.isFunctionDeclaration(matchingObjFuncSym) && Symbols.isFunctionDeclaration(
referencedFunc.symbol)) {
Optional<BLangFunction> matchingFunc = ((BLangObjectTypeNode) typeDef.typeNode)
Expand Down Expand Up @@ -1791,7 +1817,10 @@ private boolean hasSameFunctionSignature(BInvokableSymbol attachedFuncSym, BInvo

List<BVarSymbol> params = referencedFuncSym.params;
for (int i = 0; i < params.size(); i++) {
if (!params.get(i).name.value.equals(attachedFuncSym.params.get(i).name.value)) {
BVarSymbol referencedFuncParam = params.get(i);
BVarSymbol attachedFuncParam = attachedFuncSym.params.get(i);
if (!referencedFuncParam.name.value.equals(attachedFuncParam.name.value) ||
!hasSameVisibilityModifier(referencedFuncParam.flags, attachedFuncParam.flags)) {
return false;
}
}
Expand Down Expand Up @@ -1822,7 +1851,8 @@ private String getCompleteFunctionSignature(BInvokableSymbol funcSymbol) {
signatureBuilder.append(visibilityModifier).append("function ")
.append(funcSymbol.name.value.split("\\.")[1]);

funcSymbol.params.forEach(param -> paramListBuilder.add(param.type.toString() + " " + param.name.value));
funcSymbol.params.forEach(param -> paramListBuilder.add(
(Symbols.isPublic(param) ? "public " : "") + param.type.toString() + " " + param.name.value));

if (funcSymbol.restParam != null) {
paramListBuilder.add(((BArrayType) funcSymbol.restParam.type).eType.toString() + "... " +
Expand Down
6 changes: 6 additions & 0 deletions compiler/ballerina-lang/src/main/resources/compiler.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,9 @@ error.private.object.constructor=\
error.private.field.abstract.object=\
abstract object field: ''{0}'' can not be declared as private

error.field.with.default.value.abstract.object=\
fields with default values are not yet supported with abstract objects

error.private.function.abstract.object=\
interface function: ''{0}'' of abstract object ''{1}'' can not be declared as private

Expand All @@ -742,6 +745,9 @@ error.extern.function.abstract.object=\
error.incompatible.type.reference=\
incompatible types: ''{0}'' is not an abstract object

error.incompatible.type.reference.non.public.members=\
incompatible type reference ''{0}'': a referenced object cannot have non-public fields or methods

error.incompatible.record.type.reference=\
incompatible types: ''{0}'' is not a record

Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/delay-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public type DelayWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/external-time-batch-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public type ExternalTimeBatchWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/external-time-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public type ExternalTimeWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/hopping-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public type HoppingWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/length-batch-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public type LengthBatchWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/length-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public type LengthWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/sort-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public type SortWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/time-accumulating-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public type TimeAccumulatingWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/time-batch-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public type TimeBatchWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/time-length-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public type TimeLengthWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/time-order-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public type TimeOrderWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/time-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public type TimeWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/streams/src/main/ballerina/src/streams/window/unique-length-window.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public type UniqueLengthWindow object {
public function getCandidateEvents(
StreamEvent originEvent,
(function (map<anydata> e1Data, map<anydata> e2Data) returns boolean)? conditionFunc,
boolean isLHSTrigger = true)
public boolean isLHSTrigger = true)
returns @tainted [StreamEvent?, StreamEvent?][] {
[StreamEvent?, StreamEvent?][] events = [];
int i = 0;
Expand Down
2 changes: 1 addition & 1 deletion stdlib/transactions/src/main/ballerina/src/transactions/2pc_participant.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public type UProtocol LocalProtocol|RemoteProtocol;

type Participant abstract object {

string participantId = "";
string participantId;

function prepare(string protocol) returns [(PrepareResult|error)?, Participant];

Expand Down
6 changes: 4 additions & 2 deletions .../jballerina-unit-test/src/test/java/org/ballerinalang/test/object/AbstractObjectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void setup() {
public void testAbstractObjectNegative() {
CompileResult negativeResult = BCompileUtil.compile("test-src/object/abstract-object-negative.bal");
int index = 0;
Assert.assertEquals(negativeResult.getErrorCount(), 11);
Assert.assertEquals(negativeResult.getErrorCount(), 12);
BAssertUtil.validateError(negativeResult, index++, "cannot initialize abstract object 'Person1'", 3, 18);
BAssertUtil.validateError(negativeResult, index++, "cannot initialize abstract object 'Person2'", 4, 18);
BAssertUtil.validateError(negativeResult, index++, "cannot initialize abstract object 'Person1'", 8, 18);
Expand All @@ -61,8 +61,10 @@ public void testAbstractObjectNegative() {
"'Person6' can not be declared as private", 61, 5);
BAssertUtil.validateError(negativeResult, index++, "interface function: 'getName' of abstract object 'Foo' " +
"can not be declared as private", 65, 5);
BAssertUtil.validateError(negativeResult, index,
BAssertUtil.validateError(negativeResult, index++,
"external function: 'getName' not allowed in abstract object 'Person7'", 78, 5);
BAssertUtil.validateError(negativeResult, index,
"fields with default values are not yet supported with abstract objects", 83, 18);
}

@Test
Expand Down
Loading