Skip to content

Conversation

@alexprabhat99
Copy link
Contributor

@alexprabhat99 alexprabhat99 commented Dec 3, 2024

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

This PR raises logging-too-few-args when the logging message contains placeholders but no arguments are provided as suggested by @zenlyj here: #9999 (comment)

Closes #9999

@github-actions

This comment has been minimized.

@Pierre-Sassoulas
Copy link
Member

Those home assistant warnings are actual issues, this is looking very good.

num_args = _count_supplied_tokens(node.args[format_arg + 1 :])
if not num_args:
# If no args were supplied the string is not interpolated and can contain
# formatting characters - it's used verbatim. Don't check any further.
Copy link
Member

Choose a reason for hiding this comment

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

It seems to be voluntary though. Might need a dΓ©cision

@Pierre-Sassoulas Pierre-Sassoulas added False Negative πŸ¦‹ No message is emitted but something is wrong with the code Needs decision πŸ”’ Needs a decision before implemention or rejection labels Dec 4, 2024
@Pierre-Sassoulas Pierre-Sassoulas added this to the 4.0.0 milestone Dec 4, 2024
@alexprabhat99 alexprabhat99 closed this by deleting the head repository Feb 20, 2025
@github-actions
Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on home-assistant:
The following messages are now emitted:

  1. logging-too-few-args:
    Not enough arguments for logging format string
    https://github.com/home-assistant/core/blob/25865b4849b1a2eb5133fe3ab2fdfe3fd3b46818/homeassistant/components/sisyphus/light.py#L88
  2. logging-too-few-args:
    Not enough arguments for logging format string
    https://github.com/home-assistant/core/blob/25865b4849b1a2eb5133fe3ab2fdfe3fd3b46818/homeassistant/components/sisyphus/light.py#L94
  3. logging-too-few-args:
    Not enough arguments for logging format string
    https://github.com/home-assistant/core/blob/25865b4849b1a2eb5133fe3ab2fdfe3fd3b46818/homeassistant/components/shopping_list/__init__.py#L242
  4. logging-too-few-args:
    Not enough arguments for logging format string
    https://github.com/home-assistant/core/blob/25865b4849b1a2eb5133fe3ab2fdfe3fd3b46818/homeassistant/components/homeworks/config_flow.py#L175

This comment was generated for commit fe1792b

@Pierre-Sassoulas
Copy link
Member

The original decision was taken in db8b3a4 during implementation with a different rational ("If no args were supplied, then all format strings are valid don't check any further.") and was not discussed again when the comment was updated in #2713. I think the new behavior make sense especially considering that the primer shows 4 false negative in home-assistant. I'm going to open a new merge request as this branch was deleted.

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

Labels

False Negative πŸ¦‹ No message is emitted but something is wrong with the code Needs decision πŸ”’ Needs a decision before implemention or rejection

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detect missing arguments in logging format strings

2 participants