Open zackmdavis opened 6 years ago
Stripping derive has also created problems elsewhere.
cc @oli-obk @jseyfried ?
I think it would be reasonable to avoid stripping derives.
@zackmdavis
it looks like this happens twice, which is confusing: 1 [2](https://github.com/rust-lang/rust/blob/cbf5d39cca/src/libsyntax/ext/derive.rs#L25-L27
The second link is only stripping empty derives and erroneous derives (after emitting an error/warning) -- the second match arm is the successful case and retains the derive. Feel free to ping me on IRC to discuss how to proceed.
Triage: Currently the suggestion looks like this:
#![warn(missing_debug_implementations)]
#[derive(Clone)]
pub struct MissingDebug;
fn main() {
}
warning: type does not implement `Debug`; consider adding `#[derive(Debug)]` or a manual implementation
--> src/main.rs:4:1
|
4 | pub struct MissingDebug;
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> src/main.rs:1:9
|
1 | #![warn(missing_debug_implementations)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If I understand this issue correctly, it is about enabling the suggestion to be more specific by taking into account currently present derive
s. Maybe something like:
warning: type does not implement
Debug
; consider addingDebug
to#[derive(Clone, ...)]
or a manual implementation
In the spirit of #44942, it would be nice if the
missing_debug_implementations
andmissing_copy_implementations
lints suggested adding a derive attribute (in the case where one does not already exist) or addingDebug
(respectivelyCopy
) to the list of traits in the derive attribute (in the case where it already exists). (This mostly for the sake of RLS and other tools, but the span highlighting is pretty, too.) But while the lints (obviously, necessarily) know how to look up whether the trait has been implemented, it's not clear how to make them detect an existing derive attribute (and its span):cx.tcx.get_attrs
doesn't work because we apparently strip off the attribute during expansion(it looks like this happens twice, which is confusing: 1 2). But I can't modify the expansion code because I'm still not smart enough to understand it.