8000 String type loss when encoding with SingleLineStringStyle.Plain · Issue #367 · charleskorn/kaml · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

String type loss when encoding with SingleLineStringStyle.Plain #367

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
Closed
russellbanks opened this issue Jan 11, 2023 · 5 comments · Fixed by #371
Closed

String type loss when encoding with SingleLineStringStyle.Plain #367

russellbanks opened this issue Jan 11, 2023 · 5 comments · Fixed by #371

Comments

@russellbanks
Copy link
Contributor
russellbanks commented Jan 11, 2023 •
edited
Loading

Describe the bug

Encoding a string like "123" into Yaml when singleLineStringStyle is set to SingleLineStringStyle.Plain makes its associated type in Yaml if it wasn't a string. For example, "123" becomes 123 as an Int, "123.1" becomes 123.1 as a Float, and "true" becomes true as a Boolean. This should become quoted (single or double) when this is the case, regardless of the SingleLineStringStyle.

Reproduction repo

https://github.com/russellbanks/Komac

Steps to reproduce

  1. Create a YamlConfiguration with plain single line string style: YamlConfiguration(singleLineStringStyle = SingleLineStringStyle.Plain)
  2. Encode a string like "123" into Yaml with this configuration
  3. The string will be encoded as an an Integer rather than a String, and not escaped with quotes

Expected behaviour

# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.4.0.schema.json

PackageIdentifier: Package.Identifier
PackageVersion: "123" # or '123'
ManifestVersion: 1.4.0

Actual behaviour

# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.4.0.schema.json

PackageIdentifier: Package.Identifier
PackageVersion: 123 # Incorrect type. Expected "string". yaml-schema: https://aka.ms/winget-manifest.installer.1.4.0.schema.json
ManifestVersion: 1.4.0

Version information

0.49.0

Any other information

https://stackoverflow.com/a/22235064:

  • Use quotes to force a string, e.g. if your key or value is 10 but you want it to return a String and not a Fixnum, write '10' or "10".
@charleskorn
Copy link
Owner

Thanks for the report @russellbanks. Does this cause issues parsing the values back from YAML with kaml? Or is it just that winget rejects them?

@russellbanks
Copy link
Contributor Author
russellbanks commented Jan 14, 2023 •
edited
Loading

Although I know that the data I receive will have it escaped, no, there are no issues with decoding data like this:

val input = """
    PackageIdentifier: Package.Identifier
    PackageVersion: 123 # This would be escaped in a normal scenario as it's a String
    ManifestType: installer
    ManifestVersion: 1.4.0
""".trimIndent()

// YamlConfig.default is a helper object that sets the SingleLineStringStyle to Plain
val result = YamlConfig.default.decodeFromString(InstallerManifest.serializer(), input)

Despite the PackageVersion here technically being an Int, it parses correctly into a String.

Or is it just that winget rejects them?

Their schema declares that PackageVersion should be of type String. This is reflected in my data class:

@Serializable
data class InstallerManifest(
    @SerialName("PackageIdentifier") val packageIdentifier: String,
    @SerialName("PackageVersion") val packageVersion: String,
    ...
    @SerialName("ManifestType") val manifestType: String,
    @SerialName("ManifestVersion") val manifestVersion: String
) {
...
}

The issue is that the value is not escaped if it is not a String (e.g. "123" being an Int without "" or '') in the resulting Yaml

It should be noted that it does not have to be escaped if it is unambigiously a String. For example, 1.38.2 is a String in Yaml and does not have to be escaped, but 1.38 is a Float, and does have to be escaped. I use the Yaml extension by RedHat in VSCode to test this (it can read schemas in the header of the Yaml file, which is how I was able to get the type mismatch error in my original comment).

@charleskorn
Copy link
Owner

If the only error you're seeing is from the schema validation tool in VS Code and winget happily parses the YAML as-is, then this is likely due to a limitation in the schema validator rather than nonconformance from kaml.

The schema validator uses JSON Schemas, and these do not understand YAML's looser typing rules or how tolerant the parsing application is.

@russellbanks
Copy link
Contributor Author
russellbanks commented Jan 15, 2023 •
edited
Loading

I've taken a look at some other manifests in winget-pkgs and they're still fine without being escaped. It does make sense to have the option to escape them though as even if it's being strict about it. The output in Yaml is not technically a String despite it being declared as such in my data class.

@charleskorn
Copy link
Owner
charleskorn commented Jan 15, 2023 •
edited
Loading

It does make sense to have the option to escape them though as even if it's being strict about it.

If you're open to submitting a PR for this, I'd happily review it. I'd suggest adding another value to SingleLineStringStyle for this - perhaps PlainExceptAmbiguous? (I don't love that name, open to other ideas :) )

@russellbanks russellbanks mentioned this issue Jan 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add SingleLineStringStyle.PlainExceptAmbiguous and AmbiguousEscapeStyle
2 participants
@charleskorn @russellbanks
0