Skip to content

Commit

Permalink
API: Remove decorate argument from emit_lint and add docs (Breaking)
Browse files Browse the repository at this point in the history
  • Loading branch information
@xFrednet
xFrednet committed Sep 25, 2023
1 parent 2527da4 commit 3d05704
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 79 deletions.
127 changes: 116 additions & 11 deletions marker_api/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,129 @@ impl<'ast> AstContext<'ast> {
self.driver.call_lint_level_at(lint, node.emission_node_id())
}

/// This function is used to emit a lint.
///
/// Every lint emission, is bound to one specific node in the AST. This
/// node is used to check the lint level and as the main [`Span`] of the
/// diagnostic message. See [`EmissionNode`] for more information.
///
/// The lint message, will be the main message at the start of the created
/// diagnostic. This message and all messages emitted as part of the created
/// diagnostic should start with a lower letter, according to [rustc's dev guide].
///
/// The function will return a [`DiagnosticBuilder`] which can be used to decorate
/// the diagnostic message, with notes and help messages. The diagnostic message will
/// be emitted when the builder instance is dropped.
///
/// [rustc's dev guide]: <https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-output-style-guide>
///
/// ## Example 1
///
/// ```
/// # use marker_api::prelude::*;
/// # marker_api::declare_lint!(
/// # /// Dummy
/// # LINT,
/// # Warn,
/// # );
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
/// cx.emit_lint(LINT, node, "<lint message>");
/// # }
/// ```
///
/// The code above will roughly generate the following error message:
///
/// ```text
/// warning: <lint message> <-- The message that is set by this function
/// --> path/file.rs:1:1
/// |
/// 1 | node
/// | ^^^^
/// |
/// ```
///
/// ## Example 2
///
/// ```
/// # use marker_api::prelude::*;
/// # marker_api::declare_lint!(
/// # /// Dummy
/// # LINT,
/// # Warn,
/// # );
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
/// cx
/// .emit_lint(LINT, node, "<lint message>")
/// .help("<text>");
/// # }
/// ```
///
/// The [`DiagnosticBuilder::help`] call will add a help message like this:
///
/// ```text
/// warning: <lint message>
/// --> path/file.rs:1:1
/// |
/// 1 | node
/// | ^^^^
/// |
/// = help: <text> <-- The added help message
/// ```
///
/// ## Example 3
///
/// Creating suggestions can impact the performance. The
/// [`DiagnosticBuilder::decorate`] function can be used, to only add more
/// context to the lint emission if it will actually be emitted. This will
/// save performance, when the lint is allowed at the [`EmissionNode`]
///
/// ```
/// # use marker_api::prelude::*;
/// # marker_api::declare_lint!(
/// # /// Dummy
/// # LINT,
/// # Warn,
/// # );
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
/// cx.emit_lint(LINT, node, "<lint message>").decorate(|diag| {
/// // This closure is only called, if the lint is enabled. Here you
/// // can create a beautiful help message.
/// diag.help("<text>");
/// });
/// # }
/// ```
///
/// This will create the same help message as in example 2, but it will be faster
/// if the lint is enabled. The emitted message would look like this:
/// ```text
/// warning: <lint message>
/// --> path/file.rs:1:1
/// |
/// 1 | node
/// | ^^^^
/// |
/// = help: <text> <-- The added help message
/// ```
#[allow(clippy::needless_pass_by_value)] // `&impl ToString`
pub fn emit_lint<F>(&self, lint: &'static Lint, node: impl EmissionNode<'ast>, msg: impl ToString, decorate: F)
where
F: FnOnce(&mut DiagnosticBuilder<'ast>),
{
pub fn emit_lint(
&self,
lint: &'static Lint,
node: impl EmissionNode<'ast>,
msg: impl ToString,
) -> DiagnosticBuilder<'ast> {
let id = node.emission_node_id();
let Some(span) = node.emission_span(self) else {
// If the `Span` is none, we can't emit a diagnostic message for it.
return;
return DiagnosticBuilder::new_dummy(lint, id);
};
if matches!(lint.report_in_macro, MacroReport::No) && span.is_from_expansion() {
return;
return DiagnosticBuilder::new_dummy(lint, id);
}
let id = node.emission_node_id();
if self.lint_level_at(lint, node) != Level::Allow {
let mut builder = DiagnosticBuilder::new(lint, id, msg.to_string(), span);
decorate(&mut builder);
builder.emit(self);
if self.lint_level_at(lint, node) == Level::Allow {
return DiagnosticBuilder::new_dummy(lint, id);
}

DiagnosticBuilder::new(lint, id, msg.to_string(), span)
}

pub(crate) fn emit_diagnostic<'a>(&self, diag: &'a Diagnostic<'a, 'ast>) {
Expand Down
148 changes: 114 additions & 34 deletions marker_api/src/diagnostic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::fmt::Debug;

use crate::{
ast::{ExprId, FieldId, ItemId, Span, StmtId, VariantId},
context::AstContext,
context::{with_cx, AstContext},
ffi::{FfiSlice, FfiStr},
lint::Lint,
};
Expand All @@ -17,19 +17,34 @@ pub struct DiagnosticBuilder<'ast> {
lint: &'static Lint,
node: EmissionNodeId,
msg: String,
span: Span<'ast>,
span: Option<Span<'ast>>,
parts: Vec<DiagnosticPart<String, Span<'ast>>>,
/// This flag indicates, if the diagnostic messages should actually be emitted
/// at the end. This might be false, if the lint is allowed at the emission location.
emit_lint: bool,
}

#[allow(clippy::needless_pass_by_value)] // `&impl ToString` doesn't work
impl<'ast> DiagnosticBuilder<'ast> {
pub(crate) fn new_dummy(lint: &'static Lint, node: EmissionNodeId) -> Self {
Self {
lint,
node,
msg: String::new(),
span: None,
parts: vec![],
emit_lint: false,
}
}

pub(crate) fn new(lint: &'static Lint, node: EmissionNodeId, msg: String, span: Span<'ast>) -> Self {
Self {
lint,
msg,
node,
span,
span: Some(span),
parts: vec![],
emit_lint: true,
}
}

Expand All @@ -45,7 +60,9 @@ impl<'ast> DiagnosticBuilder<'ast> {
/// |
/// ```
pub fn set_main_message(&mut self, msg: impl ToString) -> &mut Self {
self.msg = msg.to_string();
if self.emit_lint {
self.msg = msg.to_string();
}
self
}

Expand All @@ -63,7 +80,10 @@ impl<'ast> DiagnosticBuilder<'ast> {
/// |
/// ```
pub fn set_main_span(&mut self, span: &Span<'ast>) -> &mut Self {
self.span = span.clone();
if self.emit_lint {
self.span = Some(span.clone());
}

self
}

Expand All @@ -82,8 +102,12 @@ impl<'ast> DiagnosticBuilder<'ast> {
/// ```
///
/// [`Self::span_note`] can be used to highlight a relevant [`Span`].
pub fn note(&mut self, msg: impl ToString) {
self.parts.push(DiagnosticPart::Note { msg: msg.to_string() });
pub fn note(&mut self, msg: impl ToString) -> &mut Self {
if self.emit_lint {
self.parts.push(DiagnosticPart::Note { msg: msg.to_string() });
}

self
}

/// This function adds a note with a [`Span`] to the diagnostic message.
Expand All @@ -106,11 +130,15 @@ impl<'ast> DiagnosticBuilder<'ast> {
/// ```
///
/// [`Self::note`] can be used to add text notes without a span.
pub fn span_note(&mut self, msg: impl ToString, span: &Span<'ast>) {
self.parts.push(DiagnosticPart::NoteSpan {
msg: msg.to_string(),
span: span.clone(),
});
pub fn span_note(&mut self, msg: impl ToString, span: &Span<'ast>) -> &mut Self {
if self.emit_lint {
self.parts.push(DiagnosticPart::NoteSpan {
msg: msg.to_string(),
span: span.clone(),
});
}

self
}

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

self
}

Expand All @@ -155,11 +186,15 @@ impl<'ast> DiagnosticBuilder<'ast> {
///
/// [`Self::help`] can be used to add a text help message without a [`Span`].
/// [`Self::span_suggestion`] can be used to add a help message with a suggestion.
pub fn span_help(&mut self, msg: impl ToString, span: &Span<'ast>) {
self.parts.push(DiagnosticPart::HelpSpan {
msg: msg.to_string(),
span: span.clone(),
});
pub fn span_help(&mut self, msg: impl ToString, span: &Span<'ast>) -> &mut Self {
if self.emit_lint {
self.parts.push(DiagnosticPart::HelpSpan {
msg: msg.to_string(),
span: span.clone(),
});
}

self
}

/// This function adds a spanned help message with a suggestion. The suggestion
Expand All @@ -184,25 +219,70 @@ impl<'ast> DiagnosticBuilder<'ast> {
span: &Span<'ast>,
suggestion: impl ToString,
app: Applicability,
) {
self.parts.push(DiagnosticPart::Suggestion {
msg: msg.to_string(),
span: span.clone(),
sugg: suggestion.to_string(),
app,
});
) -> &mut Self {
if self.emit_lint {
self.parts.push(DiagnosticPart::Suggestion {
msg: msg.to_string(),
span: span.clone(),
sugg: suggestion.to_string(),
app,
});
}
self
}

/// This function takes a closure, that is only executed, if the created
/// diagnostic is actually emitted in the end. This is useful for crafting
/// suggestions. Having them in a conditional closure will speedup the
/// linting process if the lint is allowed at a given location.
///
/// ```
/// # use marker_api::prelude::*;
/// # marker_api::declare_lint!(
/// # /// Dummy
/// # LINT,
/// # Warn,
/// # );
/// # fn value_provider<'ast>(cx: &AstContext<'ast>, node: ExprKind<'ast>) {
/// cx.emit_lint(LINT, node, "<lint message>").decorate(|diag| {
/// // This closure is only called, if the lint is enabled. Here you
/// // can create a beautiful help message.
/// diag.help("<text>");
/// });
/// # }
/// ```
pub fn decorate<F>(&mut self, decorate: F) -> &mut Self
where
F: FnOnce(&mut DiagnosticBuilder<'ast>),
{
if self.emit_lint {
decorate(self);
}
self
}

pub(crate) fn emit<'builder>(&'builder self, cx: &AstContext<'ast>) {
let parts: Vec<_> = self.parts.iter().map(DiagnosticPart::to_ffi_part).collect();
let diag = Diagnostic {
lint: self.lint,
msg: self.msg.as_str().into(),
node: self.node,
span: &self.span,
parts: parts.as_slice().into(),
};
cx.emit_diagnostic(&diag);
if self.emit_lint {
let parts: Vec<_> = self.parts.iter().map(DiagnosticPart::to_ffi_part).collect();
let span = self
.span
.as_ref()
.expect("always Some, if `DiagnosticBuilder::emit_lint` is true");
let diag = Diagnostic {
lint: self.lint,
msg: self.msg.as_str().into(),
node: self.node,
span,
parts: parts.as_slice().into(),
};
cx.emit_diagnostic(&diag);
}
}
}

impl<'ast> Drop for DiagnosticBuilder<'ast> {
fn drop(&mut self) {
with_cx(self, |cx| self.emit(cx));
}
}

Expand Down
1 change: 0 additions & 1 deletion marker_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ fn check_msg<'ast>(cx: &AstContext<'ast>, msg_expr: ExprKind<'ast>) {
DIAG_MSG_UPPERCASE_START,
msg_expr,
"this message starts with an uppercase character",
|_| {},
);
}
}
14 changes: 7 additions & 7 deletions marker_lints/tests/ui/diag_msg_uppercase_start.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ marker_api::declare_lint!(
);

pub fn accept_message<'ast>(cx: &AstContext<'ast>, expr: ExprKind<'ast>) {
cx.emit_lint(DUMMY, expr, "x <-- this is cool", |_| {});
cx.emit_lint(DUMMY, expr, "=^.^= <-- Interesting, but valid", |_| {});
cx.emit_lint(DUMMY, expr, "", |_| {});
cx.emit_lint(DUMMY, expr, "x <-- this is cool");
cx.emit_lint(DUMMY, expr, "=^.^= <-- Interesting, but valid");
cx.emit_lint(DUMMY, expr, "");

let variable = "";
cx.emit_lint(DUMMY, expr, variable, |_| {});
cx.emit_lint(DUMMY, expr, variable);
}

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

fn main() {}
Loading

0 comments on commit 3d05704

Please sign in to comment.