ballerina-platform / ballerina-lang

The Ballerina Programming Language
https://ballerina.io/
Apache License 2.0
3.58k stars 738 forks source link

[Bindgen] Module level mappings could lead to cyclic dependencies #31638

Open IrushiL opened 3 years ago

IrushiL commented 3 years ago

Description: When generating module-level mappings Java packages are mapped onto Ballerina modules. However, doing this could cause cyclic dependencies since Java packages are allowed to make cyclic imports within the Java module they reside in and since it supports dynamic class loading.

This design issue should be sorted out if we are to support module-level mappings for Ballerina bindings. The alternative approach will be to let users use the single directory mappings.

Steps to reproduce: Assume there are 2 packages org.test.first and org.test.second and that these contain the following classes.

org.test.first.A 
org.test.first.B
org.test.second.A
org.test.second.B

Imagine a scenario where the user tries to import org.test.second.A in org.test.first.A and org.test.second.B in org.test.first.B. Even though this would be a valid import in Java, this would cause cyclic imports in Ballerina bindings.

Affected Versions: Ballerina Swan Lake Beta 2

OS, DB, other environment details and versions:

Related Issues (optional): https://github.com/ballerina-platform/ballerina-lang/issues/31855

Suggested Labels (optional):

Suggested Assignees (optional):

sameerajayasoma commented 3 years ago

Random thought: Can we create a Ballerina module per Java Class?

IrushiL commented 3 years ago

In order to sort this design issue, we could generate a single Ballerina module per Java library, since we can make the assumption that Java modules do not have cyclic dependencies among them.

If we are to go ahead with this approach, the following design changes will have to be done.

Current Approach:

E.g.

import yaml_package.java.io as javaio;
import yaml_package.java.lang as javalang;
import yaml_package.org.yaml.snakeyaml as snakeyaml;

function foo() {
    snakeyaml:Yaml yaml = snakeyaml:newYaml1();
}

Proposed Approach:

  1. If it is possible to determine a simplified name that would represent the fully qualified class name, like in the case of default Java packages or by using the artifact id, we could use a unique simplified prefix before the simple class name.

E.g.

javaio_FileInputStream
javalang_String
snakeyaml_Yaml
  1. The class names need to be changed similar to the file names, we could use the same unique prefix in front of the simple class name.

E.g.

function foo() {
    snakeyaml_Yaml yaml = snakeyaml_newYaml1();
}
  1. The rest of the implementation should ideally be unaltered.
IrushiL commented 2 years ago

The workaround would be to generate single directory mappings using the (-o|--output) <output> option in the bindgen command.