Skip to content

Comments

[ty] Add 'remove unused ignore comment' code action#21582

Merged
Gankra merged 5 commits intomainfrom
micha/add-ignore-action
Nov 25, 2025
Merged

[ty] Add 'remove unused ignore comment' code action#21582
Gankra merged 5 commits intomainfrom
micha/add-ignore-action

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Nov 22, 2025

Summary

This PR adds a code action to remove unused ignore comments.

This PR also includes some infrastructure boilerplate to set up code actions in the editor:

  • Extend snapshot-diagnostics to render fixes
  • Render fixes when using --output-format=full
  • Hook up edits and the code action request in the LSP
  • Add the Unnecessary tag to unused-ignore-comment diagnostics
  • Group multiple unused codes into a single diagnostic

The same fix can be used on the CLI once we add ty fix

Note: unused-ignore-comment is currently disabled by default.

Screen.Recording.2025-11-22.at.22.36.26.mov

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Nov 22, 2025
@MichaReiser MichaReiser changed the title [ty] Add 'remove unused ignore comment' code action" [ty] Add 'remove unused ignore comment' code action Nov 22, 2025
@MichaReiser MichaReiser force-pushed the micha/add-ignore-action branch from a40403a to 820ff0c Compare November 22, 2025 21:41
@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 22, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 22, 2025

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.

@MichaReiser MichaReiser force-pushed the micha/add-ignore-action branch from 820ff0c to 8850253 Compare November 23, 2025 10:08
@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 23, 2025

mypy_primer results

Changes were detected when running on open source projects
pytest-robotframework (https://github.com/detachhead/pytest-robotframework)
- pytest_robotframework/_internal/pytest/plugin.py:127:99: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'unresolved-attribute'
+ pytest_robotframework/_internal/pytest/plugin.py:127:99: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pytest_robotframework/_internal/pytest/plugin.py:197:93: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'unresolved-attribute'
+ pytest_robotframework/_internal/pytest/plugin.py:197:93: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pytest_robotframework/_internal/pytest/plugin.py:235:89: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'non-subscriptable'
+ pytest_robotframework/_internal/pytest/plugin.py:235:89: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pytest_robotframework/_internal/robot/library.py:86:99: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'possibly-missing-attribute'
+ pytest_robotframework/_internal/robot/library.py:86:99: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:455:40: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'missing-typed-dict-key'
- pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:455:63: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:455:28: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:472:40: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'missing-typed-dict-key'
- pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:472:63: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ pytest_robotframework/_internal/robot/listeners_and_suite_visitors.py:472:28: warning[unused-ignore-comment] Unused `ty: ignore` directive
- tests/conftest.py:82:77: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'subclass-of-final-class'
+ tests/conftest.py:82:77: warning[unused-ignore-comment] Unused `ty: ignore` directive
- Found 169 diagnostics
+ Found 167 diagnostics

typeshed-stats (https://github.com/AlexWaygood/typeshed-stats)
- tests/test__cli.py:541:38: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-assignment'
+ tests/test__cli.py:541:38: warning[unused-ignore-comment] Unused `ty: ignore` directive

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
- src/scikit_build_core/build/wheel.py:98:20: error[no-matching-overload] No overload of bound method `__init__` matches arguments
- Found 45 diagnostics
+ Found 44 diagnostics

streamlit (https://github.com/streamlit/streamlit)
- lib/streamlit/config.py:2635:37: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-return-type'
+ lib/streamlit/config.py:2635:37: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/connections/sql_connection.py:221:37: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'redundant-cast'
+ lib/streamlit/connections/sql_connection.py:221:37: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/dataframe_util.py:701:41: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'redundant-cast'
+ lib/streamlit/dataframe_util.py:701:41: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/dataframe_util.py:730:41: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'redundant-cast'
+ lib/streamlit/dataframe_util.py:730:41: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/lib/built_in_chart_utils.py:637:22: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-return-type'
+ lib/streamlit/elements/lib/built_in_chart_utils.py:637:22: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/vega_charts.py:382:33: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'unresolved-attribute'
+ lib/streamlit/elements/vega_charts.py:382:33: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/vega_charts.py:416:30: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-context-manager'
+ lib/streamlit/elements/vega_charts.py:416:30: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/vega_charts.py:418:37: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-context-manager'
+ lib/streamlit/elements/vega_charts.py:418:37: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/widgets/data_editor.py:171:67: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'possibly-missing-attribute'
+ lib/streamlit/elements/widgets/data_editor.py:171:67: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/widgets/slider.py:801:63: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'unsupported-operator'
+ lib/streamlit/elements/widgets/slider.py:801:63: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/elements/widgets/time_widgets.py:167:73: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ lib/streamlit/elements/widgets/time_widgets.py:167:73: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/file_util.py:247:51: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'no-matching-overload'
+ lib/streamlit/file_util.py:247:51: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/runtime/caching/cache_utils.py:128:39: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ lib/streamlit/runtime/caching/cache_utils.py:128:39: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/runtime/caching/cache_utils.py:174:40: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ lib/streamlit/runtime/caching/cache_utils.py:174:40: warning[unused-ignore-comment] Unused `ty: ignore` directive
- lib/streamlit/runtime/caching/cache_utils.py:200:35: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ lib/streamlit/runtime/caching/cache_utils.py:200:35: warning[unused-ignore-comment] Unused `ty: ignore` directive

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
- pandas-stubs/core/groupby/generic.pyi:215:34: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ pandas-stubs/core/groupby/generic.pyi:215:34: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pandas-stubs/core/groupby/generic.pyi:220:34: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ pandas-stubs/core/groupby/generic.pyi:220:34: warning[unused-ignore-comment] Unused `ty: ignore` directive
- pandas-stubs/core/groupby/generic.pyi:225:34: warning[unused-ignore-comment] Unused `ty: ignore` directive: 'invalid-argument-type'
+ pandas-stubs/core/groupby/generic.pyi:225:34: warning[unused-ignore-comment] Unused `ty: ignore` directive

No memory usage changes detected ✅

@MichaReiser MichaReiser changed the base branch from main to micha/suppressions-diagnostic-guard November 23, 2025 11:03
@MichaReiser MichaReiser force-pushed the micha/add-ignore-action branch 2 times, most recently from ccfb4ef to 58b48c7 Compare November 23, 2025 11:33
[dependencies]
ruff_db = { workspace = true }
ruff_annotate_snippets = { workspace = true }
ruff_diagnostics = { workspace = true }
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably fold ruff_diagnostics into ruff_db. There's really not much in there but this seemed unrelated to this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this and it ran into circular reference nastiness (stuff depends on ruff_diagnostics that ruff_db in turn depends on, I guess?). I opted to just pub use the relevant items in ruff_db.

Comment on lines -637 to -642
let code = &self.source[*code_range];
let range = if codes.len() == 1 {
comment.range
} else {
*code_range
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now handled by the unused suppression lint itself

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Very excited to see this happen! A few nits from skimming

@carljm carljm removed their request for review November 24, 2025 22:30
@MichaReiser MichaReiser force-pushed the micha/suppressions-diagnostic-guard branch from b97e848 to a67e6f2 Compare November 25, 2025 08:08
@MichaReiser MichaReiser force-pushed the micha/add-ignore-action branch from 88902c8 to 5d5803c Compare November 25, 2025 08:10
@MichaReiser MichaReiser requested review from Gankra and removed request for dcreager and sharkdp November 25, 2025 08:10
Base automatically changed from micha/suppressions-diagnostic-guard to main November 25, 2025 10:54
@MichaReiser MichaReiser force-pushed the micha/add-ignore-action branch from b2cc26b to 7efe928 Compare November 25, 2025 10:59
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

🚀

(Didn't review the ty_server code in depth)

Copy link
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

Rad, yeah this is exactly the minimal version that makes sense.

[dependencies]
ruff_db = { workspace = true }
ruff_annotate_snippets = { workspace = true }
ruff_diagnostics = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this and it ran into circular reference nastiness (stuff depends on ruff_diagnostics that ruff_db in turn depends on, I guess?). I opted to just pub use the relevant items in ruff_db.

@Gankra Gankra merged commit 15cb41c into main Nov 25, 2025
41 checks passed
@Gankra Gankra deleted the micha/add-ignore-action branch November 25, 2025 13:08
carljm added a commit to mtshiba/ruff that referenced this pull request Nov 25, 2025
* main:
  [ty] Implement `typing.override` (astral-sh#21627)
  [ty] Avoid expression reinference for diagnostics (astral-sh#21267)
  [ty] Improve autocomplete suppressions of keywords in variable bindings
  [ty] Only suggest completions based on text before the cursor
  Implement goto-definition and find-references for global/nonlocal statements (astral-sh#21616)
  [ty] Inlay Hint edit follow up (astral-sh#21621)
  [ty] Implement lsp support for string annotations (astral-sh#21577)
  [ty] Add 'remove unused ignore comment' code action (astral-sh#21582)
  [ty] Refactor `CheckSuppressionContext` to use `DiagnosticGuard` (astral-sh#21587)
  [ty] Improve several "Did you mean?" suggestions (astral-sh#21597)
  [ty] Add more and update existing projects in `ty_benchmark` (astral-sh#21536)
  [ty] fix ty playground initialization and vite optimization issues (astral-sh#21471)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants