Skip to content

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

Merged
MichaReiser merged 1 commit into
astral-sh:mainfrom
Daverball:bugfix/tc-fix-safety
Jan 21, 2025
Merged

[flake8-type-checking] Fix some safe fixes being labeled unsafe#15638
MichaReiser merged 1 commit into
astral-sh:mainfrom
Daverball:bugfix/tc-fix-safety

Conversation

@Daverball

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

I disovered this bug while working on #15635

@github-actions

Copy link
Copy Markdown
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

MichaReiser commented Jan 21, 2025

Copy link
Copy Markdown
Member

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
@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