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

[Bug]: Toml quoted keys are not parsed properly #40278

Open
sameerajayasoma opened this issue Apr 21, 2023 · 5 comments
Open

[Bug]: Toml quoted keys are not parsed properly #40278

sameerajayasoma opened this issue Apr 21, 2023 · 5 comments
Assignees
Labels
Area/Configurable Runtime configurable related issues Area/TOMLParser TOML parser related issues needTriage The issue has to be inspected and labeled manually Priority/High Type/Bug userCategory/Compilation
Milestone

Comments

@sameerajayasoma
Copy link
Contributor

Description

For each toml quoted key, I can see additional double quotes around it.

[{"basePath":"/api/entitlement-service","url":["https://abc.com/api/entitlement-service"],"config":{""foo.bar.baz"":"value"}}]

Config.toml

[[endpoints]]
basePath = "/api/entitlement-service"
url = ["https://abc.com/api/entitlement-service"]
config = {"foo.bar.baz" = "value"}

Steps to Reproduce

Use the below code snippet

type Endpoint record {|
    string basePath;
    string[] url;
    map<string> config;
|};

configurable Endpoint[] endpoints = [];

Config.toml

[[endpoints]]
basePath = "/api/entitlement-service"
url = ["https://abc.com/api/entitlement-service"]
config = {"foo.bar.baz" = "value"}

Affected Version(s)

2201.5.0

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

@sameerajayasoma sameerajayasoma added Type/Bug Area/TOMLParser TOML parser related issues Area/Configurable Runtime configurable related issues labels Apr 21, 2023
@sameerajayasoma sameerajayasoma added this to the 2201.5.1 milestone Apr 21, 2023
@ballerina-bot ballerina-bot added needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Apr 21, 2023
@sameerajayasoma
Copy link
Contributor Author

This will be a blocker pretty soon. We need this to implement service registration and discovery proposal.

@xlight05
Copy link
Contributor

xlight05 commented May 2, 2023

Had a look at this today.
From toml parsers side, As I remember, adding the quotes into quoted keys was intentional. This is due to complexities when retrieving via dotted keys.

Consider the following sample -

"foo.bar" = "value"

[foo]
bar = "value1"

Both the entries are different right? First one is a key value par in root level, and 2nd one is inside a table. But when we want to retrieve via dotted keys. we always have to use "foo.bar" for both cases. I believe that's why we made surrounding quotes required when we wanna retrieve quoted strings. This is the same behavior in parsers like Toml4j as well.

However, in Config.toml case this doesn't really matter. Had a chat with @HindujaB today. We'll introduce an API without quotes for the Configurables to use in the AST level.

@HindujaB
Copy link
Contributor

HindujaB commented May 8, 2023

I have a minor concern.
Since we support identifiers with special characters in Ballerina and if we have a configurable variable with quotes as follows,
(This is not yet supported)

configurable string \"foo\.bar\" = ?;

What should be the Config.toml representation?

"foo.bar" = "hello-foo.bar"

or

"\"foo.bar\"" = "hello-foo.bar"

As " in Toml spec is used to enclose toml reserved characters inside a key, I suppose the user has to enclose the entire variable name as option 2. In that case, we can remove the inconsistency from the above example in the issue and separately handle the identifier with special characters.

WDYT?
@sameerajayasoma @warunalakshitha

@warunalakshitha
Copy link
Contributor

warunalakshitha commented May 17, 2023

@HindujaB Option 1 would be more user friendly. But if there are any ambiguities we can support both

@HindujaB
Copy link
Contributor

I agree.
With approach 1, we will need to handle the ambiguities.
For example,

type Rec record {
    string \"foo\.bar\";
    string foo\.bar; 
};

configurable Rec rec = ?;

This can create ambiguity as we can't provide values separately for both fields.

rec = {"foo.bar" = "hello"}

@xlight05 xlight05 modified the milestones: 2201.5.1, 2201.6.1 Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Configurable Runtime configurable related issues Area/TOMLParser TOML parser related issues needTriage The issue has to be inspected and labeled manually Priority/High Type/Bug userCategory/Compilation
Projects
None yet
Development

No branches or pull requests

5 participants