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

[flake8-type-checking] Fix some safe fixes being labeled unsafe #15638

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

Daverball
Copy link
Contributor

Summary

We were mistakenly using CommentRanges::has_comments to determine whether our edits
were safe, which sometimes expands the checked range to the end of a line. But in order to
determine safety we need to check exactly the range we're replacing.

This bug affected the rules runtime-cast-value (TC006) and quoted-type-alias (TC008)
although it was very unlikely to be hit for TC006 and for TC008 we never hit it because we
were checking the wrong expression.

Test Plan

cargo nextest run

This bug affected the rules `runtime-cast-value` (`TC006`) and
`quoted-type-alias` (`TC008`)
@Daverball
Copy link
Contributor Author

I disovered this bug while working on #15635

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the fixes Related to suggested fixes for violations label Jan 21, 2025
@MichaReiser
Copy link
Member

MichaReiser commented Jan 21, 2025

Thank you. Yeah, has_comments is somewhat misleading. It tries to do the right thing but its name is too generic. I think it's mainly optimized for testing if a statement (possibly compound) has any comments but it is to forgiving in that case...

@MichaReiser MichaReiser merged commit 4366473 into astral-sh:main Jan 21, 2025
21 checks passed
@Daverball Daverball deleted the bugfix/tc-fix-safety branch January 21, 2025 14:58
dcreager added a commit that referenced this pull request Jan 21, 2025
* main:
  Separate grouped and ungrouped nodes more clearly in AST generator (#15646)
  [`flake8-simplify`] Mark fixes as unsafe (`SIM201`, `SIM202`) (#15626)
  [red-knot] mdtest runner: include stderr for crashing tests (#15644)
  Change `EnvironmentOptions::venv-path` to `Option<SystemPathBuf>` (#15631)
  [`flake8-type-checking`] Fix some safe fixes being labeled unsafe (#15638)
  feat: Update RUF055 to do var == value (#15605)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants