Merge #121

121: Emit lints using a diagnostic builder r=Niki4tap a=xFrednet

This PR adds a new `DiagnosticBuilder` to the API to create beautiful diagnostics with help and note messages. This will also allow us to include the expression span, in the print tests. Generally, I'm pretty happy with the concept :)

---

Adding new context callbacks to the API currently requires a lot of boilerplate code. I doubt that we can avoid that right now, but I'll try to look at options to generate a part of it. That would at least help with the implementation and review part.

---

r? `@Niki4tap` Sorry for requesting this review direction after the last one. You are welcome to take your time or pass the review if you don't have time. It's also totally fine, if you just review the API and doc changes. The backend is mostly verified by the tests :)

Closes #47
Closes #92 (This PR adds the `id()` and `span()` method to `StmtKind`)

Co-authored-by: xFrednet <[email protected]>

marker_adapter/src/context.rs

@@ -1,11 +1,17 @@
+#![allow(
+    clippy::needless_lifetimes,
+    reason = "the lifetimes are destroyed by unsafe, but help with readability"
+)]
+
 use marker_api::{
     ast::{
         item::{Body, ItemKind},
         BodyId, ExprId, ItemId, Span, SpanOwner, SymbolId,
     },
     context::DriverCallbacks,
+    diagnostic::{Diagnostic, EmissionNode},
     ffi::{self, FfiOption},
-    lint::Lint,
+    lint::{Level, Lint},
 };
 
 /// ### Safety
@@ -34,7 +40,8 @@ impl<'ast> DriverContextWrapper<'ast> {
     pub fn create_driver_callback(&'ast self) -> DriverCallbacks<'ast> {
         DriverCallbacks {
             driver_context: unsafe { &*(self as *const DriverContextWrapper).cast::<()>() },
-            emit_lint,
+            lint_level_at,
+            emit_diag,
             item,
             body,
             get_span,
@@ -45,6 +52,17 @@ impl<'ast> DriverContextWrapper<'ast> {
     }
 }
 
+#[allow(improper_ctypes_definitions, reason = "fp because `EmissionNode` are non-exhaustive")]
+extern "C" fn lint_level_at(data: &(), lint: &'static Lint, node: EmissionNode) -> Level {
+    let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
+    wrapper.driver_cx.lint_level_at(lint, node)
+}
+
+extern "C" fn emit_diag<'a, 'ast>(data: &(), diag: &Diagnostic<'a, 'ast>) {
+    let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
+    wrapper.driver_cx.emit_diag(diag);
+}
+
 #[allow(improper_ctypes_definitions, reason = "fp because `ItemKind` is non-exhaustive")]
 extern "C" fn item<'ast>(data: &(), id: ItemId) -> FfiOption<ItemKind<'ast>> {
     let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
@@ -56,22 +74,17 @@ extern "C" fn body<'ast>(data: &(), id: BodyId) -> &'ast Body<'ast> {
     wrapper.driver_cx.body(id)
 }
 
-extern "C" fn emit_lint(data: &(), lint: &'static Lint, msg: ffi::Str, span: &Span<'_>) {
-    let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
-    wrapper.driver_cx.emit_lint(lint, (&msg).into(), span);
-}
-
 extern "C" fn get_span<'ast>(data: &(), owner: &SpanOwner) -> &'ast Span<'ast> {
     let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
     wrapper.driver_cx.get_span(owner)
 }
 
-extern "C" fn span_snippet<'ast>(data: &(), span: &Span) -> ffi::FfiOption<ffi::Str<'ast>> {
+extern "C" fn span_snippet<'ast>(data: &(), span: &Span) -> ffi::FfiOption<ffi::FfiStr<'ast>> {
     let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
     wrapper.driver_cx.span_snippet(span).map(Into::into).into()
 }
 
-extern "C" fn symbol_str<'ast>(data: &(), sym: SymbolId) -> ffi::Str<'ast> {
+extern "C" fn symbol_str<'ast>(data: &(), sym: SymbolId) -> ffi::FfiStr<'ast> {
     let wrapper = unsafe { &*(data as *const ()).cast::<DriverContextWrapper>() };
     wrapper.driver_cx.symbol_str(sym).into()
 }
@@ -82,9 +95,11 @@ extern "C" fn resolve_method_target(data: &(), id: ExprId) -> ItemId {
 }
 
 pub trait DriverContext<'ast> {
+    fn lint_level_at(&'ast self, lint: &'static Lint, node: EmissionNode) -> Level;
+    fn emit_diag(&'ast self, diag: &Diagnostic<'_, 'ast>);
+
     fn item(&'ast self, api_id: ItemId) -> Option<ItemKind<'ast>>;
     fn body(&'ast self, api_id: BodyId) -> &'ast Body<'ast>;
-    fn emit_lint(&'ast self, lint: &'static Lint, msg: &str, span: &Span<'ast>);
     fn get_span(&'ast self, owner: &SpanOwner) -> &'ast Span<'ast>;
     fn span_snippet(&'ast self, span: &Span) -> Option<&'ast str>;
     fn symbol_str(&'ast self, api_id: SymbolId) -> &'ast str;

marker_api/src/ast/common.rs

@@ -19,31 +19,6 @@ pub enum Edition {
     Edition2021,
 }
 
-// FIXME: This will need to be updated according to rust-lang/rustfix#200
-#[non_exhaustive]
-#[derive(Copy, Clone, Eq, PartialEq, Debug)]
-pub enum Applicability {
-    /// The suggestion is definitely what the user intended, or maintains the exact meaning of the
-    /// code. This suggestion should be automatically applied.
-    ///
-    /// In case of multiple `MachineApplicable` suggestions (whether as part of
-    /// the same `multipart_suggestion` or not), all of them should be
-    /// automatically applied.
-    MachineApplicable,
-
-    /// The suggestion may be what the user intended, but it is uncertain. The suggestion should
-    /// result in valid Rust code if it is applied.
-    MaybeIncorrect,
-
-    /// The suggestion contains placeholders like `(...)` or `{ /* fields */ }`. The suggestion
-    /// cannot be applied automatically because it will not result in valid Rust code. The user
-    /// will need to fill in the placeholders.
-    HasPlaceholders,
-
-    /// The suggestion can not be automatically applied or the applicability is unknown.
-    Unspecified,
-}
-
 #[non_exhaustive]
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
 pub enum Abi {

marker_api/src/ast/common/id.rs

@@ -54,6 +54,11 @@ new_id! {
     pub VariantId: u64
 }
 
+new_id! {
+    /// This ID uniquely identifies a field inside a struct during linting.
+    pub FieldId: u64
+}
+
 new_id! {
     /// This ID uniquely identifies a user defined type during linting.
     pub TyDefId: u64
@@ -111,3 +116,35 @@ new_id! {
     #[cfg_attr(feature = "driver-api", visibility::make(pub))]
     pub(crate) SymbolId: u32
 }
+
+new_id! {
+    /// This ID uniquely identifies a statement during linting.
+    pub StmtId: StmtIdInner
+}
+
+impl StmtId {
+    /// This is an extra constructor for api internal use. The `new_id` macro
+    /// only generates methods for drivers.
+    pub(crate) fn ast_new(data: StmtIdInner) -> Self {
+        Self { data }
+    }
+}
+
+#[repr(C)]
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "driver-api", visibility::make(pub))]
+#[allow(clippy::exhaustive_enums)] // Only driver public
+pub(crate) enum StmtIdInner {
+    Expr(ExprId),
+    Item(ItemId),
+    LetStmt(LetStmtId),
+}
+
+new_id! {
+    /// **Unstable**
+    ///
+    /// This id is used to identify a `let` statement. It's intended to be used
+    /// inside [`StmtIdInner`]
+    #[cfg_attr(feature = "driver-api", visibility::make(pub))]
+    pub(crate) LetStmtId: u64
+}

marker_api/src/ast/common/span.rs

@@ -1,8 +1,8 @@
 use std::{marker::PhantomData, path::PathBuf};
 
-use crate::context::with_cx;
+use crate::{context::with_cx, diagnostic::Applicability};
 
-use super::{Applicability, AstPath, ItemId, SpanId, SymbolId};
+use super::{AstPath, ItemId, SpanId, SymbolId};
 
 #[repr(C)]
 #[doc(hidden)]

marker_api/src/ast/item.rs

@@ -224,6 +224,12 @@ macro_rules! impl_item_data {
                 todo!()
             }
         }
+
+        impl<'ast> From<&'ast $self_name<'ast>> for crate::ast::item::ItemKind<'ast> {
+            fn from(value: &'ast $self_name<'ast>) -> Self {
+                $crate::ast::item::ItemKind::$enum_name(value)
+            }
+        }
     };
 }
 

marker_api/src/ast/item/adt_item.rs

@@ -1,6 +1,6 @@
 use crate::ast::generic::GenericParams;
 use crate::ast::ty::TyKind;
-use crate::ast::{Span, SpanId, SymbolId, VariantId};
+use crate::ast::{FieldId, Span, SpanId, SymbolId, VariantId};
 use crate::context::with_cx;
 use crate::ffi::FfiSlice;
 
@@ -263,13 +263,18 @@ impl<'ast> AdtKind<'ast> {
 #[repr(C)]
 #[derive(Debug)]
 pub struct Field<'ast> {
+    id: FieldId,
     vis: Visibility<'ast>,
     ident: SymbolId,
     ty: TyKind<'ast>,
     span: SpanId,
 }
 
 impl<'ast> Field<'ast> {
+    pub fn id(&self) -> FieldId {
+        self.id
+    }
+
     /// The [`Visibility`] of this item.
     pub fn visibility(&self) -> &Visibility<'ast> {
         &self.vis
@@ -294,7 +299,13 @@ impl<'ast> Field<'ast> {
 
 #[cfg(feature = "driver-api")]
 impl<'ast> Field<'ast> {
-    pub fn new(vis: Visibility<'ast>, ident: SymbolId, ty: TyKind<'ast>, span: SpanId) -> Self {
-        Self { vis, ident, ty, span }
+    pub fn new(id: FieldId, vis: Visibility<'ast>, ident: SymbolId, ty: TyKind<'ast>, span: SpanId) -> Self {
+        Self {
+            id,
+            vis,
+            ident,
+            ty,
+            span,
+        }
     }
 }

marker_api/src/ast/stmt.rs

@@ -1,6 +1,6 @@
 use crate::{context::with_cx, ffi::FfiOption};
 
-use super::{expr::ExprKind, item::ItemKind, pat::PatKind, ty::TyKind, Span, SpanId};
+use super::{expr::ExprKind, item::ItemKind, pat::PatKind, ty::TyKind, LetStmtId, Span, SpanId, StmtId, StmtIdInner};
 
 #[repr(C)]
 #[non_exhaustive]
@@ -11,9 +11,28 @@ pub enum StmtKind<'ast> {
     Expr(ExprKind<'ast>),
 }
 
+impl<'ast> StmtKind<'ast> {
+    pub fn id(&self) -> StmtId {
+        match self {
+            StmtKind::Item(node) => StmtId::ast_new(StmtIdInner::Item(node.id())),
+            StmtKind::Let(node) => node.id(),
+            StmtKind::Expr(node) => StmtId::ast_new(StmtIdInner::Expr(node.id())),
+        }
+    }
+
+    pub fn span(&self) -> &Span<'ast> {
+        match self {
+            StmtKind::Item(node) => node.span(),
+            StmtKind::Let(node) => node.span(),
+            StmtKind::Expr(node) => node.span(),
+        }
+    }
+}
+
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct LetStmt<'ast> {
+    id: LetStmtId,
     span: SpanId,
     pat: PatKind<'ast>,
     ty: FfiOption<TyKind<'ast>>,
@@ -22,6 +41,10 @@ pub struct LetStmt<'ast> {
 }
 
 impl<'ast> LetStmt<'ast> {
+    pub fn id(&self) -> StmtId {
+        StmtId::ast_new(StmtIdInner::LetStmt(self.id))
+    }
+
     pub fn span(&self) -> &Span<'ast> {
         with_cx(self, |cx| cx.get_span(self.span))
     }
@@ -50,13 +73,15 @@ impl<'ast> LetStmt<'ast> {
 #[cfg(feature = "driver-api")]
 impl<'ast> LetStmt<'ast> {
     pub fn new(
+        id: LetStmtId,
         span: SpanId,
         pat: PatKind<'ast>,
         ty: Option<TyKind<'ast>>,
         init: Option<ExprKind<'ast>>,
         els: Option<ExprKind<'ast>>,
     ) -> Self {
         Self {
+            id,
             span,
             pat,
             ty: ty.into(),

marker_api/src/context.rs

@@ -5,8 +5,9 @@ use crate::{
         item::{Body, ItemKind},
         BodyId, ExprId, ItemId, Span, SpanOwner, SymbolId,
     },
+    diagnostic::{Diagnostic, DiagnosticBuilder, EmissionNode},
     ffi,
-    lint::Lint,
+    lint::{Level, Lint},
 };
 
 thread_local! {
@@ -105,19 +106,31 @@ impl<'ast> AstContext<'ast> {
 }
 
 impl<'ast> AstContext<'ast> {
-    /// This function emits a lint at the current node with the given
-    /// message and span.
-    ///
-    /// For rustc the text output will look roughly to this:
-    /// ```text
-    /// error: ducks can't talk
-    ///  --> $DIR/file.rs:17:5
-    ///    |
-    /// 17 |     println!("The duck said: 'Hello, World!'");
-    ///    |
-    /// ```
-    pub fn emit_lint(&self, lint: &'static Lint, msg: &str, span: &Span<'ast>) {
-        self.driver.call_emit_lint(lint, msg, span);
+    pub fn lint_level_at(&self, lint: &'static Lint, node: impl Into<EmissionNode>) -> Level {
+        self.driver.call_lint_level_at(lint, node.into())
+    }
+
+    #[allow(clippy::needless_pass_by_value)] // `&impl ToString`
+    pub fn emit_lint<F>(
+        &self,
+        lint: &'static Lint,
+        node: impl Into<EmissionNode>,
+        msg: impl ToString,
+        span: &Span<'ast>,
+        decorate: F,
+    ) where
+        F: FnOnce(&mut DiagnosticBuilder<'ast>),
+    {
+        let node = node.into();
+        if self.lint_level_at(lint, node) != Level::Allow {
+            let mut builder = DiagnosticBuilder::new(lint, node, msg.to_string(), span.clone());
+            decorate(&mut builder);
+            builder.emit(self);
+        }
+    }
+
+    pub(crate) fn emit_diagnostic<'a>(&self, diag: &'a Diagnostic<'a, 'ast>) {
+        self.driver.call_emit_diagnostic(diag);
     }
 
     /// This returns the [`ItemKind`] belonging to the given [`ItemId`]. It can
@@ -177,30 +190,38 @@ struct DriverCallbacks<'ast> {
     /// get its own context.
     pub driver_context: &'ast (),
 
+    // Lint emission and information
+    pub lint_level_at: extern "C" fn(&'ast (), &'static Lint, EmissionNode) -> Level,
+    pub emit_diag: for<'a> extern "C" fn(&'ast (), &'a Diagnostic<'a, 'ast>),
+
     // Public utility
-    pub emit_lint: for<'a> extern "C" fn(&'ast (), &'static Lint, ffi::Str<'a>, &Span<'ast>),
     pub item: extern "C" fn(&'ast (), id: ItemId) -> ffi::FfiOption<ItemKind<'ast>>,
     pub body: extern "C" fn(&'ast (), id: BodyId) -> &'ast Body<'ast>,
 
     // Internal utility
     pub get_span: extern "C" fn(&'ast (), &SpanOwner) -> &'ast Span<'ast>,
-    pub span_snippet: extern "C" fn(&'ast (), &Span) -> ffi::FfiOption<ffi::Str<'ast>>,
-    pub symbol_str: extern "C" fn(&'ast (), SymbolId) -> ffi::Str<'ast>,
+    pub span_snippet: extern "C" fn(&'ast (), &Span) -> ffi::FfiOption<ffi::FfiStr<'ast>>,
+    pub symbol_str: extern "C" fn(&'ast (), SymbolId) -> ffi::FfiStr<'ast>,
     pub resolve_method_target: extern "C" fn(&'ast (), ExprId) -> ItemId,
 }
 
 impl<'ast> DriverCallbacks<'ast> {
-    fn call_emit_lint(&self, lint: &'static Lint, msg: &str, span: &Span<'ast>) {
-        (self.emit_lint)(self.driver_context, lint, msg.into(), span);
+    fn call_lint_level_at(&self, lint: &'static Lint, node: EmissionNode) -> Level {
+        (self.lint_level_at)(self.driver_context, lint, node)
+    }
+
+    fn call_emit_diagnostic<'a>(&self, diag: &'a Diagnostic<'a, 'ast>) {
+        (self.emit_diag)(self.driver_context, diag);
     }
+
     fn call_item(&self, id: ItemId) -> Option<ItemKind<'ast>> {
         (self.item)(self.driver_context, id).copy()
     }
     fn call_get_span(&self, span_owner: &SpanOwner) -> &'ast Span<'ast> {
         (self.get_span)(self.driver_context, span_owner)
     }
     fn call_span_snippet(&self, span: &Span) -> Option<String> {
-        let result: Option<ffi::Str> = (self.span_snippet)(self.driver_context, span).into();
+        let result: Option<ffi::FfiStr> = (self.span_snippet)(self.driver_context, span).into();
         result.map(|x| x.to_string())
     }
     fn call_symbol_str(&self, sym: SymbolId) -> &'ast str {