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.

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

Include actual conditions in E712 diagnostics #10254

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

tjkuson
Copy link
Contributor

@tjkuson tjkuson commented Mar 6, 2024

Summary

Changes the generic recommendation to replace

if foo == True: ...

with if cond: to if foo:.

Still uses a generic message for compound comparisons as a specific message starts to become confusing. For example,

if foo == True != False: ...

produces two recommendations, one of which would recommend if True:, which is confusing.

Resolves recommendation in a previous PR.

Test Plan

cargo nextest run

Changes the generic recommendation to replace

```python
if foo == True: ...
```

with `if cond:` to `if foo:`.

Still uses a generic message for compound comparisons as a specific
message starts to become confusing. For example,

```python
if foo == True != False: ...
```

produces two recommendations, one of which would recommend `if True:`,
which is confusing.
Copy link
Contributor

github-actions bot commented Mar 6, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@tjkuson tjkuson marked this pull request as ready for review March 6, 2024 19:46
@charliermarsh charliermarsh self-requested a review March 6, 2024 21:45
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Mar 6, 2024
@charliermarsh charliermarsh self-assigned this Mar 6, 2024
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks!

@charliermarsh charliermarsh enabled auto-merge (squash) March 8, 2024 01:14
@charliermarsh charliermarsh merged commit 72c9f7e into astral-sh:main Mar 8, 2024
17 checks passed
@tjkuson tjkuson deleted the improve-e712-message branch March 9, 2024 01:19
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
## Summary

Changes the generic recommendation to replace

```python
if foo == True: ...
```

with `if cond:` to `if foo:`.

Still uses a generic message for compound comparisons as a specific
message starts to become confusing. For example,

```python
if foo == True != False: ...
```

produces two recommendations, one of which would recommend `if True:`,
which is confusing.

Resolves [recommendation in a previous
PR](https://github.com/astral-sh/ruff/pull/8613/files#r1514915070).

## Test Plan

`cargo nextest run`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants