Click doesn't close file options during shell completion

[gregorias]

Click doesn't close file options during shell completion, which causes a resource warning if a program uses a file option.

For example, I have group like this:

@click.group()
@click.option('--config_file',
              default=CONFIG,
              type=click.File(mode='r'),
              help='help')
@click.pass_context
def cli(ctx, config_file: typing.TextIO):

and I get this warning:

/Users/grzesiek/Library/Caches/pypoetry/virtualenvs/findata-fetcher-3hK6JJJX-py3.12/lib/python3.12/site-packages/click/shell_completion.py:293: ResourceWarning: unclosed file <_io.TextIOWrapper name='/Users/grzesiek/.config/findata/fetcher.json' mode='r' encoding='UTF-8'>
  completions = self.get_completions(args, incomplete)

Details

I don't come with reproduction steps, but I can give something equally valuable, I can explain how this bug comes to be.

The issue stems from allocating a context in core.py outside of a with statement during shell completion. Here's a stack-trace of how that happens:

  File "/Users/grzesiek/.local/bin/findata-fetcher", line 8, in <module>
    sys.exit(main())
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/fetcher/tool.py", line 576, in main
    cli(obj={})
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/core.py", line 1171, in __call__
    return self.main(*args, **kwargs)
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/core.py", line 1084, in main
    self._main_shell_completion(extra, prog_name, complete_var)
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/core.py", line 1165, in _main_shell_completion
    rv = shell_complete(self, ctx_args, prog_name, complete_var,
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/shell_completion.py", line 49, in shell_complete
    echo(comp.complete())
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/shell_completion.py", line 296, in complete
    completions = self.get_completions(args, incomplete)
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/shell_completion.py", line 273, in get_completions
    ctx = _resolve_context(self.cli, self.ctx_args, self.prog_name, args)
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/shell_completion.py", line 513, in _resolve_context
    ctx = cli.make_context(prog_name, args.copy(), **ctx_args)
  File "/Users/grzesiek/.local/pipx/venvs/findata-fetcher/lib/python3.12/site-packages/click/core.py", line 952, in make_context
    with ctx.scope(cleanup=False):

This context gets returned to get_completions, but get_completions or its caller never call the context's __exit__ function.

Calling the __exit__ function is essential, because types.File depends on it for cleanup:

click/src/click/types.py

Line 740 in ca5e1c3

ctx.call_on_close(safecall(f.close))

.

Environment

• Python version: 3.12
• Click version: 8.1.7

[AndreasBackx]

@gregorias, if you have some time. Could you create a test case that can reproduce the issue? I'm having a hard time exactly reproducing the error that you're seeing. Doesn't have to be a test case, but even an example CLI and what to call it with would be sufficient. Then I can look into this.

[gregorias]

You can find the repro repo + instructions at https://github.com/gregorias/click-2644. It’s crucial to enable the dev mode to see warnings with PYTHONDEVMODE.

[AndreasBackx]

@gregorias thank you so much. I'll give it a look. I have some experience with the resource warnings and the dev mode settings from at work where we're also doing some things around it.

[AndreasBackx]

It seems that the reason why I couldn't get it to repro was because when the file (default="README.md" in this case) does not exist, it doesn't open it. It does when it exists resulting in the warning. I'll change my test to incorporate a mocked/isolated filesystem. Thanks for the repro!

[AndreasBackx]

Signing off for today. I haven't had the time to try and confirm your example is fixed now too, but it likely should be. I used it as an inspiration for the new test case. Let me know if you still see an issue! Thanks once again for the repro!