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: Indent of generated code for non-required lists. Thanks @sfowl! #1050

Merged
merged 3 commits into from
Jun 15, 2024

Conversation

sfowl
Copy link
Contributor

@sfowl sfowl commented Jun 7, 2024

The _transform macro is called at three different places in this file, but one location was not similarly indented as the other two. This caused errors in the ruff post_hook like:

error: Failed to parse foo/models/foo_request.py:125:9: Expected an indented block after `if` statement

The generated code without the indent fix looks like:

        if not isinstance(self.foo, Unset):
        _temp_foo = []  # <-- incorrect indent
            for foo_item_data in self.foo:
                foo_item: Union[None, int]
                foo_item = foo_item_data
                _temp_foo.append(foo_item)
            foo = (None, json.dumps(_temp_foo).encode(), 'application/json')

@sfowl
Copy link
Contributor Author

sfowl commented Jun 7, 2024

I believe this bug was introduced by #926

@dbanty
Copy link
Collaborator

dbanty commented Jun 8, 2024

Can you provide a minimal OpenAPI schema which triggers this issue? That way we can make sure we're exercising that code path in tests to prevent this regression.

@sfowl sfowl force-pushed the fix-transform-indent branch from 2b821b1 to ad3192b Compare June 10, 2024 01:35
@sfowl
Copy link
Contributor Author

sfowl commented Jun 10, 2024

I've modified the baseline spec in the end to end tests to cover this case, and regenerated the snapshots with pdm regen.

@dbanty dbanty changed the title Fix indent of _transform macro in list_property.py.jinja fix: Indent of generated code for non-required lists. Thanks @sfowl! Jun 15, 2024
Copy link
Collaborator

@dbanty dbanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks so much!

@dbanty dbanty enabled auto-merge June 15, 2024 18:07
@dbanty dbanty added this pull request to the merge queue Jun 15, 2024
Merged via the queue into openapi-generators:main with commit 39b9db2 Jun 15, 2024
20 checks passed
dbanty added a commit that referenced this pull request Jun 15, 2024
This PR was created by Knope. Merging it will create a new release

### Features

#### Support request body refs

You can now define and reuse bodies via refs, with a document like this:

```yaml
paths:
  /something:
    post:
      requestBody:
        "$ref": "#/components/requestBodies/SharedBody"
components:
  requestBodies:
    SharedBody:
      content:
        application/json:
          schema:
            type: string
```

Thanks to @kigawas and @supermihi for initial implementations and
@RockyMM for the initial request.

Closes #633, closes #664, resolves #595.

### Fixes

- Indent of generated code for non-required lists. Thanks @sfowl!
(#1050)
- Parsing requestBody with $ref (#633)

Co-authored-by: GitHub <[email protected]>
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.

2 participants