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

Fix invalid if let code generation #1037

Merged
merged 1 commit into from May 14, 2024

Conversation

GuillaumeGomez
Copy link
Collaborator

@GuillaumeGomez GuillaumeGomez commented May 12, 2024

Fixes #1033.

We cannot test the generated code since the if-let chain feature is still unstable. So I thought about two possibilities to still test this:

  • Testing the generated code.
  • Adding a new "nightly" feature for these tests and add a nightly check in the CI.

Here are the cases to be checked:

{% if let Some(query) = s && !query.is_empty() %}{{query}}{% endif %}

{% if let Some(s) = s && !s.is_empty() %}{{s}}{% endif %}

{% if let Some(s) = s %}{{ s }}{% endif %}

Where s is Option<String>.

@Kijewski
Copy link
Collaborator

The change looks good to me. 👍

Currently we test on stable and beta. Maybe we could add nightly, too, and enable the tests only on nightly?

@GuillaumeGomez
Copy link
Collaborator Author

After thinking some more about it, I don't think checking with nightly will be what we want: we want to check the generated code here, not that it works on nightly.

I'll open an issue to discuss about how to test generated content because I think it's a problem that we need to solve somehow.

@GuillaumeGomez GuillaumeGomez merged commit 4d18725 into djc:main May 14, 2024
16 checks passed
@GuillaumeGomez GuillaumeGomez deleted the fix-if-let branch May 14, 2024 12:51
@djc
Copy link
Owner

djc commented May 14, 2024

Maybe the generator should desugar let chains into something that works on stable? It's okay if the generated code isn't the prettiest.

@GuillaumeGomez
Copy link
Collaborator Author

I don't think we should implement our own condition lexer, we should just rely on the rust compiler to handle this part for us. For example, it becomes particularly tricky in cases like:

if let Some(x) = y || z == "x" && whatever

It requires to generate else if z == "x" and in both if branches to have also whatever.

@GuillaumeGomez
Copy link
Collaborator Author

I opened #1038 about this.

@djc
Copy link
Owner

djc commented May 14, 2024

(Would prefer if you don't merge non-trivial PRs without my approval.)

@GuillaumeGomez
Copy link
Collaborator Author

Sorry, noted for future contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

if let chain has a weird behaviour
3 participants