Skip to content

Commit

Permalink
Make the diagnostic reporter return a new abstract type ErrorReported
Browse files Browse the repository at this point in the history
Works towards making it harder to accidentally abort compilation due to an error
*without emitting a diagnostic* (#130).
Instead of blindly creating `()`, `ErrorReported::error_will_be_reported_unchecked()`
needs to be used in non-trivial cases forcing the programmer to ponder if it's
correct what they are doing.
  • Loading branch information
fmease committed Feb 14, 2022
1 parent 991fe4d commit 6cb2bfe
Show file tree
Hide file tree
Showing 19 changed files with 297 additions and 303 deletions.
6 changes: 4 additions & 2 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ use std::{
};
use unicode_width::UnicodeWidthStr;

use self::reporter::ErrorReported;

pub mod reporter;
#[cfg(test)]
mod test;
Expand Down Expand Up @@ -231,8 +233,8 @@ impl Diagnostic {
}

/// Report the diagnostic.
pub fn report(self, reporter: &Reporter) {
reporter.report(self);
pub fn report(self, reporter: &Reporter) -> ErrorReported {
reporter.report(self)
}

/// Format the diagnostic for the use in a terminal.
Expand Down
14 changes: 13 additions & 1 deletion src/diagnostics/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ pub enum Reporter {
}

impl Reporter {
pub(super) fn report(&self, diagnostic: Diagnostic) {
pub(super) fn report(&self, diagnostic: Diagnostic) -> ErrorReported {
match self {
Self::Silent => {}
Self::Stderr(reporter) => reporter.report(diagnostic),
Self::BufferedStderr(reporter) => reporter.report_or_buffer(diagnostic),
}

ErrorReported(())
}
}

Expand Down Expand Up @@ -181,3 +183,13 @@ fn print_to_stderr(message: &impl std::fmt::Display) {
eprintln!("{message}");
eprintln!();
}

// @Beacon @Task docs
#[derive(Clone, Copy, Debug)]
pub struct ErrorReported(());

impl ErrorReported {
pub(crate) fn error_will_be_reported_unchecked() -> Self {
Self(())
}
}
3 changes: 2 additions & 1 deletion src/documenter/text_processor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{format::declaration_url_fragment, node::Node};
use crate::{
diagnostics::reporter::{SilentReporter, StderrReporter},
error::Result,
package::BuildSession,
resolver::{resolve_path, Component},
span::SourceMap,
Expand Down Expand Up @@ -276,7 +277,7 @@ impl<'a> Request<'a> {
url_prefix: &str,
component: &Component,
session: &BuildSession,
) -> Result<String, ()> {
) -> Result<String> {
Ok(match self {
Request::DeclarationUrl(path) => {
// @Temporary
Expand Down
23 changes: 13 additions & 10 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
use std::fmt;

use crate::{
diagnostics::{Diagnostic, Reporter},
diagnostics::{reporter::ErrorReported, Diagnostic, Reporter},
utility::SmallVec,
};

pub type Result<T = (), E = ()> = std::result::Result<T, E>;
pub type Result<T = (), E = ErrorReported> = std::result::Result<T, E>;

#[derive(Debug)]
#[must_use]
Expand Down Expand Up @@ -45,9 +45,6 @@ impl<T> Stain<T> for Outcome<T> {
}
}

// @Beacon @Beacon @Task implement this for all Result<T, E> where E: AlreadyReported (…)
// which is implemented for () and for e.g. RegistrationError (this would allow use to get rid of
// some .map_err(drop)
impl<T: PossiblyErroneous> Stain<T> for Result<T> {
fn stain(self, health: &mut Health) -> T {
match self {
Expand All @@ -68,7 +65,8 @@ impl<T> OkIfUntaintedExt<T> for Result<T> {
fn ok_if_untainted(value: T, health: Health) -> Self {
match health {
Health::Untainted => Ok(value),
Health::Tainted => Err(()),
// @Beacon @Beacon @Beacon @Task don't use this unchecked call, use the ErrorReported inside of Tainted (once available)
Health::Tainted => Err(ErrorReported::error_will_be_reported_unchecked()),
}
}
}
Expand All @@ -93,7 +91,7 @@ impl<T: PossiblyErroneous> From<Result<T>> for Outcome<T> {
fn from(option: Result<T>) -> Self {
match option {
Ok(value) => Outcome::untainted(value),
Err(()) => Outcome::tainted(T::error()),
Err(_token) => Outcome::tainted(T::error()),
}
}
}
Expand All @@ -102,7 +100,8 @@ impl<T> From<Outcome<T>> for Result<T> {
fn from(outcome: Outcome<T>) -> Self {
match outcome.health {
Health::Untainted => Ok(outcome.value),
Health::Tainted => Err(()),
// @Beacon @Beacon @Beacon @Task don't use this unchecked call, use the ErrorReported inside of Tainted (once available)
Health::Tainted => Err(ErrorReported::error_will_be_reported_unchecked()),
}
}
}
Expand All @@ -119,6 +118,8 @@ pub enum Health {
#[default]
Untainted,
/// Marks non-fatal failures.
// @Beacon @Beacon @Beacon @Task make this take an ErrorReported enabling us to
// remove the unchecked creation calls in the combinators below
Tainted,
}

Expand Down Expand Up @@ -149,7 +150,7 @@ impl From<Result> for Health {
fn from(result: Result) -> Self {
match result {
Ok(()) => Self::Untainted,
Err(()) => Self::Tainted,
Err(_) => Self::Tainted,
}
}
}
Expand All @@ -158,7 +159,8 @@ impl From<Health> for Result {
fn from(health: Health) -> Self {
match health {
Health::Untainted => Ok(()),
Health::Tainted => Err(()),
// @Beacon @Beacon @Beacon @Task don't use this unchecked call, use the ErrorReported inside of Tainted (once available)
Health::Tainted => Err(ErrorReported::error_will_be_reported_unchecked()),
}
}
}
Expand Down Expand Up @@ -192,6 +194,7 @@ impl<T> PossiblyErroneous for Vec<T> {
// @Note very weird impl...it does not really corresp. to our
// notion of possibly errorneous but still this impl is very useful
// as it allows us to call try_in on functions that merely check (Result<(), Error>)
// @Beacon @Beacon @Beacon @Question can we replace () here with ErrorReported?
impl PossiblyErroneous for () {
fn error() -> Self {}
}
Expand Down
17 changes: 9 additions & 8 deletions src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pub(crate) use crate::syntax::lowered_ast::Item;
use crate::{
error::{PossiblyErroneous, Result},
error::PossiblyErroneous,
package::session::IntrinsicNumericType,
resolver::{Component, FunctionScope},
span::{SourceFileIndex, Span},
Expand Down Expand Up @@ -324,16 +324,17 @@ pub enum Number {
}

impl Number {
pub(crate) fn parse(source: &str, type_: IntrinsicNumericType) -> Result<Self> {
#[must_use]
pub(crate) fn parse(source: &str, type_: IntrinsicNumericType) -> Option<Self> {
use IntrinsicNumericType::*;

match type_ {
Nat => source.parse().map(Self::Nat).map_err(drop),
Nat32 => source.parse().map(Self::Nat32).map_err(drop),
Nat64 => source.parse().map(Self::Nat64).map_err(drop),
Int => source.parse().map(Self::Int).map_err(drop),
Int32 => source.parse().map(Self::Int32).map_err(drop),
Int64 => source.parse().map(Self::Int64).map_err(drop),
Nat => source.parse().map(Self::Nat).ok(),
Nat32 => source.parse().map(Self::Nat32).ok(),
Nat64 => source.parse().map(Self::Nat64).ok(),
Int => source.parse().map(Self::Int).ok(),
Int32 => source.parse().map(Self::Int32).ok(),
Int64 => source.parse().map(Self::Int64).ok(),
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
#![allow(rustdoc::private_intra_doc_links)] // we always use `--document-private-items`
#![warn(clippy::pedantic)]
#![allow(
clippy::result_unit_err, // using a reporter to forward information
clippy::items_after_statements,
clippy::enum_glob_use,
clippy::must_use_candidate,
Expand Down
22 changes: 9 additions & 13 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#![forbid(rust_2018_idioms, unused_must_use)]
#![warn(clippy::pedantic)]
#![allow(
clippy::result_unit_err, // using a reporter to forward information
clippy::items_after_statements,
clippy::enum_glob_use,
clippy::must_use_candidate,
Expand Down Expand Up @@ -70,7 +69,7 @@ fn main() {
}
}

fn main_() -> Result {
fn main_() -> Result<(), ()> {
set_panic_hook();

let (command, options) = cli::arguments();
Expand Down Expand Up @@ -139,7 +138,7 @@ fn build_package(
// @Question code?
.message("the path to the source file or the package folder is invalid")
.note(IOError(error, &path).to_string())
.report(reporter);
.report(reporter)
})
})
.transpose()?;
Expand Down Expand Up @@ -179,16 +178,15 @@ fn build_package(
// @Beacon @Task dont unwrap, handle the error cases
let path = std::env::current_dir().unwrap();
let Some(path) = find_package(&path) else {
Diagnostic::error()
return Err(Diagnostic::error()
.message(
"neither the current folder nor any of its parents is a package",
)
.note(format!(
"none of the folders contain a package manifest file named `{}`",
PackageManifest::FILE_NAME
))
.report(reporter);
return Err(());
.report(reporter));
};

build_queue.process_package(path)?;
Expand Down Expand Up @@ -263,7 +261,7 @@ fn build_package(
component.name(),
))
.note(IOError(error, component.path()).to_string())
.report(reporter);
.report(reporter)
})?;

let time = Instant::now();
Expand Down Expand Up @@ -339,15 +337,14 @@ fn build_package(

// @Task move out of main.rs
if component.is_executable() && component.program_entry.is_none() {
Diagnostic::error()
return Err(Diagnostic::error()
.code(Code::E050)
.message(format!(
"the component `{}` is missing a program entry named `{}`",
component.name(),
PROGRAM_ENTRY_IDENTIFIER,
))
.report(reporter);
return Err(());
.report(reporter));
}

'ending: {
Expand All @@ -356,13 +353,12 @@ fn build_package(
if component.is_goal(&session) {
if !component.is_executable() {
// @Question code?
Diagnostic::error()
return Err(Diagnostic::error()
.message(format!(
"the package `{}` does not contain any executable to run",
component.package(&session).name,
))
.report(reporter);
return Err(());
.report(reporter));
}

let result = typer::interpreter::evaluate_main_function(
Expand Down
Loading

0 comments on commit 6cb2bfe

Please sign in to comment.