language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-08-04 12:18:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Load and can imports inside defs

Browse source 
After parsing a module, we now recursively traverse the tree to find
all imports inside Defs, not just the top-level ones.

Previously, imported modules were available in the entire file,
but that's no longer the case. Therefore, Scope now keeps track of
imported modules and Env::qualified_lookup checks whether a module
is available in the provided scope.

Note: Unused import warnings are still global and need to be updated.
This commit is contained in:
Agus Zubiaga 2024-01-06 18:34:04 -03:00
parent 710d62f754
commit c617963b22
No known key found for this signature in database
10 changed files with 434 additions and 84 deletions
Download patch file Download diff file Expand all files Collapse all files

51
crates/compiler/can/src/def.rs
View file

@ -28,6 +28,7 @@ use roc_collections::{ImSet, MutMap, SendMap};
use roc_error_macros::internal_error;
use roc_module::ident::Ident;
use roc_module::ident::Lowercase;
use roc_module::ident::ModuleName;
use roc_module::symbol::IdentId;
use roc_module::symbol::ModuleId;
use roc_module::symbol::Symbol;
@ -2887,23 +2888,32 @@ fn to_pending_value_def<'a>(
}),
ModuleImport(module_import) => {
let module_name = ModuleName::from(module_import.name.value.name);
let module_id = env
.module_ids
.get_id(&module_name)
.expect("Module id should have been added in load");
scope.import_module(module_id);
match module_import.exposed {
None => {}
Some(exposed) if exposed.item.is_empty() => {}
Some(exposed) => {
for loc_name in exposed.item.items {
Some(exposed_kw) => {
let exposed_ids = env
.dep_idents
.get(&module_id)
.expect("Module id should have been added in load");
for loc_name in exposed_kw.item.items {
let exposed_name = loc_name.value.item();
let name = exposed_name.as_str();
let ident = name.into();
let ident = Ident::from(name);
match env.qualified_lookup(
scope,
module_import.name.value.name,
name,
loc_name.region,
) {
Ok(imported_symbol) => {
match scope.import(ident, imported_symbol, loc_name.region) {
match exposed_ids.get_id(name) {
Some(ident_id) => {
let symbol = Symbol::new(module_id, ident_id);
match scope.import_symbol(ident, symbol, loc_name.region) {
Ok(()) => {}
Err((_shadowed_symbol, _region)) => {
internal_error!(
@ -2912,8 +2922,21 @@ fn to_pending_value_def<'a>(
}
}
}
Err(problem) => {
env.problem(Problem::RuntimeError(problem.clone()));
None => {
let exposed_values = exposed_ids
.ident_strs()
.filter(|(_, ident)| {
ident.starts_with(|c: char| c.is_lowercase())
})
.map(|(_, ident)| Lowercase::from(ident))
.collect();
env.problem(Problem::RuntimeError(RuntimeError::ValueNotExposed {
module_name: module_name.clone(),
ident,
region: loc_name.region,
exposed_values,
}))
}
}
}

58
crates/compiler/can/src/env.rs
View file

@ -141,37 +141,39 @@ impl<'a> Env<'a> {
}
} else {
match self.dep_idents.get(&module_id) {
Some(exposed_ids) => match exposed_ids.get_id(ident) {
Some(ident_id) => {
let symbol = Symbol::new(module_id, ident_id);
Some(exposed_ids) if scope.has_imported_module(&module_id) => {
match exposed_ids.get_id(ident) {
Some(ident_id) => {
let symbol = Symbol::new(module_id, ident_id);
if is_type_name {
self.qualified_type_lookups.insert(symbol);
} else {
self.qualified_value_lookups.insert(symbol);
if is_type_name {
self.qualified_type_lookups.insert(symbol);
} else {
self.qualified_value_lookups.insert(symbol);
}
Ok(symbol)
}
None => {
let exposed_values = exposed_ids
.ident_strs()
.filter(|(_, ident)| ident.starts_with(|c: char| c.is_lowercase()))
.map(|(_, ident)| Lowercase::from(ident))
.collect();
Err(RuntimeError::ValueNotExposed {
module_name: self
.module_ids
.get_name(module_id)
.expect("Module ID known, but not in the module IDs somehow")
.clone(),
ident: Ident::from(ident),
region,
exposed_values,
})
}
Ok(symbol)
}
None => {
let exposed_values = exposed_ids
.ident_strs()
.filter(|(_, ident)| ident.starts_with(|c: char| c.is_lowercase()))
.map(|(_, ident)| Lowercase::from(ident))
.collect();
Err(RuntimeError::ValueNotExposed {
module_name: self
.module_ids
.get_name(module_id)
.expect("Module ID known, but not in the module IDs somehow")
.clone(),
ident: Ident::from(ident),
region,
exposed_values,
})
}
},
None => Err(RuntimeError::ModuleNotImported {
}
_ => Err(RuntimeError::ModuleNotImported {
module_name: self
.module_ids
.get_name(module_id)

4
crates/compiler/can/src/module.rs
View file

@ -326,7 +326,7 @@ pub fn canonicalize_module_defs<'a>(
let first_char = ident.as_inline_str().as_str().chars().next().unwrap();
if first_char.is_lowercase() {
match scope.import(ident, symbol, region) {
match scope.import_symbol(ident, symbol, region) {
Ok(()) => {
// Add an entry to exposed_imports using the current module's name
// as the key; e.g. if this is the Foo module and we have
@ -349,7 +349,7 @@ pub fn canonicalize_module_defs<'a>(
// but now we know this symbol by a different identifier, so we still need to add it to
// the scope
match scope.import(ident, symbol, region) {
match scope.import_symbol(ident, symbol, region) {
Ok(()) => {
// here we do nothing special
}

47
crates/compiler/can/src/scope.rs
View file

@ -29,8 +29,11 @@ pub struct Scope {
/// The first `exposed_ident_count` identifiers are exposed
exposed_ident_count: usize,
/// Identifiers that are imported (and introduced in the header)
imports: Vec<(Ident, Symbol, Region)>,
/// Modules that are imported
imported_modules: Vec<ModuleId>,
/// Identifiers that are imported
imported_symbols: Vec<(Ident, Symbol, Region)>,
/// Shadows of an ability member, for example a local specialization of `eq` for the ability
/// member `Eq implements eq : a, a -> Bool where a implements Eq` gets a shadow symbol it can use for its
@ -68,7 +71,8 @@ impl Scope {
aliases: VecMap::default(),
abilities_store: starting_abilities_store,
shadows: VecMap::default(),
imports: default_imports,
imported_modules: Vec::default(),
imported_symbols: default_imports,
ignored_locals: VecMap::default(),
}
}
@ -82,9 +86,9 @@ impl Scope {
}
pub fn add_docs_imports(&mut self) {
self.imports
self.imported_symbols
.push(("Dict".into(), Symbol::DICT_DICT, Region::zero()));
self.imports
self.imported_symbols
.push(("Set".into(), Symbol::SET_SET, Region::zero()));
}
@ -113,7 +117,7 @@ impl Scope {
fn idents_in_scope(&self) -> impl Iterator<Item = Ident> + '_ {
let it1 = self.locals.idents_in_scope();
let it2 = self.imports.iter().map(|t| t.0.clone());
let it2 = self.imported_symbols.iter().map(|t| t.0.clone());
it2.chain(it1)
}
@ -139,7 +143,7 @@ impl Scope {
},
None => {
// opaque types can only be wrapped/unwrapped in the scope they are defined in (and below)
let error = if let Some((_, decl_region)) = self.has_imported(opaque_str) {
let error = if let Some((_, decl_region)) = self.has_imported_symbol(opaque_str) {
// specific error for when the opaque is imported, which definitely does not work
RuntimeError::OpaqueOutsideScope {
opaque,
@ -202,8 +206,12 @@ impl Scope {
}
}
fn has_imported(&self, ident: &str) -> Option<(Symbol, Region)> {
for (import, shadow, original_region) in self.imports.iter() {
pub fn has_imported_module(&self, module_id: &ModuleId) -> bool {
module_id.is_builtin() || self.imported_modules.contains(module_id)
}
fn has_imported_symbol(&self, ident: &str) -> Option<(Symbol, Region)> {
for (import, shadow, original_region) in self.imported_symbols.iter() {
if ident == import.as_str() {
return Some((*shadow, *original_region));
}
@ -215,7 +223,7 @@ impl Scope {
/// Is an identifier in scope, either in the locals or imports
fn scope_contains_ident(&self, ident: &str) -> ContainsIdent {
// exposed imports are likely to be small
match self.has_imported(ident) {
match self.has_imported_symbol(ident) {
Some((symbol, region)) => ContainsIdent::InScope(symbol, region),
None => self.locals.contains_ident(ident),
}
@ -379,21 +387,25 @@ impl Scope {
///
/// Returns Err if this would shadow an existing ident, including the
/// Symbol and Region of the ident we already had in scope under that name.
pub fn import(
pub fn import_symbol(
&mut self,
ident: Ident,
symbol: Symbol,
region: Region,
) -> Result<(), (Symbol, Region)> {
if let Some((s, r)) = self.has_imported(ident.as_str()) {
if let Some((s, r)) = self.has_imported_symbol(ident.as_str()) {
return Err((s, r));
}
self.imports.push((ident, symbol, region));
self.imported_symbols.push((ident, symbol, region));
Ok(())
}
pub fn import_module(&mut self, module_id: ModuleId) {
self.imported_modules.push(module_id);
}
pub fn add_alias(
&mut self,
name: Symbol,
@ -423,17 +435,22 @@ impl Scope {
//
// - abilities_store: ability definitions not allowed in inner scopes
// - locals: everything introduced in the inner scope is marked as not in scope in the rollback
// - imports: everything that was imported in the inner scope is removed in the rollback
// - aliases: stored in a VecMap, we just discard anything added in an inner scope
// - exposed_ident_count: unchanged
// - home: unchanged
let aliases_count = self.aliases.len();
let ignored_locals_count = self.ignored_locals.len();
let locals_snapshot = self.locals.in_scope.len();
let imported_modules_snapshot = self.imported_modules.len();
let imported_symbols_snapshot = self.imported_symbols.len();
let result = f(self);
self.aliases.truncate(aliases_count);
self.ignored_locals.truncate(ignored_locals_count);
self.imported_modules.truncate(imported_modules_snapshot);
self.imported_symbols.truncate(imported_symbols_snapshot);
// anything added in the inner scope is no longer in scope now
for i in locals_snapshot..self.locals.in_scope.len() {
@ -820,7 +837,7 @@ mod test {
assert!(scope.lookup(&ident, region).is_err());
assert!(scope.import(ident.clone(), symbol, region).is_ok());
assert!(scope.import_symbol(ident.clone(), symbol, region).is_ok());
assert!(scope.lookup(&ident, region).is_ok());
@ -842,7 +859,7 @@ mod test {
let region1 = Region::from_pos(Position { offset: 10 });
let region2 = Region::from_pos(Position { offset: 20 });
scope.import(ident.clone(), symbol, region1).unwrap();
scope.import_symbol(ident.clone(), symbol, region1).unwrap();
let (original, _ident, shadow_symbol) =
scope.introduce(ident.clone(), region2).unwrap_err();

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

@ -2148,6 +2148,7 @@ fn report_unused_imported_modules(
module_id: ModuleId,
constrained_module: &ConstrainedModule,
) {
// [modules-revamp] TODO: take expr-level into account
let mut unused_imported_modules = constrained_module.imported_modules.clone();
let mut unused_imports = constrained_module.module.exposed_imports.clone();
@ -5228,35 +5229,23 @@ fn parse<'a>(
let mut imported: Vec<(QualifiedModuleName, Region)> = vec![];
for (index, either_index) in parsed_defs.tags.iter().enumerate() {
if let Err(value_index) = either_index.split() {
let value_def = &parsed_defs.value_defs[value_index.index()];
let region = parsed_defs.regions[index];
for (def, region) in ast::RecursiveValueDefIter::new(&parsed_defs) {
match def {
ValueDef::ModuleImport(import) => {
let qualified_module_name = QualifiedModuleName {
opt_package: import.name.value.package,
module: import.name.value.name.into(),
};
match value_def {
ValueDef::Annotation(..) => {}
ValueDef::ModuleImport(import) => {
let qualified_module_name = QualifiedModuleName {
opt_package: import.name.value.package,
module: import.name.value.name.into(),
};
imported.push((qualified_module_name, region));
}
ValueDef::IngestedFileImport(_ingested_file) => {
todo!("[modules-revamp] do we need to do anything here?")
}
_ => {
// [modules-revamp] TODO: traverse exprs
}
imported.push((qualified_module_name, *region));
}
ValueDef::IngestedFileImport(_ingested_file) => {
todo!("[modules-revamp] do we need to do anything here?")
}
_ => {}
}
}
let mut exposed: Vec<Symbol> = Vec::with_capacity(num_exposes);
// Make sure the module_ids has ModuleIds for all our deps,

8
crates/compiler/load_internal/tests/fixtures/build/interface_with_deps/ExposedUsedOutsideScope.roc vendored Normal file
View file

@ -0,0 +1,8 @@
interface ExposedUsedOutsideScope exposes [good, bad] imports []
good =
import Dep2 exposing [two]
two
bad =
two

12
crates/compiler/load_internal/tests/fixtures/build/interface_with_deps/ImportInsideDef.roc vendored Normal file
View file

@ -0,0 +1,12 @@
interface ImportInsideDef exposes [dep1Str, dep2TwoDobuled] imports []
dep1Str =
import Dep1
Dep1.str
dep2TwoDobuled =
2
* (
import Dep2 exposing [two]
two
)

8
crates/compiler/load_internal/tests/fixtures/build/interface_with_deps/ImportUsedOutsideScope.roc vendored Normal file
View file

@ -0,0 +1,8 @@
interface ImportUsedOutsideScope exposes [good, bad] imports []
good =
import Dep2
Dep2.two
bad =
Dep2.two

45
crates/compiler/load_internal/tests/test_load.rs
View file

@ -438,6 +438,42 @@ fn import_alias() {
);
}
#[test]
fn import_inside_def() {
let subs_by_module = Default::default();
let loaded_module = load_fixture("interface_with_deps", "ImportInsideDef", subs_by_module);
expect_types(
loaded_module,
hashmap! {
"dep1Str" => "Str",
"dep2TwoDobuled" => "Frac *",
},
);
}
#[test]
#[should_panic(expected = "LookupNotInScope")]
fn exposed_used_outside_scope() {
let subs_by_module = Default::default();
load_fixture(
"interface_with_deps",
"ExposedUsedOutsideScope",
subs_by_module,
);
}
#[test]
#[should_panic(expected = "ModuleNotImported")]
fn import_used_outside_scope() {
let subs_by_module = Default::default();
load_fixture(
"interface_with_deps",
"ImportUsedOutsideScope",
subs_by_module,
);
}
#[test]
fn test_load_and_typecheck() {
let subs_by_module = Default::default();
@ -878,6 +914,15 @@ fn opaque_wrapped_unwrapped_outside_defining_module() {
^^^
Note: Opaque types can only be wrapped and unwrapped in the module they are defined in!
── UNUSED IMPORT ─── tmp/opaque_wrapped_unwrapped_outside_defining_module/Main ─
Nothing from Age is used in this module.
3│ import Age exposing [Age]
^^^^^^^^^^^^^^^^^^^^^^^^^
Since Age isn't used, you don't need to import it.
"
),
"\n{}",

246
crates/compiler/parse/src/ast.rs
View file

@ -465,6 +465,252 @@ pub enum ValueDef<'a> {
IngestedFileImport(IngestedFileImport<'a>),
}
impl<'a> ValueDef<'a> {
pub fn expr(&self) -> Option<&'a Expr<'a>> {
match self {
ValueDef::Body(_, body) => Some(&body.value),
ValueDef::AnnotatedBody {
ann_pattern: _,
ann_type: _,
comment: _,
body_pattern: _,
body_expr,
} => Some(&body_expr.value),
ValueDef::Dbg {
condition,
preceding_comment: _,
}
| ValueDef::Expect {
condition,
preceding_comment: _,
}
| ValueDef::ExpectFx {
condition,
preceding_comment: _,
} => Some(&condition.value),
ValueDef::Annotation(_, _)
| ValueDef::ModuleImport(ModuleImport {
before_name: _,
name: _,
alias: _,
exposed: _,
})
| ValueDef::IngestedFileImport(_) => None,
}
}
}
pub struct RecursiveValueDefIter<'a, 'b> {
current: &'b Defs<'a>,
index: usize,
pending: std::vec::Vec<&'b Defs<'a>>,
}
impl<'a, 'b> RecursiveValueDefIter<'a, 'b> {
pub fn new(defs: &'b Defs<'a>) -> Self {
Self {
current: defs,
index: 0,
pending: vec![],
}
}
fn push_pending_from_expr(&mut self, expr: &'b Expr<'a>) {
let mut expr_stack = vec![expr];
use Expr::*;
macro_rules! push_stack_from_record_fields {
($fields:expr) => {
for field in $fields.items {
let mut current = field.value;
loop {
use AssignedField::*;
match current {
RequiredValue(_, _, loc_val) => break expr_stack.push(&loc_val.value),
OptionalValue(_, _, loc_val) => break expr_stack.push(&loc_val.value),
SpaceBefore(next, _) => current = *next,
SpaceAfter(next, _) => current = *next,
LabelOnly(_) | Malformed(_) => break,
}
}
}
};
}
while let Some(next) = expr_stack.pop() {
match next {
Defs(defs, cont) => {
self.pending.push(defs);
// We purposefully don't push the exprs inside defs here
// because they will be traversed when the iterator
// gets to their parent def.
expr_stack.push(&cont.value);
}
RecordAccess(expr, _) => expr_stack.push(expr),
TupleAccess(expr, _) => expr_stack.push(expr),
List(list) => {
expr_stack.reserve(list.len());
for loc_expr in list.items {
expr_stack.push(&loc_expr.value);
}
}
RecordUpdate { update, fields } => {
expr_stack.reserve(fields.len() + 1);
expr_stack.push(&update.value);
push_stack_from_record_fields!(fields);
}
Record(fields) => {
expr_stack.reserve(fields.len());
push_stack_from_record_fields!(fields);
}
Tuple(fields) => {
expr_stack.reserve(fields.len());
for loc_expr in fields.items {
expr_stack.push(&loc_expr.value);
}
}
RecordBuilder(fields) => {
expr_stack.reserve(fields.len());
for loc_record_builder_field in fields.items {
let mut current_field = loc_record_builder_field.value;
loop {
use RecordBuilderField::*;
match current_field {
Value(_, _, loc_val) => break expr_stack.push(&loc_val.value),
ApplyValue(_, _, _, loc_val) => {
break expr_stack.push(&loc_val.value)
}
SpaceBefore(next_field, _) => current_field = *next_field,
SpaceAfter(next_field, _) => current_field = *next_field,
LabelOnly(_) | Malformed(_) => break,
}
}
}
}
Closure(_, body) => expr_stack.push(&body.value),
Backpassing(_, a, b) => {
expr_stack.reserve(2);
expr_stack.push(&a.value);
expr_stack.push(&b.value);
}
Expect(condition, cont)
| Dbg(condition, cont)
| LowLevelDbg(_, condition, cont) => {
expr_stack.reserve(2);
expr_stack.push(&condition.value);
expr_stack.push(&cont.value);
}
Apply(fun, args, _) => {
expr_stack.reserve(args.len() + 1);
expr_stack.push(&fun.value);
for loc_expr in args.iter() {
expr_stack.push(&loc_expr.value);
}
}
BinOps(ops, expr) => {
expr_stack.reserve(ops.len() + 1);
for (a, _) in ops.iter() {
expr_stack.push(&a.value);
}
expr_stack.push(&expr.value);
}
UnaryOp(expr, _) => expr_stack.push(&expr.value),
If(ifs, alternate) => {
expr_stack.reserve(ifs.len() * 2 + 1);
for (condition, consequent) in ifs.iter() {
expr_stack.push(&condition.value);
expr_stack.push(&consequent.value);
}
expr_stack.push(&alternate.value);
}
When(condition, branches) => {
expr_stack.reserve(branches.len() + 1);
expr_stack.push(&condition.value);
for WhenBranch {
patterns: _,
value,
guard,
} in branches.iter()
{
expr_stack.push(&value.value);
match guard {
None => {}
Some(guard) => expr_stack.push(&guard.value),
}
}
}
SpaceBefore(expr, _) => expr_stack.push(expr),
SpaceAfter(expr, _) => expr_stack.push(expr),
ParensAround(expr) => expr_stack.push(expr),
MultipleRecordBuilders(expr) => expr_stack.push(&expr.value),
UnappliedRecordBuilder(expr) => expr_stack.push(&expr.value),
Float(_)
| Num(_)
| NonBase10Int { .. }
| Str(_)
| SingleQuote(_)
| AccessorFunction(_)
| IngestedFile(_, _)
| Var { .. }
| Underscore(_)
| Crash
| Tag(_)
| OpaqueRef(_)
| MalformedIdent(_, _)
| MalformedClosure
| PrecedenceConflict(_) => { /* terminal */ }
}
}
}
}
impl<'a, 'b> Iterator for RecursiveValueDefIter<'a, 'b> {
type Item = (&'b ValueDef<'a>, &'b Region);
fn next(&mut self) -> Option<Self::Item> {
match self.current.tags.get(self.index) {
Some(tag) => {
if let Err(def_index) = tag.split() {
let def = &self.current.value_defs[def_index.index()];
let region = &self.current.regions[self.index];
if let Some(expr) = def.expr() {
self.push_pending_from_expr(expr);
}
self.index += 1;
Some((def, region))
} else {
// Not a value def, try next
self.index += 1;
self.next()
}
}
None => {
self.current = self.pending.pop()?;
self.index = 0;
self.next()
}
}
}
}
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct ModuleImport<'a> {
pub before_name: &'a [CommentOrNewline<'a>],