language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-08-03 03:42:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

Improve reporting for doc links problems

Browse source
This commit is contained in:
Richard Feldman 2024-04-26 16:31:20 -04:00
parent 00950d2a0e
commit 17d761252f
No known key found for this signature in database
GPG key ID: F1F21AA5B1D9E43B
6 changed files with 164 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

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]]

1
crates/compiler/load_internal/src/file.rs
View file

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

8
crates/compiler/load_internal/src/module.rs
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;

18
crates/compiler/module/src/ident.rs
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;

3
crates/docs/Cargo.toml
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

153
crates/docs/src/lib.rs
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,7 +165,12 @@ pub fn generate_docs_html(root_file: PathBuf, build_dir: &Path) {
)
.replace(
"<!-- Module Docs -->",
render_module_documentation(module_docs, &loaded_module, &all_exposed_symbols)
render_module_documentation(
*module_id,
module_docs,
&loaded_module,
&all_exposed_symbols,
)
.as_str(),
);
@ -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,
);
Some((url.into(), title.into()))
None
}
}
}
_ => {
report_markdown_link_problem(
loaded_module.module_id,
filename.to_path_buf(),
&format!("[{}]", link.reference),
LinkProblem::MalformedAutoLink,
);
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);
}