Skip to content

[ruff] Implement invalid-assert-message-literal-argument (RUF040)#14488

Merged
dylwil3 merged 8 commits intoastral-sh:mainfrom
Lokejoke:non-string-literal-as-assert-message
Nov 25, 2024
Merged

[ruff] Implement invalid-assert-message-literal-argument (RUF040)#14488
dylwil3 merged 8 commits intoastral-sh:mainfrom
Lokejoke:non-string-literal-as-assert-message

Conversation

@Lokejoke
Copy link
Contributor

@Lokejoke Lokejoke commented Nov 20, 2024

Summary

This PR implements new rule discussed here.
In short, it searches for assert messages which were unintentionally used as a expression to be matched against.

Test Plan

cargo test and review of ruff-ecosystem

@Lokejoke Lokejoke marked this pull request as draft November 20, 2024 14:09
@Lokejoke Lokejoke changed the title [ruff] Implement non-string-literal-as-assert-message (RUF035) [ruff] Implement non-string-literal-as-assert-message (RUF040) Nov 20, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+11 -0 violations, +0 -0 fixes in 4 projects; 51 projects unchanged)

apache/airflow (+8 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ providers/tests/amazon/aws/hooks/test_base_aws.py:1077:24: RUF040 Non-string literal used as assert message
+ providers/tests/edge/worker_api/routes/test_rpc_api.py:219:35: RUF040 Non-string literal used as assert message
+ providers/tests/google/cloud/sensors/test_gcs.py:201:30: RUF040 Non-string literal used as assert message
+ providers/tests/google/cloud/sensors/test_gcs.py:86:30: RUF040 Non-string literal used as assert message
+ providers/tests/google/common/hooks/test_base_google.py:84:24: RUF040 Non-string literal used as assert message
+ tests/api_connexion/endpoints/test_task_instance_endpoint.py:1370:38: RUF040 Non-string literal used as assert message
+ tests/api_fastapi/core_api/routes/public/test_task_instances.py:1973:38: RUF040 Non-string literal used as assert message
+ tests/api_internal/endpoints/test_rpc_api_endpoint.py:173:31: RUF040 Non-string literal used as assert message

bokeh/bokeh (+1 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ tests/unit/bokeh/io/test_state.py:47:43: RUF040 Non-string literal used as assert message

pandas-dev/pandas (+1 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ pandas/tests/series/accessors/test_cat_accessor.py:43:37: RUF040 Non-string literal used as assert message

pytest-dev/pytest (+1 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ testing/_py/test_local.py:218:26: RUF040 Non-string literal used as assert message

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF040 11 11 0 0 0

@Lokejoke Lokejoke marked this pull request as ready for review November 20, 2024 14:45
dylwil3
dylwil3 requested changes Nov 23, 2024
View reviewed changes
Copy link
Collaborator

@dylwil3 dylwil3 left a comment

Choose a reason for hiding this comment

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

This is a really clever rule idea, and the ecosystem checks all look like genuine bugs - great job! (May be worth filing some issues in those repos, actually 😄 )

I just have a quibble with the name of the rule (see the comment below), and a question about bytes literals, but otherwise this looks awesome. Thank you!

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Nov 25, 2024
@Lokejoke Lokejoke changed the title [ruff] Implement non-string-literal-as-assert-message (RUF040) [ruff] Implement invalid-assert-message-literal-argument (RUF040) Nov 25, 2024
@MichaReiser MichaReiser requested a review from dylwil3 November 25, 2024 17:28
dylwil3
dylwil3 approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@dylwil3 dylwil3 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the great contribution!

@dylwil3 dylwil3 merged commit 9e4ee98 into astral-sh:main Nov 25, 2024
@Lokejoke
Copy link
Contributor Author

Lokejoke commented Nov 26, 2024

Thank you for your guidance.

@potiuk
Copy link

potiuk commented Nov 28, 2024

Very helpful - thanks for the heads up @Lokejoke and team :D apache/airflow#44460 is coming in Airflow.

BTW, Some of them were not really "bugs" (i.e. when we had both assert with == and literal added after , but it was good to fix those anyway (True as message printed when assert fails is rather useless hint).

@BrewTestBot BrewTestBot mentioned this pull request Nov 29, 2024
Merged
@MichaReiser MichaReiser mentioned this pull request Dec 2, 2024
Merged
@Lokejoke Lokejoke mentioned this pull request Dec 8, 2024
Open
@MichaReiser MichaReiser mentioned this pull request Mar 16, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser left review comments

@dylwil3 dylwil3 dylwil3 approved these changes

Assignees

No one assigned

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Lokejoke @potiuk @MichaReiser @dylwil3