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

Client: Guarantee case-insensitive name uniqueness? #633

Closed
kubukoz opened this issue Nov 15, 2020 · 3 comments · Fixed by #644
Closed

Client: Guarantee case-insensitive name uniqueness? #633

kubukoz opened this issue Nov 15, 2020 · 3 comments · Fixed by #644
Labels
bug Something isn't working good first issue Good for newcomers tools Issue related to Caliban tools like code generation or schema comparison

Comments

@kubukoz
Copy link
Contributor

kubukoz commented Nov 15, 2020

Hi!

I tried to regenerate the sources for the GitLab API again (for https://github.com/kubukoz/caliban-gitlab/), but noticed I'm getting compilation warnings:

[E]      Generated class io.pg.gitlab.graphql$IssueSort$UPDATED_ASC$ differs only in case from io.pg.gitlab.graphql$IssueSort$updated_asc$.
[E]        Such classes will overwrite one another on case-insensitive filesystems.
[E]      L961:     case object UPDATED_ASC extends IssueSort
[E]                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

and some more.

Context:

object IssueSort {
    case object updated_desc extends IssueSort
    case object updated_asc extends IssueSort //here
    case object created_desc extends IssueSort
    case object created_asc extends IssueSort
    case object UPDATED_DESC extends IssueSort
    case object UPDATED_ASC extends IssueSort //here
    case object CREATED_DESC extends IssueSort
    case object CREATED_ASC extends IssueSort
    case object PRIORITY_ASC extends IssueSort
    case object PRIORITY_DESC extends IssueSort
    case object LABEL_PRIORITY_ASC extends IssueSort
    case object LABEL_PRIORITY_DESC extends IssueSort
    case object MILESTONE_DUE_ASC extends IssueSort
    case object MILESTONE_DUE_DESC extends IssueSort
    case object DUE_DATE_ASC extends IssueSort
    case object DUE_DATE_DESC extends IssueSort
    case object RELATIVE_POSITION_ASC extends IssueSort
    case object SEVERITY_ASC extends IssueSort
    case object SEVERITY_DESC extends IssueSort
    case object WEIGHT_ASC extends IssueSort
    case object WEIGHT_DESC extends IssueSort
    case object PUBLISHED_ASC extends IssueSort
    case object PUBLISHED_DESC extends IssueSort
    case object SLA_DUE_AT_ASC extends IssueSort
    case object SLA_DUE_AT_DESC extends IssueSort
// ...
}

The API in question: https://gitlab.com/api/graphql

As you can see in [https://docs.gitlab.com/ee/api/graphql/reference/index.html#issuesort](the documentation for this type), the lowercase values have been deprecated, so in theory shouldn't appear in the responses.

Still, it would be great to avoid generating classes / objects with the same name. Do you think this is something the Caliban generator should take care of?

Reproduction at https://github.com/kubukoz/caliban-gitlab/tree/da346a5440cb62b93f09d12c72e0718f3e34c626

@kubukoz kubukoz changed the title Guarantee case-insensitive name uniqueness? Client: Guarantee case-insensitive name uniqueness? Nov 15, 2020
@ghostdogpr
Copy link
Owner

Interesting, I haven't considered that before. This is valid as per the spec so the codegen should take care of that and add a suffix to avoid any clash.

It's a little tricky because the type name can be referenced somewhere else (e.g. an interface) but it surely can be done. Probably the best way is to add an entry to a set whenever we write a new type and deal with the conflicts there.

@ghostdogpr ghostdogpr added bug Something isn't working good first issue Good for newcomers tools Issue related to Caliban tools like code generation or schema comparison ziohackathon labels Nov 15, 2020
@javimartinez
Copy link
Contributor

I would like to work on this one

@ghostdogpr
Copy link
Owner

@kubukoz in addition to fixing this, I added an issue for the option to exclude deprecated fields #645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers tools Issue related to Caliban tools like code generation or schema comparison
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants