-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Automatic annotation of type signatures #7130
base: main
Are you sure you want to change the base?
Changes from all commits
9f67c79
40a0086
6e39425
6313461
d4e558d
78ba0ca
1ec619f
c519ab7
406cc6c
bbe2d7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,26 @@ | ||
use std::ffi::OsStr; | ||
use std::io::Write; | ||
use std::ops::Range; | ||
use std::path::{Path, PathBuf}; | ||
|
||
use bumpalo::Bump; | ||
use roc_can::abilities::{IAbilitiesStore, Resolved}; | ||
use roc_can::expr::{DeclarationTag, Declarations, Expr}; | ||
use roc_error_macros::{internal_error, user_error}; | ||
use roc_fmt::def::fmt_defs; | ||
use roc_fmt::header::fmt_header; | ||
use roc_fmt::Buf; | ||
use roc_fmt::MigrationFlags; | ||
use roc_load::{ExecutionMode, FunctionKind, LoadConfig, LoadedModule, LoadingProblem, Threading}; | ||
use roc_module::symbol::{Interns, ModuleId}; | ||
use roc_packaging::cache::{self, RocCacheDir}; | ||
use roc_parse::ast::{FullAst, SpacesBefore}; | ||
use roc_parse::header::parse_module_defs; | ||
use roc_parse::normalize::Normalize; | ||
use roc_parse::{header, parser::SyntaxError, state::State}; | ||
use roc_reporting::report::{RenderTarget, DEFAULT_PALETTE}; | ||
use roc_target::Target; | ||
use roc_types::subs::{Content, Subs, Variable}; | ||
|
||
#[derive(Copy, Clone, Debug)] | ||
pub enum FormatMode { | ||
|
@@ -263,10 +272,161 @@ fn fmt_all<'a>(buf: &mut Buf<'a>, ast: &'a FullAst) { | |
buf.fmt_end_of_file(); | ||
} | ||
|
||
pub fn annotate_file(arena: &Bump, file: PathBuf) -> Result<(), LoadingProblem> { | ||
let load_config = LoadConfig { | ||
target: Target::default(), | ||
function_kind: FunctionKind::from_env(), | ||
render: RenderTarget::ColorTerminal, | ||
palette: DEFAULT_PALETTE, | ||
threading: Threading::AllAvailable, | ||
exec_mode: ExecutionMode::Check, | ||
}; | ||
|
||
let mut loaded = roc_load::load_and_typecheck( | ||
arena, | ||
file.clone(), | ||
None, | ||
RocCacheDir::Persistent(cache::roc_cache_dir().as_path()), | ||
load_config, | ||
)?; | ||
|
||
let buf = annotate_module(&mut loaded); | ||
|
||
std::fs::write(&file, buf.as_str()) | ||
.unwrap_or_else(|e| internal_error!("failed to write annotated file to {file:?}: {e}")); | ||
|
||
Ok(()) | ||
} | ||
|
||
fn annotate_module(loaded: &mut LoadedModule) -> String { | ||
let (decls, subs, abilities) = | ||
if let Some(decls) = loaded.declarations_by_id.get(&loaded.module_id) { | ||
let subs = loaded.solved.inner_mut(); | ||
let abilities = &loaded.abilities_store; | ||
|
||
(decls, subs, abilities) | ||
} else if let Some(checked) = loaded.typechecked.get_mut(&loaded.module_id) { | ||
let decls = &checked.decls; | ||
let subs = checked.solved_subs.inner_mut(); | ||
let abilities = &checked.abilities_store; | ||
|
||
(decls, subs, abilities) | ||
} else { | ||
internal_error!("Could not find file's module"); | ||
}; | ||
|
||
let src = &loaded | ||
.sources | ||
.get(&loaded.module_id) | ||
.unwrap_or_else(|| internal_error!("Could not find the file's source")) | ||
.1; | ||
|
||
let mut edits = annotation_edits( | ||
decls, | ||
subs, | ||
abilities, | ||
src, | ||
loaded.module_id, | ||
&loaded.interns, | ||
); | ||
edits.sort_by_key(|(offset, _)| *offset); | ||
|
||
let mut buffer = String::new(); | ||
let mut file_progress = 0; | ||
|
||
for (position, edit) in edits { | ||
if file_progress > position { | ||
internal_error!("Module definitions are out of order"); | ||
}; | ||
|
||
buffer.push_str(&src[file_progress..position]); | ||
buffer.push_str(&edit); | ||
|
||
file_progress = position; | ||
} | ||
buffer.push_str(&src[file_progress..]); | ||
|
||
buffer | ||
} | ||
|
||
pub fn annotation_edits( | ||
decls: &Declarations, | ||
subs: &Subs, | ||
abilities: &IAbilitiesStore<Resolved>, | ||
src: &str, | ||
module_id: ModuleId, | ||
interns: &Interns, | ||
) -> Vec<(usize, String)> { | ||
decls | ||
.iter_bottom_up() | ||
.flat_map(|(index, tag)| { | ||
let var = decls.variables[index]; | ||
let symbol = decls.symbols[index]; | ||
let expr = &decls.expressions[index].value; | ||
|
||
use roc_problem::can::RuntimeError::ExposedButNotDefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this use be moved to the top? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep! |
||
if decls.annotations[index].is_some() | ||
| matches!( | ||
*expr, | ||
Expr::RuntimeError(ExposedButNotDefined(..)) | Expr::ImportParams(..) | ||
) | ||
| abilities.is_specialization_name(symbol.value) | ||
| matches!(subs.get_content_without_compacting(var), Content::Error) | ||
| matches!(tag, DeclarationTag::MutualRecursion { .. }) | ||
{ | ||
return None; | ||
} | ||
|
||
let byte_range = match tag { | ||
DeclarationTag::Destructure(i) => { | ||
decls.destructs[i.index()].loc_pattern.byte_range() | ||
} | ||
_ => symbol.byte_range(), | ||
}; | ||
|
||
let edit = annotation_edit(src, subs, interns, module_id, var, byte_range); | ||
|
||
Some(edit) | ||
}) | ||
.collect() | ||
} | ||
|
||
pub fn annotation_edit( | ||
src: &str, | ||
subs: &Subs, | ||
interns: &Interns, | ||
module_id: ModuleId, | ||
var: Variable, | ||
symbol_range: Range<usize>, | ||
) -> (usize, String) { | ||
let signature = roc_types::pretty_print::name_and_print_var( | ||
var, | ||
&mut subs.clone(), | ||
module_id, | ||
interns, | ||
roc_types::pretty_print::DebugPrint::NOTHING, | ||
) | ||
// Generated names for errors start with `#` | ||
.replace('#', ""); | ||
Comment on lines
+409
to
+410
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that if there's an error in the type, don't we want to cancel the operation, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe? These only seem show up when a type doesn't implement an ability it's supposed to. I'm not sure how I'd go about detecting all type errors. I could try looking for
But that still allows things like This stuff is a bit over my head so maybe I'm missing something but I don't see anything in the codebase for this already The other way to disallow errors is not from the type but if the code itself has any warnings or errors but that seems a bit heavy handed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think (this is not really my area...) that may be fine, and the constraint is that Maybe @bhansconnect has more specific advice about how we should be applying types here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would short circuit and break out if there are any errors in types, I think it's too easy to generate wrong annotations otherwise. If the easiest method to implement it is to check if the code has any errors, I think that's fine at this moment in time |
||
|
||
let line_start = src[..symbol_range.start] | ||
.rfind('\n') | ||
.map_or(symbol_range.start, |pos| pos + 1); | ||
let indent = src[line_start..] | ||
.split_once(|c: char| !c.is_ascii_whitespace()) | ||
.map_or("", |pair| pair.0); | ||
|
||
let symbol_str = &src[symbol_range.clone()]; | ||
let edit = format!("{indent}{symbol_str} : {signature}\n"); | ||
|
||
(line_start, edit) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use std::fs::File; | ||
use indoc::indoc; | ||
use std::fs::{read_to_string, File}; | ||
use std::io::Write; | ||
use tempfile::{tempdir, TempDir}; | ||
|
||
|
@@ -379,4 +539,75 @@ main = | |
|
||
cleanup_temp_dir(dir); | ||
} | ||
|
||
const HEADER: &str = indoc! {r#" | ||
interface Test | ||
exposes [] | ||
imports [] | ||
|
||
"#}; | ||
|
||
fn annotate_string(before: String) -> String { | ||
let dir = tempdir().unwrap(); | ||
let file_path = setup_test_file(dir.path(), "before.roc", &before); | ||
|
||
let arena = Bump::new(); | ||
let result = annotate_file(&arena, file_path.clone()); | ||
result.unwrap(); | ||
|
||
let annotated = read_to_string(file_path).unwrap(); | ||
|
||
cleanup_temp_dir(dir); | ||
annotated | ||
} | ||
|
||
#[test] | ||
fn test_annotate_simple() { | ||
let before = HEADER.to_string() | ||
+ indoc! {r#" | ||
main = | ||
"Hello, World!""#}; | ||
|
||
let after = HEADER.to_string() | ||
+ indoc! {r#" | ||
main : Str | ||
main = | ||
"Hello, World!" | ||
"#}; | ||
|
||
let annotated = annotate_string(before); | ||
|
||
assert_eq!(annotated, after); | ||
} | ||
|
||
#[test] | ||
fn test_annotate_empty() { | ||
let before = HEADER.to_string(); | ||
let after = HEADER.to_string() + "\n"; | ||
let annotated = annotate_string(before); | ||
|
||
assert_eq!(annotated, after); | ||
} | ||
|
||
#[test] | ||
fn test_annotate_destructure() { | ||
let before = HEADER.to_string() | ||
+ indoc! {r#" | ||
{a, b} = {a: "zero", b: (1, 2)} | ||
|
||
main = a"#}; | ||
|
||
let after = HEADER.to_string() | ||
+ indoc! {r#" | ||
{a, b} : { a : Str, b : ( Num *, Num * )* } | ||
{a, b} = {a: "zero", b: (1, 2)} | ||
|
||
main : Str | ||
main = a | ||
"#}; | ||
|
||
let annotated = annotate_string(before); | ||
|
||
assert_eq!(annotated, after); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've exposed this and annotations_edit() publicly to be used in the language server but It feels weird to expose them from the cli crate. If it should be in another crate what one?