Skip to content

Commit 66c6483

Browse files
committed
API: Remove decorate argument from emit_lint and add docs (Breaking)
1 parent 26945b8 commit 66c6483

File tree

7 files changed

+265
-79
lines changed

7 files changed

+265
-79
lines changed

marker_api/src/context.rs

Lines changed: 116 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,24 +105,129 @@ impl<'ast> AstContext<'ast> {
105105
self.driver.call_lint_level_at(lint, node.emission_node_id())
106106
}
107107

108+
/// This function is used to emit a lint.
109+
///
110+
/// Every lint emission, is bound to one specific node in the AST. This
111+
/// node is used to check the lint level and as the main [`Span`] of the
112+
/// diagnostic message. See [`EmissionNode`] for more information.
113+
///
114+
/// The lint message, will be the main message at the start of the created
115+
/// diagnostic. This message and all messages emitted as part of the created
116+
/// diagnostic should start with a lower letter, according to [rustc's dev guide].
117+
///
118+
/// The function will return a [`DiagnosticBuilder`] which can be used to decorate
119+
/// the diagnostic message, with notes and help messages. The diagnostic message will
120+
/// be emitted when the builder instance is dropped.
121+
///
122+
/// [rustc's dev guide]: <https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-output-style-guide>
123+
///
124+
/// ## Example 1
125+
///
126+
/// ```
127+
/// # use marker_api::prelude::*;
128+
/// # marker_api::declare_lint!(
129+
/// # /// Dummy
130+
/// # LINT,
131+
/// # Warn,
132+
/// # );
133+
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
134+
/// cx.emit_lint(LINT, node, "<lint message>");
135+
/// # }
136+
/// ```
137+
///
138+
/// The code above will roughly generate the following error message:
139+
///
140+
/// ```text
141+
/// warning: <lint message> <-- The message that is set by this function
142+
/// --> path/file.rs:1:1
143+
/// |
144+
/// 1 | node
145+
/// | ^^^^
146+
/// |
147+
/// ```
148+
///
149+
/// ## Example 2
150+
///
151+
/// ```
152+
/// # use marker_api::prelude::*;
153+
/// # marker_api::declare_lint!(
154+
/// # /// Dummy
155+
/// # LINT,
156+
/// # Warn,
157+
/// # );
158+
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
159+
/// cx
160+
/// .emit_lint(LINT, node, "<lint message>")
161+
/// .help("<text>");
162+
/// # }
163+
/// ```
164+
///
165+
/// The [`DiagnosticBuilder::help`] call will add a help message like this:
166+
///
167+
/// ```text
168+
/// warning: <lint message>
169+
/// --> path/file.rs:1:1
170+
/// |
171+
/// 1 | node
172+
/// | ^^^^
173+
/// |
174+
/// = help: <text> <-- The added help message
175+
/// ```
176+
///
177+
/// ## Example 3
178+
///
179+
/// Creating suggestions can impact the performance. The
180+
/// [`DiagnosticBuilder::decorate`] function can be used, to only add more
181+
/// context to the lint emission if it will actually be emitted. This will
182+
/// save performance, when the lint is allowed at the [`EmissionNode`]
183+
///
184+
/// ```
185+
/// # use marker_api::prelude::*;
186+
/// # marker_api::declare_lint!(
187+
/// # /// Dummy
188+
/// # LINT,
189+
/// # Warn,
190+
/// # );
191+
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
192+
/// cx.emit_lint(LINT, node, "<lint message>").decorate(|diag| {
193+
/// // This closure is only called, if the lint is enabled. Here you
194+
/// // can create a beautiful help message.
195+
/// diag.help("<text>");
196+
/// });
197+
/// # }
198+
/// ```
199+
///
200+
/// This will create the same help message as in example 2, but it will be faster
201+
/// if the lint is enabled. The emitted message would look like this:
202+
/// ```text
203+
/// warning: <lint message>
204+
/// --> path/file.rs:1:1
205+
/// |
206+
/// 1 | node
207+
/// | ^^^^
208+
/// |
209+
/// = help: <text> <-- The added help message
210+
/// ```
108211
#[allow(clippy::needless_pass_by_value)] // `&impl ToString`
109-
pub fn emit_lint<F>(&self, lint: &'static Lint, node: impl EmissionNode<'ast>, msg: impl ToString, decorate: F)
110-
where
111-
F: FnOnce(&mut DiagnosticBuilder<'ast>),
112-
{
212+
pub fn emit_lint(
213+
&self,
214+
lint: &'static Lint,
215+
node: impl EmissionNode<'ast>,
216+
msg: impl ToString,
217+
) -> DiagnosticBuilder<'ast> {
218+
let id = node.emission_node_id();
113219
let Some(span) = node.emission_span(self) else {
114220
// If the `Span` is none, we can't emit a diagnostic message for it.
115-
return;
221+
return DiagnosticBuilder::new_dummy(lint, id);
116222
};
117223
if matches!(lint.report_in_macro, MacroReport::No) && span.is_from_expansion() {
118-
return;
224+
return DiagnosticBuilder::new_dummy(lint, id);
119225
}
120-
let id = node.emission_node_id();
121-
if self.lint_level_at(lint, node) != Level::Allow {
122-
let mut builder = DiagnosticBuilder::new(lint, id, msg.to_string(), span);
123-
decorate(&mut builder);
124-
builder.emit(self);
226+
if self.lint_level_at(lint, node) == Level::Allow {
227+
return DiagnosticBuilder::new_dummy(lint, id);
125228
}
229+
230+
DiagnosticBuilder::new(lint, id, msg.to_string(), span)
126231
}
127232

128233
pub(crate) fn emit_diagnostic<'a>(&self, diag: &'a Diagnostic<'a, 'ast>) {

marker_api/src/diagnostic.rs

Lines changed: 114 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::fmt::Debug;
55

66
use crate::{
77
ast::{ExprId, FieldId, ItemId, Span, StmtId, VariantId},
8-
context::AstContext,
8+
context::{with_cx, AstContext},
99
ffi::{FfiSlice, FfiStr},
1010
lint::Lint,
1111
};
@@ -17,19 +17,34 @@ pub struct DiagnosticBuilder<'ast> {
1717
lint: &'static Lint,
1818
node: EmissionNodeId,
1919
msg: String,
20-
span: Span<'ast>,
20+
span: Option<Span<'ast>>,
2121
parts: Vec<DiagnosticPart<String, Span<'ast>>>,
22+
/// This flag indicates, if the diagnostic messages should actually be emitted
23+
/// at the end. This might be false, if the lint is allowed at the emission location.
24+
emit_lint: bool,
2225
}
2326

2427
#[allow(clippy::needless_pass_by_value)] // `&impl ToString` doesn't work
2528
impl<'ast> DiagnosticBuilder<'ast> {
29+
pub(crate) fn new_dummy(lint: &'static Lint, node: EmissionNodeId) -> Self {
30+
Self {
31+
lint,
32+
node,
33+
msg: String::new(),
34+
span: None,
35+
parts: vec![],
36+
emit_lint: false,
37+
}
38+
}
39+
2640
pub(crate) fn new(lint: &'static Lint, node: EmissionNodeId, msg: String, span: Span<'ast>) -> Self {
2741
Self {
2842
lint,
2943
msg,
3044
node,
31-
span,
45+
span: Some(span),
3246
parts: vec![],
47+
emit_lint: true,
3348
}
3449
}
3550

@@ -45,7 +60,9 @@ impl<'ast> DiagnosticBuilder<'ast> {
4560
/// |
4661
/// ```
4762
pub fn set_main_message(&mut self, msg: impl ToString) -> &mut Self {
48-
self.msg = msg.to_string();
63+
if self.emit_lint {
64+
self.msg = msg.to_string();
65+
}
4966
self
5067
}
5168

@@ -63,7 +80,10 @@ impl<'ast> DiagnosticBuilder<'ast> {
6380
/// |
6481
/// ```
6582
pub fn set_main_span(&mut self, span: &Span<'ast>) -> &mut Self {
66-
self.span = span.clone();
83+
if self.emit_lint {
84+
self.span = Some(span.clone());
85+
}
86+
6787
self
6888
}
6989

@@ -82,8 +102,12 @@ impl<'ast> DiagnosticBuilder<'ast> {
82102
/// ```
83103
///
84104
/// [`Self::span_note`] can be used to highlight a relevant [`Span`].
85-
pub fn note(&mut self, msg: impl ToString) {
86-
self.parts.push(DiagnosticPart::Note { msg: msg.to_string() });
105+
pub fn note(&mut self, msg: impl ToString) -> &mut Self {
106+
if self.emit_lint {
107+
self.parts.push(DiagnosticPart::Note { msg: msg.to_string() });
108+
}
109+
110+
self
87111
}
88112

89113
/// This function adds a note with a [`Span`] to the diagnostic message.
@@ -106,11 +130,15 @@ impl<'ast> DiagnosticBuilder<'ast> {
106130
/// ```
107131
///
108132
/// [`Self::note`] can be used to add text notes without a span.
109-
pub fn span_note(&mut self, msg: impl ToString, span: &Span<'ast>) {
110-
self.parts.push(DiagnosticPart::NoteSpan {
111-
msg: msg.to_string(),
112-
span: span.clone(),
113-
});
133+
pub fn span_note(&mut self, msg: impl ToString, span: &Span<'ast>) -> &mut Self {
134+
if self.emit_lint {
135+
self.parts.push(DiagnosticPart::NoteSpan {
136+
msg: msg.to_string(),
137+
span: span.clone(),
138+
});
139+
}
140+
141+
self
114142
}
115143

116144
/// This function adds a help message. Help messages are intended to provide
@@ -130,7 +158,10 @@ impl<'ast> DiagnosticBuilder<'ast> {
130158
/// [`Self::span_help`] can be used to highlight a relevant [`Span`].
131159
/// [`Self::span_suggestion`] can be used to add a help message with a suggestion.
132160
pub fn help(&mut self, msg: impl ToString) -> &mut Self {
133-
self.parts.push(DiagnosticPart::Help { msg: msg.to_string() });
161+
if self.emit_lint {
162+
self.parts.push(DiagnosticPart::Help { msg: msg.to_string() });
163+
}
164+
134165
self
135166
}
136167

@@ -155,11 +186,15 @@ impl<'ast> DiagnosticBuilder<'ast> {
155186
///
156187
/// [`Self::help`] can be used to add a text help message without a [`Span`].
157188
/// [`Self::span_suggestion`] can be used to add a help message with a suggestion.
158-
pub fn span_help(&mut self, msg: impl ToString, span: &Span<'ast>) {
159-
self.parts.push(DiagnosticPart::HelpSpan {
160-
msg: msg.to_string(),
161-
span: span.clone(),
162-
});
189+
pub fn span_help(&mut self, msg: impl ToString, span: &Span<'ast>) -> &mut Self {
190+
if self.emit_lint {
191+
self.parts.push(DiagnosticPart::HelpSpan {
192+
msg: msg.to_string(),
193+
span: span.clone(),
194+
});
195+
}
196+
197+
self
163198
}
164199

165200
/// This function adds a spanned help message with a suggestion. The suggestion
@@ -184,25 +219,70 @@ impl<'ast> DiagnosticBuilder<'ast> {
184219
span: &Span<'ast>,
185220
suggestion: impl ToString,
186221
app: Applicability,
187-
) {
188-
self.parts.push(DiagnosticPart::Suggestion {
189-
msg: msg.to_string(),
190-
span: span.clone(),
191-
sugg: suggestion.to_string(),
192-
app,
193-
});
222+
) -> &mut Self {
223+
if self.emit_lint {
224+
self.parts.push(DiagnosticPart::Suggestion {
225+
msg: msg.to_string(),
226+
span: span.clone(),
227+
sugg: suggestion.to_string(),
228+
app,
229+
});
230+
}
231+
self
232+
}
233+
234+
/// This function takes a closure, that is only executed, if the created
235+
/// diagnostic is actually emitted in the end. This is useful for crafting
236+
/// suggestions. Having them in a conditional closure will speedup the
237+
/// linting process if the lint is allowed at a given location.
238+
///
239+
/// ```
240+
/// # use marker_api::prelude::*;
241+
/// # marker_api::declare_lint!(
242+
/// # /// Dummy
243+
/// # LINT,
244+
/// # Warn,
245+
/// # );
246+
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
247+
/// cx.emit_lint(LINT, node, "<lint message>").decorate(|diag| {
248+
/// // This closure is only called, if the lint is enabled. Here you
249+
/// // can create a beautiful help message.
250+
/// diag.help("<text>");
251+
/// });
252+
/// # }
253+
/// ```
254+
pub fn decorate<F>(&mut self, decorate: F) -> &mut Self
255+
where
256+
F: FnOnce(&mut DiagnosticBuilder<'ast>),
257+
{
258+
if self.emit_lint {
259+
decorate(self);
260+
}
261+
self
194262
}
195263

196264
pub(crate) fn emit<'builder>(&'builder self, cx: &AstContext<'ast>) {
197-
let parts: Vec<_> = self.parts.iter().map(DiagnosticPart::to_ffi_part).collect();
198-
let diag = Diagnostic {
199-
lint: self.lint,
200-
msg: self.msg.as_str().into(),
201-
node: self.node,
202-
span: &self.span,
203-
parts: parts.as_slice().into(),
204-
};
205-
cx.emit_diagnostic(&diag);
265+
if self.emit_lint {
266+
let parts: Vec<_> = self.parts.iter().map(DiagnosticPart::to_ffi_part).collect();
267+
let span = self
268+
.span
269+
.as_ref()
270+
.expect("always Some, if `DiagnosticBuilder::emit_lint` is true");
271+
let diag = Diagnostic {
272+
lint: self.lint,
273+
msg: self.msg.as_str().into(),
274+
node: self.node,
275+
span,
276+
parts: parts.as_slice().into(),
277+
};
278+
cx.emit_diagnostic(&diag);
279+
}
280+
}
281+
}
282+
283+
impl<'ast> Drop for DiagnosticBuilder<'ast> {
284+
fn drop(&mut self) {
285+
with_cx(self, |cx| self.emit(cx));
206286
}
207287
}
208288

marker_lints/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ fn check_msg<'ast>(cx: &AstContext<'ast>, msg_expr: ExprKind<'ast>) {
5050
DIAG_MSG_UPPERCASE_START,
5151
msg_expr,
5252
"this message starts with an uppercase character",
53-
|_| {},
5453
);
5554
}
5655
}

marker_lints/tests/ui/diag_msg_uppercase_start.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ marker_api::declare_lint!(
99
);
1010

1111
pub fn accept_message<'ast>(cx: &AstContext<'ast>, expr: ExprKind<'ast>) {
12-
cx.emit_lint(DUMMY, expr, "x <-- this is cool", |_| {});
13-
cx.emit_lint(DUMMY, expr, "=^.^= <-- Interesting, but valid", |_| {});
14-
cx.emit_lint(DUMMY, expr, "", |_| {});
15-
12+
cx.emit_lint(DUMMY, expr, "x <-- this is cool");
13+
cx.emit_lint(DUMMY, expr, "=^.^= <-- Interesting, but valid");
14+
cx.emit_lint(DUMMY, expr, "");
15+
1616
let variable = "";
17-
cx.emit_lint(DUMMY, expr, variable, |_| {});
17+
cx.emit_lint(DUMMY, expr, variable);
1818
}
1919

2020
pub fn warn_about_message<'ast>(cx: &AstContext<'ast>, expr: ExprKind<'ast>) {
21-
cx.emit_lint(DUMMY, expr, "X <-- starting with upper case", |_| {});
22-
cx.emit_lint(DUMMY, expr, "Hey <-- starting with upper case", |_| {});
21+
cx.emit_lint(DUMMY, expr, "X <-- starting with upper case");
22+
cx.emit_lint(DUMMY, expr, "Hey <-- starting with upper case");
2323
}
2424

2525
fn main() {}

0 commit comments

Comments
 (0)