Add a lint for non-exported local types in visible type signatures.#11773
Add a lint for non-exported local types in visible type signatures.#11773huonw wants to merge 3 commits intorust-lang:masterfrom
Conversation
The fundamental bug (expressions in fixed-length vectors make the lint passes unhappy) is still there, so we just sneak around it by (manually) avoiding recurring into that expression.
This warns whenever a publically visible type (e.g. in the signature of a public function, or the non-priv field of a public struct) is not exported.
|
This ends up encoding some logic about whether a type is local or not inside lint, which makes it less nice... (If this doesn't land, I'm going to submit (most of) the last patch separately because there's a pile of accidentally non-exported types (or non-priv fields).) |
|
In general, I like the idea for this lint, but I'd like to garner some more opinions on this (I don't think we've quite reached a consensus on #10573 yet). Implementation-wise, I'm troubled to see how invasive it is to the other lint passes and the visiting context itself. I would expect this to be a pretty self contained lint which only checks function signatures, structs, and enums (checks in visit_item and visit_fn). It looks like the invasive parts elsewhere are mainly to get a better error message, which I think is ok to sacrifice for complexity reduction, but I may have also missed a point where they're more crucial to the analysis. |
|
I'm also unhappy how invasive this is, but, unfortunately, I don't see any other way to handle it, other than doing some manual visit_type calls in visit_fn and visit_item. I guess I can try it. Re #10573: I've implemented this because I think it's very bad code style to have private types in public code signatures (e.g. users of your library can no longer name the type that functions return; e.g. you can't write wrappers around them, you can only use call the functions directly) and so I'd like to have the option for the compiler to warn about it whether or not we decide to outright disallow it. |
|
I talked with @huonw on IRC about this, and he's going to look into either simplifying the logic to have it a little more contained, or move the logic to its own pass in |
|
This is on the backburner for now; I'll get back to it after I'm done with |
Except for traits, to allow them to be used as closed type-classes and/or PIMPL-style polymorphic trait objects