Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes towards a language server friendly compiler #673

Merged
merged 27 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e55dbbf
Experiment with ranged diagnostics
edusporto Aug 1, 2024
8426a3c
Change `Parser` to error with span information
edusporto Aug 6, 2024
7cd4ddf
Add file and span information to parsing error diagnostics
edusporto Aug 6, 2024
8ba7f95
Change `range` to `span`
edusporto Aug 6, 2024
cf376af
Fix tests
edusporto Aug 6, 2024
46b24fc
Fix byte span to line position translation
edusporto Aug 7, 2024
9c3eb35
Report range on unused definition warning
edusporto Aug 7, 2024
811e511
Optional origin in file span
edusporto Aug 7, 2024
f9198f4
Add function to display single diagnostic with its origin
edusporto Aug 7, 2024
1fe3dd1
Bump HVM version to 2.0.22
edusporto Aug 9, 2024
f1d419d
Remove unused diagnostics code
edusporto Aug 13, 2024
4e155cc
Implement function diagnostic origin
edusporto Aug 14, 2024
c9c7219
Add word to cspell
edusporto Aug 14, 2024
ca4da8f
Remove `Rule` diagnostic origin in favor of `Function`
edusporto Aug 14, 2024
e42add1
Update Cargo.toml to avoid semantic versioning for HVM
edusporto Aug 14, 2024
c86daac
Change "repeated field" message to avoid printing expected term
edusporto Aug 14, 2024
66ac5dc
Change `Stmt::into_fun` back into returning strings as errors
edusporto Aug 14, 2024
e263c98
Merge branch 'main' into lsp-experiments
edusporto Aug 14, 2024
d67080b
Refactor range reporting system to be a simple `Source`
edusporto Aug 15, 2024
2208ca9
Merge branch 'main' into lsp-experiments
edusporto Aug 16, 2024
6365c0c
Fix conflict between error expecting `String` getting `Diagnostics`
edusporto Aug 16, 2024
9b20e51
Keep original source in generated definitions
edusporto Aug 16, 2024
bde36fc
Improve `definition_merge` comments
edusporto Aug 16, 2024
8e20371
Change diagnostics printing so it doesn't include "During execution:"
edusporto Aug 16, 2024
9862533
Fix clippy warning
edusporto Aug 16, 2024
e1ba43d
Add changelog
edusporto Aug 19, 2024
cd268fa
Merge branch 'main' into lsp-experiments
edusporto Aug 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ default = ["cli"]
cli = ["dep:clap"]

[dependencies]
TSPL = "0.0.12"
TSPL = "0.0.13"
clap = { version = "4.4.1", features = ["derive"], optional = true }
highlight_error = "0.1.1"
hvm = "=2.0.19"
hvm = "=2.0.22"
indexmap = "2.2.3"
interner = "0.2.1"
itertools = "0.11.0"
Expand Down
220 changes: 202 additions & 18 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use crate::fun::{display::DisplayFn, Name};
use TSPL::ParseError;

use crate::fun::{display::DisplayFn, Name, Source};
use std::{
collections::BTreeMap,
fmt::{Display, Formatter},
ops::Range,
};

pub const ERR_INDENT_SIZE: usize = 2;
Expand All @@ -28,16 +31,19 @@ pub struct DiagnosticsConfig {

#[derive(Debug, Clone)]
pub struct Diagnostic {
message: String,
severity: Severity,
pub message: String,
pub severity: Severity,
pub span: Option<FileSpan>,
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum DiagnosticOrigin {
/// An error when parsing source code.
Parsing,
/// An error from the relationship between multiple top-level definitions.
Book,
/// An error in a pattern-matching function definition rule.
Rule(Name),
/// An error in a function definition.
Function(Name),
/// An error in a compiled inet.
edusporto marked this conversation as resolved.
Show resolved Hide resolved
Inet(String),
/// An error during readback of hvm-core run results.
Expand Down Expand Up @@ -68,39 +74,63 @@ impl Diagnostics {
Self { err_counter: 0, diagnostics: Default::default(), config }
}

pub fn add_parsing_error(&mut self, err: impl std::fmt::Display, span: FileSpan) {
self.err_counter += 1;
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Parsing, Some(span));
}

pub fn add_book_error(&mut self, err: impl std::fmt::Display) {
self.err_counter += 1;
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Book);
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Book, None);
}

pub fn add_rule_error(&mut self, err: impl std::fmt::Display, def_name: Name) {
pub fn add_function_error(&mut self, err: impl std::fmt::Display, name: Name, source: Option<&Source>) {
self.err_counter += 1;
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Rule(def_name.def_name_from_generated()));
let span = source.and_then(|src| src.span()).map(|s| FileSpan::new(s, None));
self.add_diagnostic(
err,
Severity::Error,
DiagnosticOrigin::Function(name.def_name_from_generated()),
span,
);
}

pub fn add_inet_error(&mut self, err: impl std::fmt::Display, def_name: String) {
self.err_counter += 1;
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Inet(def_name));
self.add_diagnostic(err, Severity::Error, DiagnosticOrigin::Inet(def_name), None);
}

pub fn add_rule_warning(&mut self, warn: impl std::fmt::Display, warn_type: WarningType, def_name: Name) {
pub fn add_function_warning(
&mut self,
warn: impl std::fmt::Display,
warn_type: WarningType,
def_name: Name,
source: Option<&Source>,
) {
let severity = self.config.warning_severity(warn_type);
if severity == Severity::Error {
self.err_counter += 1;
}
self.add_diagnostic(warn, severity, DiagnosticOrigin::Rule(def_name.def_name_from_generated()));
let span = source.and_then(|src| src.span()).map(|s| FileSpan::new(s, None));
self.add_diagnostic(warn, severity, DiagnosticOrigin::Function(def_name.def_name_from_generated()), span);
}

pub fn add_book_warning(&mut self, warn: impl std::fmt::Display, warn_type: WarningType) {
let severity = self.config.warning_severity(warn_type);
if severity == Severity::Error {
self.err_counter += 1;
}
self.add_diagnostic(warn, severity, DiagnosticOrigin::Book);
self.add_diagnostic(warn, severity, DiagnosticOrigin::Book, None);
}

pub fn add_diagnostic(&mut self, msg: impl ToString, severity: Severity, orig: DiagnosticOrigin) {
let diag = Diagnostic { message: msg.to_string(), severity };
pub fn add_diagnostic(
&mut self,
msg: impl std::fmt::Display,
severity: Severity,
orig: DiagnosticOrigin,
range: Option<FileSpan>,
) {
let diag = Diagnostic { message: msg.to_string(), severity, span: range };
self.diagnostics.entry(orig).or_default().push(diag)
}

Expand All @@ -112,7 +142,7 @@ impl Diagnostics {
match result {
Ok(t) => Some(t),
Err(e) => {
self.add_rule_error(e, def_name);
self.add_function_error(e, def_name, None);
None
}
}
Expand Down Expand Up @@ -171,12 +201,17 @@ impl Diagnostics {
let mut errs = filter(errs, severity).peekable();
if errs.peek().is_some() {
match orig {
DiagnosticOrigin::Parsing => {
for err in errs {
writeln!(f, "{err}")?;
}
}
DiagnosticOrigin::Book => {
for err in errs {
writeln!(f, "{err}")?;
}
}
DiagnosticOrigin::Rule(nam) => {
DiagnosticOrigin::Function(nam) => {
writeln!(f, "\x1b[1mIn definition '\x1b[4m{}\x1b[0m\x1b[1m':\x1b[0m", nam)?;
for err in errs {
writeln!(f, "{:ERR_INDENT_SIZE$}{err}", "")?;
Expand Down Expand Up @@ -204,6 +239,15 @@ impl Diagnostics {
Ok(())
})
}

pub fn display_only_messages(&self) -> impl std::fmt::Display + '_ {
DisplayFn(move |f| {
for err in self.diagnostics.values().flatten() {
writeln!(f, "{err}")?;
}
Ok(())
})
}
}

impl Display for Diagnostics {
Expand All @@ -223,7 +267,24 @@ impl From<String> for Diagnostics {
Self {
diagnostics: BTreeMap::from_iter([(
DiagnosticOrigin::Book,
vec![Diagnostic { message: value, severity: Severity::Error }],
vec![Diagnostic { message: value, severity: Severity::Error, span: None }],
)]),
..Default::default()
}
}
}

impl From<ParseError> for Diagnostics {
/// Transforms a parse error into `Diagnostics`.
///
/// NOTE: Since `ParseError` does not include the source code, we can't get the `TextLocation` of the error,
/// so it is not included in the diagnostic.
/// range is set as None.
fn from(value: ParseError) -> Self {
Self {
diagnostics: BTreeMap::from_iter([(
DiagnosticOrigin::Parsing,
vec![Diagnostic { message: value.into(), severity: Severity::Error, span: None }],
)]),
..Default::default()
}
Expand Down Expand Up @@ -270,6 +331,129 @@ impl Default for DiagnosticsConfig {

impl Display for Diagnostic {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.message)
// TODO: this could be problematic when printing multiple errors from the same file
// currently, there shouldn't be any `Diagnostics` with multiple problems `FileSpan`s
match &self.span {
Some(FileSpan { file: Some(file), .. }) => write!(f, "In {} :\n{}", file, self.message),
_ => write!(f, "{}", self.message),
}
}
}

impl Diagnostic {
pub fn display_with_origin<'a>(&'a self, origin: &'a DiagnosticOrigin) -> impl std::fmt::Display + '_ {
DisplayFn(move |f| {
match origin {
DiagnosticOrigin::Parsing => writeln!(f, "{self}")?,
DiagnosticOrigin::Book => writeln!(f, "{self}")?,
DiagnosticOrigin::Function(nam) => {
writeln!(f, "\x1b[1mIn definition '\x1b[4m{}\x1b[0m\x1b[1m':\x1b[0m", nam)?;
writeln!(f, "{:ERR_INDENT_SIZE$}{self}", "")?;
}
DiagnosticOrigin::Inet(nam) => {
writeln!(f, "\x1b[1mIn compiled inet '\x1b[4m{}\x1b[0m\x1b[1m':\x1b[0m", nam)?;
writeln!(f, "{:ERR_INDENT_SIZE$}{self}", "")?;
}
DiagnosticOrigin::Readback => {
writeln!(f, "\x1b[1mDuring readback:\x1b[0m")?;
writeln!(f, "{:ERR_INDENT_SIZE$}{self}", "")?;
}
};
Ok(())
})
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, PartialOrd, Ord, Eq)]
pub struct TextLocation {
pub line: usize,
pub char: usize,
}

impl TextLocation {
pub fn new(line: usize, char: usize) -> Self {
TextLocation { line, char }
}

/// Transforms a `usize` byte index on `code` into a `TextLocation`.
pub fn from_byte_loc(code: &str, loc: usize) -> Self {
let code = code.as_bytes();
let mut line = 0;
let mut char = 0;
let mut cur_idx = 0;
while cur_idx < loc && cur_idx < code.len() {
if code[cur_idx] == b'\n' {
line += 1;
char = 0;
} else {
char += 1;
}
cur_idx += 1;
}

TextLocation { line, char }
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, PartialOrd, Ord, Eq)]
pub struct TextSpan {
pub start: TextLocation,
pub end: TextLocation,
}

impl TextSpan {
pub fn new(start: TextLocation, end: TextLocation) -> Self {
TextSpan { start, end }
}

/// Transforms a `usize` byte range on `code` into a `TextLocation`.
pub fn from_byte_span(code: &str, span: Range<usize>) -> Self {
// Will loop for way too long otherwise
assert!(span.start <= span.end);

let code = code.as_bytes();
let mut start_line = 0;
let mut start_char = 0;
let mut end_line;
let mut end_char;

let mut cur_idx = 0;
while cur_idx < span.start && cur_idx < code.len() {
if code[cur_idx] == b'\n' {
start_line += 1;
start_char = 0;
} else {
start_char += 1;
}
cur_idx += 1;
}

end_line = start_line;
end_char = start_char;
while cur_idx < span.end && cur_idx < code.len() {
if code[cur_idx] == b'\n' {
end_line += 1;
end_char = 0;
} else {
end_char += 1;
}
cur_idx += 1;
}

TextSpan::new(TextLocation::new(start_line, start_char), TextLocation::new(end_line, end_char))
}
}

#[derive(Debug, Clone, Hash, PartialEq, PartialOrd, Ord, Eq)]
pub struct FileSpan {
pub span: TextSpan,
// Storing files as Strings, could be done as file IDs in the future
// This is currently optional, we might want to change it later
pub file: Option<String>,
}

impl FileSpan {
pub fn new(span: TextSpan, origin: Option<&str>) -> Self {
FileSpan { span, file: origin.map(|s| s.into()) }
}
}
6 changes: 5 additions & 1 deletion src/fun/check/unbound_refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ impl Ctx<'_> {
rule.body.check_unbound_refs(self.book, &mut unbounds);
}
for unbound in unbounds {
self.info.add_rule_error(format!("Reference to undefined function '{unbound}'"), def.name.clone());
self.info.add_function_error(
format!("Reference to undefined function '{unbound}'"),
def.name.clone(),
Some(&def.source),
);
}
}
self.info.fatal(())
Expand Down
2 changes: 1 addition & 1 deletion src/fun/check/unbound_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl Ctx<'_> {
}

for err in errs {
self.info.add_rule_error(err, def_name.clone());
self.info.add_function_error(err, def_name.clone(), Some(&def.source));
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/fun/load_book.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::{
Book, Name,
};
use crate::{
diagnostics::{Diagnostics, DiagnosticsConfig},
diagnostics::{Diagnostics, DiagnosticsConfig, FileSpan, TextSpan},
imports::PackageLoader,
};
use std::path::Path;
Expand Down Expand Up @@ -39,11 +39,16 @@ pub fn load_to_book(
book.load_imports(package_loader, diag)
}

pub fn do_parse_book(code: &str, origin: &Path, mut book: ParseBook) -> Result<ParseBook, String> {
pub fn do_parse_book(code: &str, origin: &Path, mut book: ParseBook) -> Result<ParseBook, Diagnostics> {
book.source = Name::new(origin.to_string_lossy());
TermParser::new(code).parse_book(book, false).map_err(|e| format!("In {} :\n{}", origin.display(), e))
TermParser::new(code).parse_book(book, false).map_err(|err| {
let mut diagnostics = Diagnostics::default();
let span = TextSpan::from_byte_span(code, err.span.0..err.span.1);
diagnostics.add_parsing_error(err, FileSpan { span, file: Some(origin.to_string_lossy().into()) });
diagnostics
})
}

pub fn do_parse_book_default(code: &str, origin: &Path) -> Result<Book, String> {
pub fn do_parse_book_default(code: &str, origin: &Path) -> Result<Book, Diagnostics> {
do_parse_book(code, origin, ParseBook::builtins())?.to_fun()
}
Loading
Loading