Skip to content

Commit 064d894

Browse files
giacomocavalierilpil
authored andcommitted
stop warning for modules that are used only by private functions
1 parent f7dc3b6 commit 064d894

8 files changed

+139
-36
lines changed

CHANGELOG.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,9 @@
304304

305305
([Surya Rose](https://github.com/GearsDatapacks))
306306

307-
- The "Add type annotations" and "Generate function" code actions now ignore type
308-
variables defined in other functions, improving the generated code. For example:
307+
- The "Add type annotations" and "Generate function" code actions now ignore
308+
type variables defined in other functions, improving the generated code.
309+
For example:
309310

310311
```gleam
311312
fn something(a: a, b: b, c: c) -> d { todo }
@@ -369,8 +370,8 @@
369370

370371
### Bug fixes
371372

372-
- Fixed a bug where literals using `\u{XXXX}` syntax in bit array pattern segments were not
373-
translated to Erlang's `\x{XXXX}` syntax correctly.
373+
- Fixed a bug where literals using `\u{XXXX}` syntax in bit array pattern
374+
segments were not translated to Erlang's `\x{XXXX}` syntax correctly.
374375
([Benjamin Peinhardt](https://github.com/bcpeinhardt))
375376

376377
- Fixed a bug where `echo` could crash on JavaScript if the module contains
@@ -433,3 +434,7 @@
433434
- Fixed a bug where the compiler would reference a redeclared variable in a let
434435
assert message, instead of the original variable, on the Erlang target.
435436
([Danielle Maywood](https://github.com/DanielleMaywood))
437+
438+
- Fixed a bug where the compiler would report an imported module as unused if it
439+
were used by private functions only.
440+
([Giacomo Cavalieri](https://github.com/giacomocavalieri))

compiler-core/src/reference.rs

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::HashMap;
1+
use std::collections::{HashMap, HashSet};
22

33
use crate::ast::{Publicity, SrcSpan};
44
use bimap::{BiMap, Overwritten};
@@ -84,7 +84,7 @@ pub struct ReferenceTracker {
8484
graph: StableGraph<(), (), Directed>,
8585
entities: BiMap<Entity, NodeIndex>,
8686
current_node: NodeIndex,
87-
public_entities: Vec<Entity>,
87+
public_entities: HashSet<Entity>,
8888
entity_information: HashMap<Entity, EntityInformation>,
8989

9090
/// The locations of the references to each value in this module, used for
@@ -193,7 +193,7 @@ impl ReferenceTracker {
193193
self.create_node_and_maybe_shadow(entity.clone(), self.current_node);
194194
match publicity {
195195
Publicity::Public | Publicity::Internal { .. } => {
196-
self.public_entities.push(entity.clone());
196+
let _ = self.public_entities.insert(entity.clone());
197197
}
198198
Publicity::Private => {}
199199
}
@@ -223,7 +223,7 @@ impl ReferenceTracker {
223223
};
224224
match publicity {
225225
Publicity::Public | Publicity::Internal { .. } => {
226-
self.public_entities.push(entity.clone());
226+
let _ = self.public_entities.insert(entity.clone());
227227
}
228228
Publicity::Private => {}
229229
}
@@ -257,7 +257,7 @@ impl ReferenceTracker {
257257
};
258258
match publicity {
259259
Publicity::Public | Publicity::Internal { .. } => {
260-
self.public_entities.push(entity.clone());
260+
let _ = self.public_entities.insert(entity.clone());
261261
}
262262
Publicity::Private => {}
263263
}
@@ -423,6 +423,20 @@ impl ReferenceTracker {
423423
}
424424
}
425425

426+
for (entity, _) in self.entities.iter() {
427+
let Some(index) = self.entities.get_by_left(entity) else {
428+
continue;
429+
};
430+
431+
if self.public_entities.contains(entity) {
432+
self.mark_entity_as_used(&mut unused_values, entity, *index);
433+
} else {
434+
// If the entity is not public, we still want to mark referenced
435+
// imports as used.
436+
self.mark_referenced_imports_as_used(&mut unused_values, entity, *index);
437+
}
438+
}
439+
426440
unused_values
427441
}
428442

@@ -440,4 +454,45 @@ impl ReferenceTracker {
440454
}
441455
}
442456
}
457+
458+
fn mark_referenced_imports_as_used(
459+
&self,
460+
unused: &mut HashMap<Entity, EntityInformation>,
461+
entity: &Entity,
462+
index: NodeIndex,
463+
) {
464+
// If the entity is a module there's no way it can reference other
465+
// modules so we just ignore it.
466+
// This also means that module aliases do not count as using a module!
467+
if entity.layer == EntityLayer::Module {
468+
return;
469+
}
470+
471+
for node in self.graph.neighbors_directed(index, Direction::Outgoing) {
472+
// We only want to mark referenced modules as used, so if the node
473+
// is not a module we just skip it.
474+
let Some(
475+
module @ Entity {
476+
layer: EntityLayer::Module,
477+
..
478+
},
479+
) = self.entities.get_by_right(&node)
480+
else {
481+
continue;
482+
};
483+
484+
// If the value appears in the module import list, it doesn't count
485+
// as using it!
486+
let is_imported_type = self
487+
.type_references
488+
.contains_key(&(module.name.clone(), entity.name.clone()));
489+
let is_imported_value = self
490+
.value_references
491+
.contains_key(&(module.name.clone(), entity.name.clone()));
492+
let appears_in_module_import_list = is_imported_type || is_imported_value;
493+
if !(appears_in_module_import_list) {
494+
self.mark_entity_as_used(unused, module, node);
495+
}
496+
}
497+
}
443498
}

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__imported_module_alias_only_referenced_by_unused_function.snap

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,6 @@ fn unused() {
2020

2121

2222
----- WARNING
23-
warning: Unused imported module
24-
┌─ /src/warning/wrn.gleam:2:1
25-
26-
2import wibble as wobble
27-
^^^^^^^^^^^^^^^^^^^^^^^ This imported module is never used
28-
29-
Hint: You can safely remove it.
30-
3123
warning: Unused private function
3224
┌─ /src/warning/wrn.gleam:4:1
3325

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__imported_module_alias_only_referenced_by_unused_function_with_unqualified.snap

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,6 @@ pub fn main() -> Wibble {
2424

2525

2626
----- WARNING
27-
warning: Unused imported module alias
28-
┌─ /src/warning/wrn.gleam:2:29
29-
30-
2import wibble.{type Wibble} as wobble
31-
^^^^^^^^^ This alias is never used
32-
33-
Hint: You can safely remove it.
34-
35-
import wibble as _
36-
37-
3827
warning: Unused private function
3928
┌─ /src/warning/wrn.gleam:4:1
4029

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__imported_module_only_referenced_by_unused_function.snap

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,6 @@ fn unused() {
2020

2121

2222
----- WARNING
23-
warning: Unused imported module
24-
┌─ /src/warning/wrn.gleam:2:1
25-
26-
2import wibble
27-
^^^^^^^^^^^^^ This imported module is never used
28-
29-
Hint: You can safely remove it.
30-
3123
warning: Unused private function
3224
┌─ /src/warning/wrn.gleam:4:1
3325
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "import wibble as woo\nfn wobble() {\n woo.a\n}\n"
4+
---
5+
----- SOURCE CODE
6+
-- wibble.gleam
7+
pub const a = 1
8+
9+
-- main.gleam
10+
import wibble as woo
11+
fn wobble() {
12+
woo.a
13+
}
14+
15+
16+
----- WARNING
17+
warning: Unused private function
18+
┌─ /src/warning/wrn.gleam:2:1
19+
20+
2 │ fn wobble() {
21+
^^^^^^^^^^^ This private function is never used
22+
23+
Hint: You can safely remove it.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "import wibble\nfn wobble() {\n wibble.a\n}\n"
4+
---
5+
----- SOURCE CODE
6+
-- wibble.gleam
7+
pub const a = 1
8+
9+
-- main.gleam
10+
import wibble
11+
fn wobble() {
12+
wibble.a
13+
}
14+
15+
16+
----- WARNING
17+
warning: Unused private function
18+
┌─ /src/warning/wrn.gleam:2:1
19+
20+
2 │ fn wobble() {
21+
^^^^^^^^^^^ This private function is never used
22+
23+
Hint: You can safely remove it.

compiler-core/src/type_/tests/warnings.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,30 @@ pub fn main() {
15161516
);
15171517
}
15181518

1519+
#[test]
1520+
fn module_used_by_unused_function_is_not_marked_as_unused() {
1521+
assert_warning!(
1522+
("wibble", "pub const a = 1"),
1523+
"import wibble
1524+
fn wobble() {
1525+
wibble.a
1526+
}
1527+
"
1528+
);
1529+
}
1530+
1531+
#[test]
1532+
fn aliased_module_used_by_unused_function_is_not_marked_as_unused() {
1533+
assert_warning!(
1534+
("wibble", "pub const a = 1"),
1535+
"import wibble as woo
1536+
fn wobble() {
1537+
woo.a
1538+
}
1539+
"
1540+
);
1541+
}
1542+
15191543
#[test]
15201544
fn calling_function_from_other_module_is_not_marked_unused() {
15211545
assert_no_warnings!(

0 commit comments

Comments
 (0)