Skip to content

Commit

Permalink
use annotate_snippets for error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmehri01
rmehri01 committed Jan 9, 2025
1 parent c9c4ad0 commit 5213e6f
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 33 deletions.
1 change: 1 addition & 0 deletions libc-test/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ libc = { path = "..", version = "1.0.0-alpha.1", default-features = false }
syn = { version = "2.0.91", features = ["full", "visit"] }
proc-macro2 = { version = "1.0.92", features = ["span-locations"] }
glob = "0.3.2"
annotate-snippets = { version = "0.11.5", features = ["testing-colors"] }

[build-dependencies]
cc = "1.0.83"
Expand Down
124 changes: 91 additions & 33 deletions libc-test/test/style/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
//! This is split out so that the implementation itself can be tested
//! separately, see test/check_style.rs for how it's used.
use std::fmt::Display;
use std::fs;
use std::ops::Deref;
use std::path::{Path, PathBuf};

use annotate_snippets::{Level, Renderer, Snippet};
use proc_macro2::Span;
use syn::parse::{Parse, ParseStream};
use syn::spanned::Spanned;
use syn::visit::{self, Visit};
Expand All @@ -20,6 +21,9 @@ pub type Result<T> = std::result::Result<T, Error>;
#[derive(Default)]
pub struct StyleChecker {
state: State,
/// Span of the first item encountered in this state
/// to use in help diagnostic text.
state_span: Option<Span>,
// FIXME: see StyleChecker::set_state
_s_macros: usize,
f_macros: usize,
Expand Down Expand Up @@ -88,28 +92,65 @@ impl StyleChecker {
return Ok(());
}

let renderer = Renderer::styled();
for error in self.errors {
eprintln!("{error}");
let source = fs::read_to_string(&error.path)?;

let mut snippet = Snippet::source(&source)
.origin(error.path.to_str().expect("path to be UTF-8"))
.fold(true)
.annotation(Level::Error.span(error.span.byte_range()).label(&error.msg));
if let Some((help_span, help_msg)) = &error.help {
if let Some(help_span) = help_span {
snippet = snippet
.annotation(Level::Help.span(help_span.byte_range()).label(help_msg));
}
}

let mut msg = Level::Error.title(&error.title).snippet(snippet);
if let Some((help_span, help_msg)) = &error.help {
if help_span.is_none() {
msg = msg.footer(Level::Help.title(help_msg))
}
}

eprintln!("{}", renderer.render(msg));
}

Err("some tests failed".into())
}

fn set_state(&mut self, new_state: State, line: usize) {
fn set_state(&mut self, new_state: State, span: Span) {
if self.state > new_state {
self.error(
line,
self.error_with_help(
"incorrect module layout".to_string(),
span,
format!(
"{} found after {} when it belongs before",
new_state.desc(),
self.state.desc()
),
(
Some(
self.state_span
.expect("state_span should be set since we are on a second state"),
),
format!(
"move the {} to before this {}",
new_state.desc(),
self.state.desc()
),
),
);
}

if self.f_macros == 2 {
self.f_macros += 1;
self.error(line, "multiple f! macros in one module".to_string());
self.error(
"multiple f! macros in one module".to_string(),
span,
"found multiple f! macros in one module".to_string(),
);
}

// FIXME(#4109): multiple should be allowed if at least one is `cfg(not) within `cfg_if`.
Expand All @@ -120,6 +161,7 @@ impl StyleChecker {
// }

self.state = new_state;
self.state_span = Some(span);
}

/// Visit the items inside the [ExprCfgIf], restoring the state after
Expand All @@ -145,64 +187,84 @@ impl StyleChecker {
self.state = initial_state;
}

fn error(&mut self, line: usize, msg: String) {
fn push_error(&mut self, title: String, span: Span, msg: String, help: Option<Help>) {
self.errors.push(FileError {
path: self.path.clone(),
line,
title,
span,
msg,
help,
});
}

fn error(&mut self, title: String, span: Span, msg: String) {
self.push_error(title, span, msg, None);
}

fn error_with_help(&mut self, title: String, span: Span, msg: String, help: Help) {
self.push_error(title, span, msg, Some(help));
}
}

impl<'ast> Visit<'ast> for StyleChecker {
fn visit_meta_list(&mut self, meta_list: &'ast syn::MetaList) {
let line = meta_list.span().start().line;
let span = meta_list.span();
let meta_str = meta_list.tokens.to_string();
if meta_list.path.is_ident("cfg")
&& !(meta_str.contains("target_endian") || meta_str.contains("target_arch"))
&& self.state != State::Structs
{
self.error(
line,
"use cfg_if! and submodules instead of #[cfg]".to_string(),
self.error_with_help(
"#[cfg] over cfg_if!".to_string(),
span,
String::new(),
(
None,
"use cfg_if! and submodules instead of #[cfg]".to_string(),
),
);
} else if meta_list.path.is_ident("derive")
&& (meta_str.contains("Copy") || meta_str.contains("Clone"))
{
self.error(line, "impl Copy and Clone manually".to_string());
self.error_with_help(
"impl Copy and Clone manually".to_string(),
span,
"found manual implementation of Copy and/or Clone".to_string(),
(None, "use one of the s! macros instead".to_string()),
);
}

visit::visit_meta_list(self, meta_list);
}

fn visit_item_use(&mut self, item_use: &'ast syn::ItemUse) {
let line = item_use.span().start().line;
let span = item_use.span();
let new_state = if matches!(item_use.vis, syn::Visibility::Public(_)) {
State::Modules
} else {
State::Imports
};
self.set_state(new_state, line);
self.set_state(new_state, span);

visit::visit_item_use(self, item_use);
}

fn visit_item_const(&mut self, item_const: &'ast syn::ItemConst) {
let line = item_const.span().start().line;
self.set_state(State::Constants, line);
let span = item_const.span();
self.set_state(State::Constants, span);

visit::visit_item_const(self, item_const);
}

fn visit_item_type(&mut self, item_type: &'ast syn::ItemType) {
let line = item_type.span().start().line;
self.set_state(State::Typedefs, line);
let span = item_type.span();
self.set_state(State::Typedefs, span);

visit::visit_item_type(self, item_type);
}

fn visit_macro(&mut self, mac: &'ast syn::Macro) {
let line = mac.span().start().line;
let span = mac.span();
if mac.path.is_ident("cfg_if") {
let cfg_expr_if: ExprCfgIf = mac
.parse_body()
Expand All @@ -226,22 +288,22 @@ impl<'ast> Visit<'ast> for StyleChecker {
} else {
self.state
};
self.set_state(new_state, line);
self.set_state(new_state, span);
}

visit::visit_macro(self, mac);
}

fn visit_item_foreign_mod(&mut self, item_foreign_mod: &'ast syn::ItemForeignMod) {
let line = item_foreign_mod.span().start().line;
self.set_state(State::Functions, line);
let span = item_foreign_mod.span();
self.set_state(State::Functions, span);

visit::visit_item_foreign_mod(self, item_foreign_mod);
}

fn visit_item_mod(&mut self, item_mod: &'ast syn::ItemMod) {
let line = item_mod.span().start().line;
self.set_state(State::Modules, line);
let span = item_mod.span();
self.set_state(State::Modules, span);

visit::visit_item_mod(self, item_mod);
}
Expand Down Expand Up @@ -308,14 +370,10 @@ impl State {
#[derive(Debug)]
struct FileError {
path: PathBuf,
line: usize,
span: Span,
title: String,
msg: String,
help: Option<Help>,
}

impl Display for FileError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}:{}: {}", self.path.display(), self.line, self.msg)
}
}

impl std::error::Error for FileError {}
type Help = (Option<Span>, String);

0 comments on commit 5213e6f

Please sign in to comment.