Skip to content

Mark deprecated types and fields with annotation - #123

Open
anuraaga wants to merge 3 commits into
bufbuild:mainfrom
anuraaga:deprecated-annotation
Open

anuraaga wants to merge 3 commits into
bufbuild:mainfrom
anuraaga:deprecated-annotation

Conversation

@anuraaga

@anuraaga anuraaga commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Came up in connectrpc/connect-py#382. While I was initially hesitant since it feels messy, in the end it seems worth having parity with mypy-protobuf and does feel like a nice feature. deprecation is by nature messy so the gencode getting messy doesn't seem that bad actually. The trickiest part is enum values, but since I have seen enum values deprecated similarly to fields, I very much want to support them if going with this at all. The deprecated metaclass approach is the same as mypy-protobuf's

This also goes ahead and always emits suppressions for deprecation usage in gencode since similarly to ruff, it would cause type checking failure without much recourse, and it makes it easier for plugin authors to use deprecation too. I considered making it optional, let me know if you see any reasons to prefer that.

Minor, it also changes __init__ to use ... instead of pass. They're mostly the same and I don't think there was any reason to prefer pass, or not, but with overload generation, it becomes simpler to consistently use ....

Comment thread src/protobuf/plugin/_file.py Outdated
Comment on lines +505 to +507
"# pyright: reportDeprecated=false",
"# ty: ignore[unused-ignore-comment]",
"# ty: ignore[deprecated, unused-ignore-comment]",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

drive-by, but it's too bad that this list keeps growing (especially with ty, unfortunately, given it's 0.x status, although I'd hope it'd be fairly stable) 😄

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's unfortunate. I actually tried every type of approach and went back and forth between them some times

  1. Always add suppression
  2. Only add suppression when deprecated types are referenced
  3. No suppression and add to every line
  1. is most precise but it adds suppressions quite all over the place and seemed fragile, I expected to need followups for some sort of corner case. 1. felt simplest to me then, but actually I am starting to lean towards 2 heh. I updated so it only adds the suppressions to the whole file when needed via a preamble option.

Use the `no_fmt_off` option (see [Options](./options.md#no_fmt_off)) to omit
`# fmt: off` if you want ruff to format the output.

If the generated code references deprecated members, pass

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it possible to detect when deprecated members have been referenced, and emit the directive automatically? It's unfortunate that we have to add in a plugin option for this.

@anuraaga anuraaga Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not in a great way - the biggest reason is this is a string printer with a bit of pythonic functionality, not an AST-builder etc type of thing. So a user can just string interpolate some reference (e.g., connect-py generates a full function signature with a potentially deprecated RPC method as a single string) and we have no way of knowing that. On top of that, without an AST we can't determine if a deprecated thing is being referenced or just declared, the latter would cause a lint error on unused suppression. And while types may be closer to being detectable, fields are even harder.

Our of curiosity, FWIU protobuf-es generates @deprecated, I'm guessing it is checked by eslint etc so suppressing lint globally (similar to how we suppress ruff globally) handles it, here it's type-checker so we need type-checker suppression here, with unused suppressions themselves flagged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants