Skip to content

Commit

Permalink
refactor: allow multiple footers in error/warning messages
Browse files Browse the repository at this point in the history
With this change we open the possibility of adding multiple footers to error messages.
  • Loading branch information
plusvic committed Sep 18, 2024
1 parent 92651c7 commit 67c7979
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 48 deletions.
14 changes: 13 additions & 1 deletion go/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ type CompileError struct {
Title string `json:"title"`
// Each of the labels in the error report.
Labels []Label `json:"labels"`
// Each of the footers in the error report.
Footers []Footer `json:"footers"`
// The error's full report, as shown by the command-line tool.
Text string `json:"text"`
}
Expand All @@ -131,8 +133,10 @@ type Warning struct {
Code string `json:"code"`
// Error title (e.g: "slow pattern").
Title string `json:"title"`
// Each of the labels in the error report.
// Each of the labels in the warning report.
Labels []Label `json:"labels"`
// Each of the footers in the warning report.
Footers []Footer `json:"footers"`
// The error's full report, as shown by the command-line tool.
Text string `json:"text"`
}
Expand All @@ -149,6 +153,14 @@ type Label struct {
Text string `json:"text"`
}

// Footer represents a footer in a [CompileError].
type Footer struct {
// Footer's level (e.g: "error", "warning", "info", "note", "help").
Level string `json:"level"`
// Footer's text.
Text string `json:"text"`
}

// Span represents the starting and ending point of some piece of source
// code.
type Span struct {
Expand Down
30 changes: 30 additions & 0 deletions go/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,33 @@ func TestErrors(t *testing.T) {
c.AddSource("rule test_1 { condition: true }")
assert.Equal(t, []CompileError{}, c.Errors())

assert.Equal(t, []Warning{
{
Code: "invariant_expr",
Title: "invariant boolean expression",
Labels: []Label{
{
Level: "warning",
Span: Span{Start: 25, End: 29},
Text: "this expression is always true",
},
},
Footers: []Footer{
{
Level: "note",
Text: "rule `test_1` is always `true`",
},
},
Text: `warning[invariant_expr]: invariant boolean expression
--> line:1:26
|
1 | rule test_1 { condition: true }
| ---- this expression is always true
|
= note: rule ` + "`test_1` is always `true`",
},
}, c.Warnings())

c.AddSource("rule test_2 { condition: foo }", WithOrigin("test.yar"))
assert.Equal(t, []CompileError{
{
Expand All @@ -144,6 +171,7 @@ func TestErrors(t *testing.T) {
Text: "this identifier has not been declared",
},
},
Footers: []Footer{},
Text: `error[E009]: unknown identifier ` + "`foo`" + `
--> test.yar:1:26
|
Expand Down Expand Up @@ -241,6 +269,7 @@ func TestWarnings(t *testing.T) {
Text: "these consecutive jumps will be treated as [0-2]",
},
},
Footers: []Footer{},
Text: `warning[consecutive_jumps]: consecutive jumps in hex pattern ` + "`$a`" + `
--> line:1:31
|
Expand All @@ -259,6 +288,7 @@ func TestWarnings(t *testing.T) {
Text: "this pattern may slow down the scan",
},
},
Footers: []Footer{},
Text: `warning[slow_pattern]: slow pattern
--> line:1:22
|
Expand Down
14 changes: 7 additions & 7 deletions lib/src/compiler/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use yara_x_macros::ErrorEnum;
use yara_x_macros::ErrorStruct;
use yara_x_parser::ast;

use crate::compiler::report::{Level, Report, ReportBuilder, CodeLoc, Label};
use crate::compiler::report::{Level, Report, ReportBuilder, CodeLoc, Label, Footer};

/// Error returned while serializing/deserializing compiled rules.
#[derive(Error, Debug)]
Expand Down Expand Up @@ -203,7 +203,7 @@ pub struct MismatchingTypes {
#[associated_enum(CompileError)]
#[error(code = "E004", title = "wrong arguments")]
#[label("wrong arguments in this call", error_loc)]
#[note(note)]
#[footer(note)]
pub struct WrongArguments {
report: Report,
error_loc: CodeLoc,
Expand Down Expand Up @@ -263,7 +263,7 @@ pub struct UnknownField {
#[associated_enum(CompileError)]
#[error(code = "E009", title = "unknown identifier `{identifier}`")]
#[label("this identifier has not been declared", identifier_loc)]
#[note(note)]
#[footer(note)]
pub struct UnknownIdentifier {
report: Report,
identifier: String,
Expand Down Expand Up @@ -346,7 +346,7 @@ pub struct ConflictingRuleIdentifier {
#[associated_enum(CompileError)]
#[error(code = "E014", title = "invalid regular expression")]
#[label("{error}", error_loc)]
#[note(note)]
#[footer(note)]
pub struct InvalidRegexp {
report: Report,
error: String,
Expand Down Expand Up @@ -376,7 +376,7 @@ pub struct MixedGreediness {
#[associated_enum(CompileError)]
#[error(code = "E016", title = "no matching patterns")]
#[label("there's no pattern in this set", error_loc)]
#[note(note)]
#[footer(note)]
pub struct EmptyPatternSet {
report: Report,
error_loc: CodeLoc,
Expand Down Expand Up @@ -417,7 +417,7 @@ pub struct SlowPattern {
)]
#[label("`{modifier1}` modifier used here", modifier1_loc)]
#[label("`{modifier2}` modifier used here", modifier2_loc)]
#[note(note)]
#[footer(note)]
pub struct InvalidModifierCombination {
report: Report,
modifier1: String,
Expand Down Expand Up @@ -481,7 +481,7 @@ pub struct DuplicatePattern {
#[associated_enum(CompileError)]
#[error(code = "E024", title = "invalid pattern `{pattern_ident}`")]
#[label("{error}", error_loc)]
#[note(note)]
#[footer(note)]
pub struct InvalidPattern {
report: Report,
pattern_ident: String,
Expand Down
1 change: 0 additions & 1 deletion lib/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,6 @@ impl<'a> Compiler<'a> {
};

rules.build_ac_automaton();

rules
}

Expand Down
62 changes: 43 additions & 19 deletions lib/src/compiler/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub(crate) struct Report {
code: &'static str,
title: String,
labels: Vec<(Level, CodeLoc, String)>,
note: Option<String>,
footers: Vec<(Level, String)>,
}

impl Report {
Expand All @@ -112,22 +112,21 @@ impl Report {
let code_origin =
code_cache.get(&source_id).unwrap().origin.clone();

let level = match level {
Level::Error => "error",
Level::Warning => "warning",
Level::Info => "info",
Level::Note => "note",
Level::Help => "help",
};

Label { level, code_origin, span: code_loc.span.clone(), text }
Label {
level: level_as_text(*level),
code_origin,
span: code_loc.span.clone(),
text,
}
})
}

/// Returns the report's note.
/// Returns the report's footers.
#[inline]
pub(crate) fn note(&self) -> Option<&str> {
self.note.as_deref()
pub(crate) fn footers(&self) -> impl Iterator<Item = Footer> {
self.footers
.iter()
.map(|(level, text)| Footer { level: level_as_text(*level), text })
}
}

Expand All @@ -140,7 +139,7 @@ impl Serialize for Report {
s.serialize_field("code", &self.code)?;
s.serialize_field("title", &self.title)?;
s.serialize_field("labels", &self.labels().collect::<Vec<_>>())?;
s.serialize_field("note", &self.note)?;
s.serialize_field("footers", &self.footers().collect::<Vec<_>>())?;
s.serialize_field("text", &self.to_string())?;
s.end()
}
Expand All @@ -152,7 +151,7 @@ impl PartialEq for Report {
&& self.code.eq(other.code)
&& self.title.eq(&other.title)
&& self.labels.eq(&other.labels)
&& self.note.eq(&other.note)
&& self.footers.eq(&other.footers)
}
}

Expand Down Expand Up @@ -210,8 +209,8 @@ impl Display for Report {

message = message.snippet(snippet);

if let Some(note) = &self.note {
message = message.footer(Level::Note.title(note.as_str()));
for (level, text) in &self.footers {
message = message.footer(level.title(text.as_str()));
}

let renderer = if self.with_colors {
Expand All @@ -236,6 +235,13 @@ pub struct Label<'a> {
text: &'a str,
}

/// Represents a footer in an error or warning report.
#[derive(Serialize)]
pub struct Footer<'a> {
level: &'a str,
text: &'a str,
}

/// Builds error and warning reports.
///
/// `ReportBuilder` helps to create error and warning reports. It stores a copy
Expand Down Expand Up @@ -373,11 +379,19 @@ impl ReportBuilder {
code: &'static str,
title: String,
labels: Vec<(Level, CodeLoc, String)>,
note: Option<String>,
footers: Vec<(Level, Option<String>)>,
) -> Report {
// Make sure there's at least one label.
assert!(!labels.is_empty());

// Remove footers where text is None.
let footers = footers
.into_iter()
.filter_map(|(level, text)| {
text.map(|text| (level, text))
})
.collect();

Report {
code_cache: self.code_cache.clone(),
with_colors: self.with_colors,
Expand All @@ -389,7 +403,17 @@ impl ReportBuilder {
code,
title,
labels,
note,
footers,
}
}
}

fn level_as_text(level: Level) -> &'static str {
match level {
Level::Error => "error",
Level::Warning => "warning",
Level::Info => "info",
Level::Note => "note",
Level::Help => "help",
}
}
2 changes: 1 addition & 1 deletion lib/src/compiler/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ fn errors_serialization() {
"text": "this identifier has not been declared"
}
],
"note": null,
"footers": [],
"text": r#"error[E009]: unknown identifier `foo`
--> test.yar:1:23
|
Expand Down
8 changes: 4 additions & 4 deletions lib/src/compiler/warnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use thiserror::Error;
use yara_x_macros::ErrorEnum;
use yara_x_macros::ErrorStruct;

use crate::compiler::report::{Level, Report, ReportBuilder, CodeLoc, Label};
use crate::compiler::report::{Level, Report, ReportBuilder, CodeLoc, Label, Footer};

/// A warning raised while compiling YARA rules.
#[allow(missing_docs)]
Expand Down Expand Up @@ -169,7 +169,7 @@ pub struct PotentiallyUnsatisfiableExpression {
"this expression is always {expr_value}",
expr_loc
)]
#[note(note)]
#[footer(note)]
pub struct InvariantBooleanExpression {
report: Report,
expr_value: bool,
Expand Down Expand Up @@ -200,7 +200,7 @@ pub struct InvariantBooleanExpression {
"this expression is `{expr_type}` but is being used as `bool`",
expr_loc
)]
#[note(note)]
#[footer(note)]
pub struct NonBooleanAsBoolean {
report: Report,
expr_type: String,
Expand Down Expand Up @@ -383,7 +383,7 @@ pub struct SlowPattern {
"module `{module_name}` used here",
module_name_loc
)]
#[note(note)]
#[footer(note)]
pub struct IgnoredModule {
report: Report,
module_name: String,
Expand Down
Loading

0 comments on commit 67c7979

Please sign in to comment.