Skip to content

Comments

[ty] Make unused-ignore-comment disabled by default for now#17955

Merged
AlexWaygood merged 1 commit intomainfrom
alex/unused-ignore
May 8, 2025
Merged

[ty] Make unused-ignore-comment disabled by default for now#17955
AlexWaygood merged 1 commit intomainfrom
alex/unused-ignore

Conversation

@AlexWaygood
Copy link
Member

Summary

Right now it seems like there are too many situations where:

  1. There is a typing error on a line that all other type checkers detect
  2. The user has suppressed that error for Reasons with a type: ignore comment
  3. But we don't yet detect the typing error due to a missing feature, leading us to complain about the unused ignore comment

The rule feels like more of a lint anyway, and mypy's version is opt-in. I'd like for it to be enabled by default for the GA release, but right now it feels like we're getting a lot of user questions about it. Let's make it disabled-by-default until we're closer to feature-parity with other type checkers.

Test Plan

  • cargo test
  • I ran ty locally on a file that just had x = 1 + 2 # type: ignore on it and verified that no diagnostics were reported by ty

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label May 8, 2025
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks. Would you mind opening an issue so that we don't forget to re-enable the rule.

@MichaReiser MichaReiser added the configuration Related to settings and configuration label May 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2025

mypy_primer results

No ecosystem changes detected ✅

@AlexWaygood AlexWaygood force-pushed the alex/unused-ignore branch from 784a8b6 to 34367f3 Compare May 8, 2025 16:15
@AlexWaygood
Copy link
Member Author

Thanks. Would you mind opening an issue so that we don't forget to re-enable the rule.

Will do

@AlexWaygood AlexWaygood merged commit da85408 into main May 8, 2025
34 checks passed
@AlexWaygood AlexWaygood deleted the alex/unused-ignore branch May 8, 2025 16:21
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood
Copy link
Member Author

Thanks. Would you mind opening an issue so that we don't forget to re-enable the rule.

Will do

astral-sh/ty#278

dcreager added a commit that referenced this pull request May 8, 2025
* main:
  [ty] Respect the gradual guarantee when reporting errors in resolving MROs (#17962)
  Convert `Message::SyntaxError` to use `Diagnostic` internally (#17784)
  [ty] Support extending `__all__` with a literal tuple or set as well as a literal list (#17948)
  [ty] Make `unused-ignore-comment` disabled by default for now (#17955)
  [ty] Change default severity for `unbound-reference` to `error` (#17936)
  [ty] Ignore `possibly-unresolved-reference` by default (#17934)
  [ty] Default to latest supported python version (#17938)
  [ty] Generate and add rules table (#17953)
  Update the schemastore script to match changes in ty (#17952)
  [ty] Implement `global` handling and `load-before-global-declaration` syntax error (#17637)
dcreager added a commit that referenced this pull request May 8, 2025
* dcreager/default-typevars:
  clean up the diff
  remove trait
  track in type again
  clippy
  Better expansion of default typevars
  specialize_partial
  enum for TypeMapping
  [ty] Respect the gradual guarantee when reporting errors in resolving MROs (#17962)
  Specialize trait
  Convert `Message::SyntaxError` to use `Diagnostic` internally (#17784)
  [ty] Support extending `__all__` with a literal tuple or set as well as a literal list (#17948)
  [ty] Make `unused-ignore-comment` disabled by default for now (#17955)
  [ty] Change default severity for `unbound-reference` to `error` (#17936)
  [ty] Ignore `possibly-unresolved-reference` by default (#17934)
  [ty] Default to latest supported python version (#17938)
  [ty] Generate and add rules table (#17953)
  Update the schemastore script to match changes in ty (#17952)
  [ty] Implement `global` handling and `load-before-global-declaration` syntax error (#17637)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

configuration Related to settings and configuration ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants