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.

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

include_all_inputs = false leads to IndexError in isort #308

Open
weblab-misha opened this issue Aug 13, 2024 · 7 comments · May be fixed by #337
Open

include_all_inputs = false leads to IndexError in isort #308

weblab-misha opened this issue Aug 13, 2024 · 7 comments · May be fixed by #337

Comments

@weblab-misha
Copy link

weblab-misha commented Aug 13, 2024 •
edited
Loading

ariadne-codegen --config ariadne-codegen.toml runs fine unless I add include_all_inputs = false in my ariadne-codegen.toml file. With include_all_inputs = false command fails with error:

  File "/Users/user/my-project/venv/bin/ariadne-codegen", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/ariadne_codegen/main.py", line 37, in main
    client(config_dict)
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/ariadne_codegen/main.py", line 81, in client
    generated_files = package_generator.generate()
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/ariadne_codegen/client_generators/package.py", line 152, in generate
    self._generate_input_types()
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/ariadne_codegen/client_generators/package.py", line 307, in _generate_input_types
    code = self._add_comments_to_code(ast_to_str(module), self.schema_source)
                                      ^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/ariadne_codegen/utils.py", line 33, in ast_to_str
    return format_str(isort.code(code), mode=Mode())
                      ^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/isort/api.py", line 92, in sort_code_string
    sort_stream(
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/isort/api.py", line 210, in sort_stream
    changed = core.process(
              ^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/isort/core.py", line 422, in process
    parsed_content = parse.file_contents(import_section, config=config)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/my-project/venv/lib/python3.12/site-packages/isort/parse.py", line 522, in file_contents
    if "," in import_string.split(just_imports[-1])[-1]:
                                  ~~~~~~~~~~~~^^^^
IndexError: list index out of range

BTW, nothing is wrong with include_all_enums = false, it doesn't cause any issues.

UPD:

$ ariadne-codegen --version
ariadne-codegen, version 0.14.0
$ isort --version

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.12.0
@weblab-misha
Copy link
Author

I've managed to run ariadne-codegen in VSCode python debugger, this is a screenshot from just before exception is raised:
image

Looks like "line" == "import_string" == 'from .enums import ', so just_imports variable is empty list

This is the same moment of execution, but I've switched to higher function in call stack:

image

And this is ast_to_str() (the same moment of execution):
image

Here is the beginning of code variable:

from typing import Optional, Any, Union, List, Annotated
from pydantic import Field, PlainSerializer
from .base_model import BaseModel
from .base_model import Upload
from .enums import 

class ...

Looks like from .enums import line causes the issue when passed to isort.code().

I tried to create a simple file test.py with a wrong from ... import line:

from .enums import

class A:
    pass

and run isort test.py, but it works fine. Looks like isort CLI handles these issues, so ariadne-codegen may be using isort.code() in a wrong way (?)

@DamianCzajkowski
Copy link
Contributor

Hi @weblab-misha, I know how to fix this issue, but I found it hard to recreate in my own schema. Could you provide a schema example so I can better understand where the issue lies?

@mjpieters
Copy link

mjpieters commented Jan 7, 2025 •
edited
Loading

Hi @weblab-misha, I know how to fix this issue, but I found it hard to recreate in my own schema. Could you provide a schema example so I can better understand where the issue lies?

I'm experiencing this with the Github schema: https://docs.github.com/en/graphql/overview/public-schema

What happens is that my queries use no enums, and you end up with an empty enums.py module.

@mjpieters
Copy link

I'm working around this by using a dummy plugin. This simply removes the from enums import line from generated code:

import isort
from ariadne_codegen.plugins.base import Plugin

orig = isort.code

EMPTY_ENUMS = "from .enums import \n"


def mocked_isort_code(code: str) -> str:
    if EMPTY_ENUMS in code:
        code = "".join([line for line in code.splitlines(True) if line != EMPTY_ENUMS])
    return orig(code)


if isort.code is not mocked_isort_code:
    isort.code = mocked_isort_code


class CustomPlugin(Plugin):
    pass

@mjpieters
Copy link

Here is a better version of the plugin; this simply drops the empty ImportFrom node from the inputs module:

import ast

from ariadne_codegen.plugins.base import Plugin


class RemoveEmptyImportFrom(ast.NodeTransformer):
    def visit_ImportFrom(self, node: ast.ImportFrom) -> ast.ImportFrom | None:
        return node if node.names else None


class RepairInputsModulePlugin(Plugin):
    def generate_inputs_module(self, module: ast.Module) -> ast.Module:
        return RemoveEmptyImportFrom().visit(module)

@mjpieters
Copy link

So, essentially the problem originates here:

ariadne-codegen/ariadne_codegen/client_generators/input_types.py

Lines 84 to 87 in 11bfe35

if self._used_enums:
self._imports.append(
generate_import_from(self.get_used_enums(), self.enums_module, 1)
)

self._used_enums is a map from input type to the enums that that input type references. It is built before the input types are filtered down to those that are actually used, so is very likely to not be empty. However, self.get_used_enums() only returns actually used enums, and that can be an empty list.

Since it's safe to call self.get_used_enums() even if self._used_enums is empty, I'd change it to just getting the used enums and testing if that is non-empty:

        if used_enums := self.get_used_enums():
            self._imports.append(
                generate_import_from(used_enums, self.enums_module, 1)
            )

@mjpieters
Copy link

mjpieters commented Jan 8, 2025 •
edited
Loading

Looking into a PR for this, I thought I'd start with an assertion:

def generate_import_from(
    names: List[str], from_: Optional[str] = None, level: int = 0
) -> ast.ImportFrom:
    """Generate import from statement."""
    assert names, "Using ImportFrom with no names would produce invalid Python code"
    return ast.ImportFrom(
        module=from_, names=[ast.alias(n) for n in names], level=level
    )

and promptly, 58 tests fail and 3 error out. :-D

Luckily, these are down to just two callsites.

@mjpieters mjpieters linked a pull request Jan 8, 2025 that will close this issue
Open
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
3 participants
@mjpieters @DamianCzajkowski @weblab-misha