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

Automatic annotation of type signatures #7130

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ roc_module.workspace = true
roc_mono.workspace = true
roc_packaging.workspace = true
roc_parse.workspace = true
roc_problem.workspace = true
roc_region.workspace = true
roc_reporting.workspace = true
roc_target.workspace = true
roc_tracing.workspace = true
roc_types.workspace = true
roc_repl_cli = { workspace = true, optional = true }
roc_wasm_interp = { workspace = true, optional = true }

Expand Down
233 changes: 232 additions & 1 deletion crates/cli/src/format.rs
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 {
Expand Down Expand Up @@ -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(
Copy link
Contributor Author

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?

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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use be moved to the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

@snobee snobee Jan 21, 2025

Choose a reason for hiding this comment

The 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

  • Error
  • FlexVar(None) or FlexAbleVar(None, ...) that show up anywhere except as arguments
  • FlexVar or FlexAbleVar that have names that start with '#'

But that still allows things like List.first(8) which has the type Result a [ListWasEmpty]. I'm not sure if that would be counted as an error or not? a doesn't come from anywhere so maybe that means it is?

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a doesn't come from anywhere so maybe that means it is?

I think (this is not really my area...) that may be fine, and the constraint is that a needs to be a fresh name (not bound in the outer scope).

Maybe @bhansconnect has more specific advice about how we should be applying types here?

Copy link
Member

Choose a reason for hiding this comment

The 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};

Expand Down Expand Up @@ -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);
}
}
15 changes: 14 additions & 1 deletion crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ use strum::IntoEnumIterator;
use tempfile::TempDir;

mod format;
pub use format::{format_files, format_src, FormatMode};
pub use format::{
annotate_file, annotation_edit, annotation_edits, format_files, format_src, FormatMode,
};

pub const CMD_BUILD: &str = "build";
pub const CMD_RUN: &str = "run";
Expand All @@ -52,6 +54,7 @@ pub const CMD_DOCS: &str = "docs";
pub const CMD_CHECK: &str = "check";
pub const CMD_VERSION: &str = "version";
pub const CMD_FORMAT: &str = "format";
pub const CMD_FORMAT_ANNOTATE: &str = "annotate";
pub const CMD_TEST: &str = "test";
pub const CMD_GLUE: &str = "glue";
pub const CMD_PREPROCESS_HOST: &str = "preprocess-host";
Expand Down Expand Up @@ -380,6 +383,16 @@ pub fn build_app() -> Command {
.required(false),
)
.after_help("If DIRECTORY_OR_FILES is omitted, the .roc files in the current working\ndirectory are formatted.")
.subcommand(Command::new(CMD_FORMAT_ANNOTATE)
.about("Annotate all top level definitions from a .roc file")
.arg(
Arg::new(ROC_FILE)
.help("The .roc file ot annotate")
.value_parser(value_parser!(PathBuf))
.required(false)
.default_value(DEFAULT_ROC_FILENAME),
)
)
)
.subcommand(Command::new(CMD_VERSION)
.about(concatcp!("Print the Roc compiler’s version, which is currently ", VERSION)))
Expand Down
38 changes: 31 additions & 7 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ use bumpalo::Bump;
use roc_build::link::LinkType;
use roc_build::program::{check_file, CodeGenBackend};
use roc_cli::{
build_app, default_linking_strategy, format_files, format_src, test, BuildConfig, FormatMode,
CMD_BUILD, CMD_CHECK, CMD_DEV, CMD_DOCS, CMD_FORMAT, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL,
CMD_RUN, CMD_TEST, CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_DOCS_ROOT,
FLAG_LIB, FLAG_MAIN, FLAG_MIGRATE, FLAG_NO_COLOR, FLAG_NO_HEADER, FLAG_NO_LINK, FLAG_OUTPUT,
FLAG_PP_DYLIB, FLAG_PP_HOST, FLAG_PP_PLATFORM, FLAG_STDIN, FLAG_STDOUT, FLAG_TARGET, FLAG_TIME,
FLAG_VERBOSE, GLUE_DIR, GLUE_SPEC, ROC_FILE, VERSION,
annotate_file, build_app, default_linking_strategy, format_files, format_src, test,
BuildConfig, FormatMode, CMD_BUILD, CMD_CHECK, CMD_DEV, CMD_DOCS, CMD_FORMAT,
CMD_FORMAT_ANNOTATE, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL, CMD_RUN, CMD_TEST, CMD_VERSION,
DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_DOCS_ROOT, FLAG_LIB, FLAG_MAIN, FLAG_MIGRATE,
FLAG_NO_COLOR, FLAG_NO_HEADER, FLAG_NO_LINK, FLAG_OUTPUT, FLAG_PP_DYLIB, FLAG_PP_HOST,
FLAG_PP_PLATFORM, FLAG_STDIN, FLAG_STDOUT, FLAG_TARGET, FLAG_TIME, FLAG_VERBOSE, GLUE_DIR,
GLUE_SPEC, ROC_FILE, VERSION,
};
use roc_docs::generate_docs_html;
use roc_error_macros::user_error;
use roc_error_macros::{internal_error, user_error};
use roc_fmt::MigrationFlags;
use roc_gen_dev::AssemblyBackendMode;
use roc_gen_llvm::llvm::build::LlvmBackendMode;
Expand Down Expand Up @@ -346,6 +347,29 @@ fn main() -> io::Result<()> {

Ok(0)
}
Some((CMD_FORMAT, fmatches)) if Some(CMD_FORMAT_ANNOTATE) == fmatches.subcommand_name() => {
let matches = fmatches
.subcommand_matches(CMD_FORMAT_ANNOTATE)
.unwrap_or_else(|| internal_error!("No annotate subcommand present"));

let arena = Bump::new();
let roc_file_path = matches
.get_one::<PathBuf>(ROC_FILE)
.unwrap_or_else(|| internal_error!("No default for ROC_FILE"));

let annotate_exit_code = match annotate_file(&arena, roc_file_path.to_owned()) {
Ok(()) => 0,
Err(LoadingProblem::FormattedReport(report, ..)) => {
eprintln!("{report}");
1
}
Err(other) => {
internal_error!("build_file failed with error:\n{other:?}");
}
};

Ok(annotate_exit_code)
}
Some((CMD_FORMAT, matches)) => {
let from_stdin = matches.get_flag(FLAG_STDIN);
let to_stdout = matches.get_flag(FLAG_STDOUT);
Expand Down
Loading