Merge pull request #6676 from roc-lang/ok-err

Allow the names `Ok` and `Err` as type aliases
This commit is contained in:
Richard Feldman 2024-04-27 06:49:46 -04:00 committed by GitHub
commit f7716188a1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 218 additions and 96 deletions

2
Cargo.lock generated
View file

@ -2502,12 +2502,14 @@ dependencies = [
"roc_module",
"roc_packaging",
"roc_parse",
"roc_problem",
"roc_region",
"roc_reporting",
"roc_solve",
"roc_target",
"roc_types",
"snafu",
"ven_pretty",
]
[[package]]

View file

@ -54,6 +54,7 @@ mod cli_run {
const OPTIMIZE_FLAG: &str = concatcp!("--", roc_cli::FLAG_OPTIMIZE);
const LINKER_FLAG: &str = concatcp!("--", roc_cli::FLAG_LINKER);
const CHECK_FLAG: &str = concatcp!("--", roc_cli::FLAG_CHECK);
#[allow(dead_code)]
const PREBUILT_PLATFORM: &str = concatcp!("--", roc_cli::FLAG_PREBUILT);
#[allow(dead_code)]
const TARGET_FLAG: &str = concatcp!("--", roc_cli::FLAG_TARGET);
@ -1025,20 +1026,21 @@ mod cli_run {
// TODO not sure if this cfg should still be here: #[cfg(not(debug_assertions))]
// this is for testing the benchmarks, to perform proper benchmarks see crates/cli/benches/README.md
mod test_benchmarks {
#[allow(unused_imports)]
use super::{TestCliCommands, UseValgrind};
use cli_utils::helpers::cli_testing_dir;
#[allow(unused_imports)]
use super::{check_output_with_stdin, OPTIMIZE_FLAG, PREBUILT_PLATFORM};
#[allow(unused_imports)]
use std::{path::Path, sync::Once};
static BENCHMARKS_BUILD_PLATFORM: Once = Once::new();
fn test_benchmark(
roc_filename: &str,
stdin: &[&str],
expected_ending: &str,
use_valgrind: UseValgrind,
_use_valgrind: UseValgrind,
) {
let file_name = cli_testing_dir("benchmarks").join(roc_filename);
@ -1062,15 +1064,18 @@ mod cli_run {
}
#[cfg(all(not(feature = "wasm32-cli-run"), not(feature = "i386-cli-run")))]
check_output_regular(&file_name, stdin, expected_ending, use_valgrind);
check_output_regular(&file_name, stdin, expected_ending, _use_valgrind);
#[cfg(feature = "wasm32-cli-run")]
check_output_wasm(&file_name, stdin, expected_ending);
#[cfg(feature = "i386-cli-run")]
check_output_i386(&file_name, stdin, expected_ending, use_valgrind);
check_output_i386(&file_name, stdin, expected_ending, _use_valgrind);
}
#[cfg(all(not(feature = "wasm32-cli-run"), not(feature = "i386-cli-run")))]
static BENCHMARKS_BUILD_PLATFORM: Once = Once::new();
#[cfg(all(not(feature = "wasm32-cli-run"), not(feature = "i386-cli-run")))]
fn check_output_regular(
file_name: &Path,

View file

@ -53,13 +53,11 @@ impl Scope {
initial_ident_ids: IdentIds,
starting_abilities_store: PendingAbilitiesStore,
) -> Scope {
let default_imports =
// Add all `Apply` types.
(Symbol::apply_types_in_scope().into_iter())
// Add all tag names we might want to suggest as hints in error messages.
.chain(Symbol::symbols_in_scope_for_hints());
let default_imports = default_imports.map(|(a, (b, c))| (a, b, c)).collect();
// Add all `Apply` types.
let default_imports = Symbol::apply_types_in_scope()
.into_iter()
.map(|(a, (b, c))| (a, b, c))
.collect();
Scope {
home,
@ -728,13 +726,7 @@ mod test {
assert_eq!(
&idents,
&[
Ident::from("Str"),
Ident::from("List"),
Ident::from("Box"),
Ident::from("Ok"),
Ident::from("Err"),
]
&[Ident::from("Str"), Ident::from("List"), Ident::from("Box"),]
);
}
@ -751,13 +743,7 @@ mod test {
assert_eq!(
&idents,
&[
Ident::from("Str"),
Ident::from("List"),
Ident::from("Box"),
Ident::from("Ok"),
Ident::from("Err"),
]
&[Ident::from("Str"), Ident::from("List"), Ident::from("Box"),]
);
let builtin_count = idents.len();

View file

@ -647,7 +647,7 @@ mod test_reporting {
if true then 1 else 2
"
),
@r"
@r###"
UNRECOGNIZED NAME in /code/proj/Main.roc
Nothing is named `true` in this scope.
@ -657,11 +657,11 @@ mod test_reporting {
Did you mean one of these?
Str
Frac
Num
Str
Err
"
U8
"###
);
test_report!(
@ -812,10 +812,10 @@ mod test_reporting {
Did you mean one of these?
Ok
List
Err
Box
Str
isDisabled
"
),
);
@ -2212,10 +2212,10 @@ mod test_reporting {
Did you mean one of these?
Ok
U8
Box
Eq
f
"
);
@ -5805,9 +5805,9 @@ All branches in an `if` must have the same type!
Did you mean one of these?
Str
Err
U8
F64
Box
"###
);

View file

@ -3395,6 +3395,7 @@ fn finish(
LoadedModule {
module_id: state.root_id,
filename: state.root_path,
interns,
solved,
can_problems: state.module_cache.can_problems,

View file

@ -30,6 +30,7 @@ use std::time::{Duration, Instant};
#[derive(Debug)]
pub struct LoadedModule {
pub module_id: ModuleId,
pub filename: PathBuf,
pub interns: Interns,
pub solved: Solved<Subs>,
pub can_problems: MutMap<ModuleId, Vec<roc_problem::can::Problem>>,
@ -54,6 +55,13 @@ pub struct LoadedModule {
}
impl LoadedModule {
/// Infer the filename for the given ModuleId, based on this root module's filename.
pub fn filename(&self, module_id: ModuleId) -> PathBuf {
let module_name = self.interns.module_name(module_id);
module_name.filename(&self.filename)
}
pub fn total_problems(&self) -> usize {
let mut total = 0;

View file

@ -989,8 +989,8 @@ fn issue_2863_module_type_does_not_exist() {
Did you mean one of these?
Decoding
Dict
Result
Dict
DecodeError
"
)

View file

@ -1,5 +1,8 @@
pub use roc_ident::IdentStr;
use std::fmt::{self, Debug};
use std::{
fmt::{self, Debug},
path::{Path, PathBuf},
};
use crate::symbol::PQModuleName;
@ -45,6 +48,19 @@ impl<'a> QualifiedModuleName<'a> {
#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct ModuleName(IdentStr);
impl ModuleName {
/// Given the root module's path, infer this module's path based on its name.
pub fn filename(&self, root_filename: impl AsRef<Path>) -> PathBuf {
let mut answer = root_filename.as_ref().with_file_name("");
for part in self.split('.') {
answer = answer.join(part);
}
answer.with_extension("roc")
}
}
impl std::ops::Deref for ModuleName {
type Target = str;

View file

@ -876,7 +876,6 @@ macro_rules! define_builtins {
module_id.register_debug_idents(&ident_ids);
}
exposed_idents_by_module.insert(
module_id,
ident_ids
@ -1016,27 +1015,6 @@ macro_rules! define_builtins {
m => roc_error_macros::internal_error!("{:?} is not a builtin module!", m),
}
}
/// Symbols that should be added to the default scope, for hints as suggestions of
/// names you might want to use.
///
/// TODO: this is a hack to get tag names to show up in error messages as suggestions,
/// really we should be extracting tag names from candidate type aliases in scope.
pub fn symbols_in_scope_for_hints() -> VecMap<Ident, (Symbol, Region)> {
let mut scope = VecMap::default();
$(
$(
$(
if $in_scope_for_hints {
scope.insert($ident_name.into(), (Symbol::new(ModuleId::$module_const, IdentId($ident_id)), Region::zero()));
}
)?
)*
)+
scope
}
}
};
}
@ -1434,15 +1412,13 @@ define_builtins! {
}
7 RESULT: "Result" => {
0 RESULT_RESULT: "Result" exposed_type=true // the Result.Result type alias
1 RESULT_OK: "Ok" in_scope_for_hints=true // Result.Result a e = [Ok a, Err e]
2 RESULT_ERR: "Err" in_scope_for_hints=true // Result.Result a e = [Ok a, Err e]
1 RESULT_IS_ERR: "isErr"
2 RESULT_ON_ERR: "onErr"
3 RESULT_MAP: "map"
4 RESULT_MAP_ERR: "mapErr"
5 RESULT_WITH_DEFAULT: "withDefault"
6 RESULT_TRY: "try"
7 RESULT_IS_OK: "isOk"
8 RESULT_IS_ERR: "isErr"
9 RESULT_ON_ERR: "onErr"
}
8 DICT: "Dict" => {
0 DICT_DICT: "Dict" exposed_type=true // the Dict.Dict type alias

View file

@ -50,17 +50,17 @@ procedure Num.22 (#Attr.2, #Attr.3):
let Num.275 : Int1 = lowlevel NumLt #Attr.2 #Attr.3;
ret Num.275;
procedure Result.5 (Result.12, Result.13):
let Result.39 : U8 = 1i64;
let Result.40 : U8 = GetTagId Result.12;
let Result.41 : Int1 = lowlevel Eq Result.39 Result.40;
if Result.41 then
dec Result.13;
let Result.14 : Str = UnionAtIndex (Id 1) (Index 0) Result.12;
ret Result.14;
procedure Result.5 (Result.10, Result.11):
let Result.37 : U8 = 1i64;
let Result.38 : U8 = GetTagId Result.10;
let Result.39 : Int1 = lowlevel Eq Result.37 Result.38;
if Result.39 then
dec Result.11;
let Result.12 : Str = UnionAtIndex (Id 1) (Index 0) Result.10;
ret Result.12;
else
dec Result.12;
ret Result.13;
dec Result.10;
ret Result.11;
procedure Test.10 (Test.11):
let Test.12 : Str = CallByName Test.2 Test.11;

View file

@ -21,6 +21,9 @@ roc_reporting = { path = "../reporting" }
roc_solve = { path = "../compiler/solve" }
roc_target = { path = "../compiler/roc_target" }
roc_types = { path = "../compiler/types" }
roc_problem = { path = "../compiler/problem" }
ven_pretty = { path = "../vendor/pretty" }
bumpalo.workspace = true
pulldown-cmark.workspace = true

View file

@ -13,6 +13,7 @@ use roc_packaging::cache::{self, RocCacheDir};
use roc_parse::ident::{parse_ident, Accessor, Ident};
use roc_parse::keyword;
use roc_parse::state::State;
use roc_problem::Severity;
use roc_region::all::Region;
use std::fs;
use std::path::{Path, PathBuf};
@ -146,7 +147,7 @@ pub fn generate_docs_html(root_file: PathBuf, build_dir: &Path) {
}
// Write each package module's index.html file
for (_, module_docs) in exposed_module_docs.iter() {
for (module_id, module_docs) in exposed_module_docs.iter() {
let module_name = module_docs.name.as_str();
let module_dir = build_dir.join(module_name.replace('.', "/").as_str());
@ -164,8 +165,13 @@ pub fn generate_docs_html(root_file: PathBuf, build_dir: &Path) {
)
.replace(
"<!-- Module Docs -->",
render_module_documentation(module_docs, &loaded_module, &all_exposed_symbols)
.as_str(),
render_module_documentation(
*module_id,
module_docs,
&loaded_module,
&all_exposed_symbols,
)
.as_str(),
);
fs::write(module_dir.join("index.html"), rendered_module)
@ -235,6 +241,7 @@ fn render_package_index(docs_by_module: &[(ModuleId, ModuleDocumentation)]) -> S
}
fn render_module_documentation(
module_id: ModuleId,
module: &ModuleDocumentation,
root_module: &LoadedModule,
all_exposed_symbols: &VecSet<Symbol>,
@ -292,6 +299,7 @@ fn render_module_documentation(
if let Some(docs) = &doc_def.docs {
markdown_to_html(
&mut buf,
&root_module.filename(module_id),
all_exposed_symbols,
&module.scope,
docs,
@ -305,6 +313,7 @@ fn render_module_documentation(
DocEntry::ModuleDoc(docs) => {
markdown_to_html(
&mut buf,
&root_module.filename(module_id),
all_exposed_symbols,
&module.scope,
docs,
@ -314,6 +323,7 @@ fn render_module_documentation(
DocEntry::DetachedDoc(docs) => {
markdown_to_html(
&mut buf,
&root_module.filename,
all_exposed_symbols,
&module.scope,
docs,
@ -899,13 +909,20 @@ struct DocUrl {
title: String,
}
enum LinkProblem {
MalformedAutoLink,
AutoLinkIdentNotInScope,
AutoLinkNotExposed,
AutoLinkModuleNotImported,
}
fn doc_url<'a>(
all_exposed_symbols: &VecSet<Symbol>,
scope: &Scope,
interns: &'a Interns,
mut module_name: &'a str,
ident: &str,
) -> DocUrl {
) -> Result<DocUrl, (String, LinkProblem)> {
if module_name.is_empty() {
// This is an unqualified lookup, so look for the ident
// in scope!
@ -918,10 +935,7 @@ fn doc_url<'a>(
module_name = symbol.module_string(interns);
}
Err(_) => {
// TODO return Err here
panic!(
"Tried to generate an automatic link in docs for symbol `{ident}`, but that symbol was not in scope in this module."
);
return Err((format!("[{ident}]"), LinkProblem::AutoLinkIdentNotInScope));
}
}
} else {
@ -940,9 +954,10 @@ fn doc_url<'a>(
// Note: You can do qualified lookups on your own module, e.g.
// if I'm in the Foo module, I can do a `Foo.bar` lookup.
else if !all_exposed_symbols.contains(&symbol) {
// TODO return Err here
panic!(
"Tried to generate an automatic link in docs for `{module_name}.{ident}`, but `{module_name}` does not expose `{ident}`.");
return Err((
format!("[{module_name}.{ident}]"),
LinkProblem::AutoLinkNotExposed,
));
}
// This is a valid symbol for this dependency,
@ -952,8 +967,10 @@ fn doc_url<'a>(
// incorporate the package name into the link.
}
None => {
// TODO return Err here
panic!("Tried to generate a doc link for `{module_name}.{ident}` but the `{module_name}` module was not imported!");
return Err((
format!("[{module_name}.{ident}]"),
LinkProblem::AutoLinkModuleNotImported,
));
}
}
}
@ -967,14 +984,15 @@ fn doc_url<'a>(
url.push('#');
url.push_str(ident);
DocUrl {
Ok(DocUrl {
url,
title: format!("Docs for {module_name}.{ident}"),
}
})
}
fn markdown_to_html(
buf: &mut String,
filename: &Path,
all_exposed_symbols: &VecSet<Symbol>,
scope: &Scope,
markdown: &str,
@ -1010,20 +1028,33 @@ fn markdown_to_html(
match iter.next() {
Some(Accessor::RecordField(symbol_name)) if iter.next().is_none() => {
let DocUrl { url, title } = doc_url(
match doc_url(
all_exposed_symbols,
scope,
&loaded_module.interns,
module_name,
symbol_name,
);
) {
Ok(DocUrl { url, title }) => Some((url.into(), title.into())),
Err((link_markdown, problem)) => {
report_markdown_link_problem(
loaded_module.module_id,
filename.to_path_buf(),
&link_markdown,
problem,
);
Some((url.into(), title.into()))
None
}
}
}
_ => {
// This had record field access,
// e.g. [foo.bar] - which we
// can't create a doc link to!
report_markdown_link_problem(
loaded_module.module_id,
filename.to_path_buf(),
&format!("[{}]", link.reference),
LinkProblem::MalformedAutoLink,
);
None
}
}
@ -1031,17 +1062,36 @@ fn markdown_to_html(
Ok((_, Ident::Tag(type_name), _)) => {
// This looks like a tag name, but it could
// be a type alias that's in scope, e.g. [I64]
let DocUrl { url, title } = doc_url(
match doc_url(
all_exposed_symbols,
scope,
&loaded_module.interns,
"",
type_name,
) {
Ok(DocUrl { url, title }) => Some((url.into(), title.into())),
Err((link_markdown, problem)) => {
report_markdown_link_problem(
loaded_module.module_id,
filename.to_path_buf(),
&link_markdown,
problem,
);
None
}
}
}
_ => {
report_markdown_link_problem(
loaded_module.module_id,
filename.to_path_buf(),
&format!("[{}]", link.reference),
LinkProblem::MalformedAutoLink,
);
Some((url.into(), title.into()))
None
}
_ => None,
}
}
_ => None,
@ -1137,3 +1187,62 @@ fn markdown_to_html(
pulldown_cmark::html::push_html(buf, docs_parser.into_iter());
}
/// TODO: this should be moved into Reporting, and the markdown checking
/// for docs should be part of `roc check`. Problems like these should
/// be reported as `roc check` warnings and included in the total count
/// of warnings at the end.
fn report_markdown_link_problem(
module_id: ModuleId,
filename: PathBuf,
link_markdown: &str,
problem: LinkProblem,
) {
use roc_reporting::report::{Report, RocDocAllocator, DEFAULT_PALETTE};
use ven_pretty::DocAllocator;
// Report parsing and canonicalization problems
let interns = Interns::default();
let alloc = RocDocAllocator::new(&[], module_id, &interns);
let report = {
const AUTO_LINK_TIP: &str = "Tip: When a link in square brackets doesn't have a URL immediately after it in parentheses, the part in square brackets needs to be the name of either an uppercase type in scope, or a lowercase value in scope. Then Roc will generate a link to its docs, if available.";
let link_problem = match problem {
LinkProblem::MalformedAutoLink => alloc.stack([
alloc.reflow("The part in square brackets is not a Roc type or value name that can be automatically linked to."),
alloc.reflow(AUTO_LINK_TIP),
]),
LinkProblem::AutoLinkIdentNotInScope => alloc.stack([
alloc.reflow("The name in square brackets was not found in scope."),
alloc.reflow(AUTO_LINK_TIP),
]),
LinkProblem::AutoLinkNotExposed => alloc.stack([
alloc.reflow("The name in square brackets is not exposed by the module where it's defined."),
alloc.reflow(AUTO_LINK_TIP),
]),
LinkProblem::AutoLinkModuleNotImported => alloc.stack([
alloc.reflow("The name in square brackets is not in scope because its module is not imported."),
alloc.reflow(AUTO_LINK_TIP),
])
};
let doc = alloc.stack([
alloc.reflow("This link in a doc comment is invalid:"),
alloc.reflow(link_markdown).indent(4),
link_problem,
]);
Report {
filename,
doc,
title: "INVALID DOCS LINK".to_string(),
severity: Severity::Warning,
}
};
let palette = DEFAULT_PALETTE;
let mut buf = String::new();
report.render_color_terminal(&mut buf, &alloc, &palette);
}

View file

@ -1200,6 +1200,22 @@ fn tag_with_type_behind_alias() {
);
}
#[test]
fn aliases_named_err_and_ok() {
expect_success(
indoc!(
r#"
Err : [Blah]
Ok : [Stuff]
v : (Err, Ok)
v = (Blah, Stuff)
v"#
),
r#"(Blah, Stuff) : ( [Blah], [Stuff] )*"#,
);
}
#[cfg(not(feature = "wasm"))]
#[test]
fn issue_2588_record_with_function_and_nonfunction() {