Report unused imports in modules

This commit is contained in:
Ayaz Hafiz 2022-09-21 15:05:55 -05:00
parent 5dc51ce444
commit 92aa0912ea
No known key found for this signature in database
GPG key ID: 0E2A37416A25EF58
8 changed files with 72 additions and 27 deletions

View file

@ -122,7 +122,7 @@ pub struct ExposedModuleTypes {
#[derive(Debug)]
pub struct Module {
pub module_id: ModuleId,
pub exposed_imports: MutMap<Symbol, Variable>,
pub exposed_imports: MutMap<Symbol, Region>,
pub exposed_symbols: VecSet<Symbol>,
pub referenced_values: VecSet<Symbol>,
pub referenced_types: VecSet<Symbol>,
@ -145,8 +145,7 @@ pub struct ModuleOutput {
pub aliases: MutMap<Symbol, Alias>,
pub rigid_variables: RigidVariables,
pub declarations: Declarations,
pub exposed_imports: MutMap<Symbol, Variable>,
pub lookups: Vec<(Symbol, Variable, Region)>,
pub exposed_imports: MutMap<Symbol, Region>,
pub problems: Vec<Problem>,
pub referenced_values: VecSet<Symbol>,
pub referenced_types: VecSet<Symbol>,
@ -277,7 +276,6 @@ pub fn canonicalize_module_defs<'a>(
let mut can_exposed_imports = MutMap::default();
let mut scope = Scope::new(home, exposed_ident_ids, imported_abilities_state);
let mut env = Env::new(arena, home, dep_idents, module_ids);
let num_deps = dep_idents.len();
for (name, alias) in aliases.into_iter() {
scope.add_alias(
@ -301,7 +299,6 @@ pub fn canonicalize_module_defs<'a>(
// rules multiple times unnecessarily.
crate::operator::desugar_defs(arena, loc_defs);
let mut lookups = Vec::with_capacity(num_deps);
let mut rigid_variables = RigidVariables::default();
// Exposed values are treated like defs that appear before any others, e.g.
@ -319,20 +316,13 @@ pub fn canonicalize_module_defs<'a>(
let first_char = ident.as_inline_str().as_str().chars().next().unwrap();
if first_char.is_lowercase() {
// this is a value definition
let expr_var = var_store.fresh();
match scope.import(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
// exposes [Bar.{ baz }] then insert Foo.baz as the key, so when
// anything references `baz` in this Foo module, it will resolve to Bar.baz.
can_exposed_imports.insert(symbol, expr_var);
// This will be used during constraint generation,
// to add the usual Lookup constraint as if this were a normal def.
lookups.push((symbol, expr_var, region));
can_exposed_imports.insert(symbol, region);
}
Err((_shadowed_symbol, _region)) => {
panic!("TODO gracefully handle shadowing in imports.")
@ -795,7 +785,6 @@ pub fn canonicalize_module_defs<'a>(
problems: env.problems,
symbols_from_requires,
pending_derives,
lookups,
loc_expects,
}
}

View file

@ -1984,13 +1984,16 @@ fn report_unused_imported_modules<'a>(
constrained_module: &ConstrainedModule,
) {
let mut unused_imported_modules = constrained_module.imported_modules.clone();
let mut unused_imports = constrained_module.module.exposed_imports.clone();
for symbol in constrained_module.module.referenced_values.iter() {
unused_imported_modules.remove(&symbol.module_id());
unused_imports.remove(&symbol);
}
for symbol in constrained_module.module.referenced_types.iter() {
unused_imported_modules.remove(&symbol.module_id());
unused_imports.remove(&symbol);
}
let existing = match state.module_cache.can_problems.entry(module_id) {
@ -2000,9 +2003,15 @@ fn report_unused_imported_modules<'a>(
for (unused, region) in unused_imported_modules.drain() {
if !unused.is_builtin() {
existing.push(roc_problem::can::Problem::UnusedImport(unused, region));
existing.push(roc_problem::can::Problem::UnusedModuleImport(
unused, region,
));
}
}
for (unused, region) in unused_imports.drain() {
existing.push(roc_problem::can::Problem::UnusedImport(unused, region));
}
}
fn extend_header_with_builtin(header: &mut ModuleHeader, module: ModuleId) {
@ -3539,7 +3548,7 @@ fn send_header<'a>(
}
};
let mut imported: Vec<(QualifiedModuleName, Vec<Ident>, Region)> =
let mut imported: Vec<(QualifiedModuleName, Vec<Loc<Ident>>, Region)> =
Vec::with_capacity(imports.len());
let mut imported_modules: MutMap<ModuleId, Region> = MutMap::default();
let mut scope_size = 0;
@ -3611,7 +3620,11 @@ fn send_header<'a>(
// to the same symbols as the ones we're using here.
let ident_ids = ident_ids_by_module.get_or_insert(module_id);
for ident in exposed_idents {
for Loc {
region,
value: ident,
} in exposed_idents
{
let ident_id = ident_ids.get_or_insert(ident.as_str());
let symbol = Symbol::new(module_id, ident_id);
@ -3750,7 +3763,7 @@ fn send_header_two<'a>(
let declared_name: ModuleName = "".into();
let mut symbols_from_requires = Vec::with_capacity(requires.len());
let mut imported: Vec<(QualifiedModuleName, Vec<Ident>, Region)> =
let mut imported: Vec<(QualifiedModuleName, Vec<Loc<Ident>>, Region)> =
Vec::with_capacity(imports.len());
let mut imported_modules: MutMap<ModuleId, Region> = MutMap::default();
@ -3835,7 +3848,11 @@ fn send_header_two<'a>(
// to the same symbols as the ones we're using here.
let ident_ids = ident_ids_by_module.get_or_insert(module_id);
for ident in exposed_idents {
for Loc {
region,
value: ident,
} in exposed_idents
{
let ident_id = ident_ids.get_or_insert(ident.as_str());
let symbol = Symbol::new(module_id, ident_id);
@ -4657,7 +4674,7 @@ fn parse<'a>(arena: &'a Bump, header: ModuleHeader<'a>) -> Result<Msg<'a>, Loadi
Ok(Msg::Parsed(parsed))
}
fn exposed_from_import<'a>(entry: &ImportsEntry<'a>) -> (QualifiedModuleName<'a>, Vec<Ident>) {
fn exposed_from_import<'a>(entry: &ImportsEntry<'a>) -> (QualifiedModuleName<'a>, Vec<Loc<Ident>>) {
use roc_parse::header::ImportsEntry::*;
match entry {
@ -4665,7 +4682,7 @@ fn exposed_from_import<'a>(entry: &ImportsEntry<'a>) -> (QualifiedModuleName<'a>
let mut exposed = Vec::with_capacity(exposes.len());
for loc_entry in exposes.iter() {
exposed.push(ident_from_exposed(&loc_entry.value));
exposed.push(loc_entry.map(ident_from_exposed));
}
let qualified_module_name = QualifiedModuleName {
@ -4680,7 +4697,7 @@ fn exposed_from_import<'a>(entry: &ImportsEntry<'a>) -> (QualifiedModuleName<'a>
let mut exposed = Vec::with_capacity(exposes.len());
for loc_entry in exposes.iter() {
exposed.push(ident_from_exposed(&loc_entry.value));
exposed.push(loc_entry.map(ident_from_exposed));
}
let qualified_module_name = QualifiedModuleName {

View file

@ -31,7 +31,8 @@ pub enum ShadowKind {
#[derive(Clone, Debug, PartialEq)]
pub enum Problem {
UnusedDef(Symbol, Region),
UnusedImport(ModuleId, Region),
UnusedImport(Symbol, Region),
UnusedModuleImport(ModuleId, Region),
ExposedButNotDefined(Symbol),
UnknownGeneratesWith(Loc<Ident>),
/// First symbol is the name of the closure with that argument

View file

@ -144,7 +144,7 @@ pub fn helper(
for problem in can_problems.into_iter() {
// Ignore "unused" problems
match problem {
UnusedDef(_, _) | UnusedArgument(_, _, _, _) | UnusedImport(_, _) => {
UnusedDef(_, _) | UnusedArgument(_, _, _, _) | UnusedModuleImport(_, _) => {
delayed_errors.push(problem);
continue;
}

View file

@ -129,7 +129,7 @@ fn create_llvm_module<'a>(
// Ignore "unused" problems
UnusedDef(_, _)
| UnusedArgument(_, _, _, _)
| UnusedImport(_, _)
| UnusedModuleImport(_, _)
| RuntimeError(_)
| UnsupportedPattern(_, _)
| ExposedButNotDefined(_) => {

View file

@ -87,7 +87,24 @@ pub fn can_problem<'b>(
title = UNUSED_DEF.to_string();
severity = Severity::Warning;
}
Problem::UnusedImport(module_id, region) => {
Problem::UnusedImport(symbol, region) => {
doc = alloc.stack([
alloc.concat([
alloc.symbol_qualified(symbol),
alloc.reflow(" is not used in this module."),
]),
alloc.region(lines.convert_region(region)),
alloc.concat([
alloc.reflow("Since "),
alloc.symbol_qualified(symbol),
alloc.reflow(" isn't used, you don't need to import it."),
]),
]);
title = UNUSED_IMPORT.to_string();
severity = Severity::Warning;
}
Problem::UnusedModuleImport(module_id, region) => {
doc = alloc.stack([
alloc.concat([
alloc.reflow("Nothing from "),

View file

@ -10718,6 +10718,27 @@ All branches in an `if` must have the same type!
"###
);
test_report!(
unused_value_import,
indoc!(
r#"
app "test" imports [List.{ concat }] provides [main] to "./platform"
main = ""
"#
),
@r###"
UNUSED IMPORT /code/proj/Main.roc
`List.concat` is not used in this module.
1 app "test" imports [List.{ concat }] provides [main] to "./platform"
^^^^^^
Since `List.concat` isn't used, you don't need to import it.
"###
);
test_report!(
unnecessary_builtin_module_import,
indoc!(

View file

@ -1,6 +1,6 @@
app "args"
packages { pf: "cli-platform/main.roc" }
imports [pf.Stdout, pf.Arg, pf.Program.{ Program }]
imports [pf.Stdout, pf.Arg, pf.Program.{ Program, exitCode }]
provides [main] to pf
main : Program