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 included object's functions have to have the same parameter names #42667

Closed
nirmal070125 opened this issue Apr 30, 2024 · 3 comments · Fixed by #43197
Closed

Type included object's functions have to have the same parameter names #42667

nirmal070125 opened this issue Apr 30, 2024 · 3 comments · Fixed by #43197
Assignees
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. Type/SpecDeviation userCategory/Compilation
Milestone

Comments

@nirmal070125
Copy link
Contributor

nirmal070125 commented Apr 30, 2024

Description

See the below object type;

public type Finder isolated object {
 public isolated function findCodeSystem(r4:uri? system = (), string? id = (), string? version = ()) returns r4:CodeSystem|r4:FHIRError;
}

when I type included this in another object and changed the name of a parameter like below;

isolated class InmemoryTerminology {
    *Finder;
   public isolated function findCodeSystem(r4:uri? systems, string? id, string? version) returns r4:CodeSystem|r4:FHIRError {
}

Ballerina complains saying;

mismatched function signatures: expected 'public function findCodeSystem(ballerinax/health.fhir.r4:4.3.4:uri? system, string? id, string? version) returns (ballerinax/health.fhir.r4:4.3.4:CodeSystem|ballerinax/health.fhir.r4:4.3.4:FHIRError)', found 'public function findCodeSystem(ballerinax/health.fhir.r4:4.3.4:uri? systems, string? id, string? version) returns (ballerinax/health.fhir.r4:4.3.4:CodeSystem|ballerinax/health.fhir.r4:4.3.4:FHIRError)'(BCE2094)
ballerinax/health.fhir.r4:4.3.4:uri? systems

ballerinax/health.fhir.r4:4.3.4:uri? systems

Steps to Reproduce

No response

Affected Version(s)

Ballerina 2201.8.4 (Swan Lake Update 8)

OS, DB, other environment details and versions

No response

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@ballerina-bot ballerina-bot added needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Apr 30, 2024
@SasinduDilshara
Copy link
Contributor

SasinduDilshara commented May 2, 2024

In Ballerina compiler function argument is represented a combination of argument type and the argument name (In java it is only type).

Following is a valid use case that used the argument name.

type Data record {|int a; string c; boolean b;|};

function foo(int a, string c, boolean b) {
}

public function main() {
    Data data = {a: 1, c: "hello", b: true};
    foo(...data);
}

In the above code we support ...data as the function argument, So Ballerina will map the each record field with function argument by mapping the record field name with function argument name.

@MaryamZi MaryamZi added Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. and removed needTriage The issue has to be inspected and labeled manually labels May 20, 2024
@MaryamZi
Copy link
Member

I'm not sure if the current restriction is required in the spec, I've created a spec issue for this.

In Ballerina compiler function argument is represented a combination of argument type and the argument name (In java it is only type).

Following is a valid use case that used the argument name.

type Data record {|int a; string c; boolean b;|};

function foo(int a, string c, boolean b) {
}

public function main() {
    Data data = {a: 1, c: "hello", b: true};
    foo(...data);
}

In the above code we support ...data as the function argument, So Ballerina will map the each record field with function argument by mapping the record field name with function argument name.

Parameter names don't affect typing. The argument list is always constructed at the call site and is based on the function-reference of the function-call-expr, so this isn't an issue. E.g., modifying your example slightly

import ballerina/io;

type Data record {|int a; string c; boolean b;|};

function foo(int x, string y, boolean z) {
    io:println("x: ", x);
    io:println("y: ", y);
    io:println("z: ", z);
}

public function main() {
    Data data = {a: 1, c: "hello", b: true};

    // Allowed since parameter names don't affect typing.
    function (int a, string c, boolean b) fn = foo;

    // Argument list is constructed as
    // `(1, "hello", true)` in `fn(1, "hello", true)`
    fn(...data);

    // Disallowed since parameter types don't match by position,
    // irrespective of names.
    // error: expected 'function (string,int,boolean) returns ()', 
    //          found 'function (int,string,boolean) returns ()'
    // function (string y, int x, boolean z) _ = foo;
}
x: 1
y: hello
z: true

@MaryamZi
Copy link
Member

MaryamZi commented Jul 8, 2024

We can get rid of this restriction - ballerina-platform/ballerina-spec#1304

@MaryamZi MaryamZi self-assigned this Jul 30, 2024
@MaryamZi MaryamZi moved this to In Progress in Ballerina Team Main Board Jul 30, 2024
@MaryamZi MaryamZi moved this from In Progress to PR Sent in Ballerina Team Main Board Jul 30, 2024
@MaryamZi MaryamZi added this to the 2201.10.0 milestone Jul 31, 2024
@MaryamZi MaryamZi changed the title [Bug]: Type included object's functions have to have the same parameter names Type included object's functions have to have the same parameter names Jul 31, 2024
@github-project-automation github-project-automation bot moved this from PR Sent to Done in Ballerina Team Main Board Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. Type/SpecDeviation userCategory/Compilation
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants