Skip to content

fix: Return empty list instead of None from _get_required_fields for schemas without properties#6377

Open
ItsMacto wants to merge 1 commit into
google:mainfrom
ItsMacto:fix-get-required-fields-empty-schema
Open

fix: Return empty list instead of None from _get_required_fields for schemas without properties#6377
ItsMacto wants to merge 1 commit into
google:mainfrom
ItsMacto:fix-get-required-fields-empty-schema

Conversation

@ItsMacto

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:
_get_required_fields in tools/_function_parameter_parse_util.py is annotated -> list[str] but has a bare return when the schema has no properties, so it returns None. Any caller that relies on the declared contract (e.g. iterating the result, as reported in #5920) gets TypeError: 'NoneType' object is not iterable. A maintainer confirmed the bare return as the culprit in the issue thread and endorsed return [].

Solution:
Return [] instead, honoring the declared return type. One-line change.

One honest scoping note: the two current callers in _automatic_function_calling_util.py guard the call behind a truthy properties dict, so today the empty branch is only reachable by calling the util directly (a function whose sole parameter is tool_context produces parameters=None rather than required=None). The fix corrects the contract violation at its source and hardens the util for present and future callers; the regression test pins the contract at the util level, and an added declaration-path test documents the tool_context-only behavior.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Three tests added to tests/unittests/tools/test_build_function_declaration.py:

  • _get_required_fields returns [] (not None) for a schema without properties — fails on the unfixed code (assert None == []), passes with the fix.
  • _get_required_fields returns the correct non-empty list for a schema with properties (guards against over-correction).
  • A tool_context-only function's declaration has parameters is None (documents the actual public-path behavior).
# before fix:
FAILED test_get_required_fields_no_properties_returns_empty_list - assert None == []

# with fix:
$ pytest tests/unittests/tools/test_build_function_declaration.py -q
41 passed

pyink --check and isort --check-only are clean on both touched files.

Manual End-to-End (E2E) Tests:

Not applicable beyond the unit level — the change is a pure return-value correction in a private utility with no runtime surface of its own; the declaration-path test covers the integrated behavior.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

None.

@ItsMacto

Copy link
Copy Markdown
Contributor Author

/gemini review

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Jul 11, 2026
@adk-bot

adk-bot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Response from ADK Triaging Agent

Hello @ItsMacto, thank you for creating this PR!

This PR is extremely well-structured and fully complies with our contribution guidelines:

I have labeled this PR with the tools label because it addresses tool function parameter parsing (tools/_function_parameter_parse_util.py). I have also assigned @xuanyang15 as the shepherd for this component.

Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: _get_required_fields returns None instead of [] for empty properties, causing TypeError

3 participants