Restore UNNCESSARY DEFINITION errors for top-level defs

Non-top-level defs are already covered
roc-lang · Oct 30, 2024 · 2b8d9aa · 2b8d9aa
1 parent 949fdcf
commit 2b8d9aa
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 1 deletion.
diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs
@@ -1267,6 +1267,14 @@ fn canonicalize_value_defs<'a>(
 
         output = temp_output.output;
 
+        if let (PatternType::TopLevelDef, DefKind::Ignored(_)) =
+            (pattern_type, temp_output.def.kind)
+        {
+            env.problems.push(Problem::NoIdentifiersIntroduced(
+                temp_output.def.loc_pattern.region,
+            ))
+        }
+
         defs.push(Some(temp_output.def));
 
         def_ordering.insert_symbol_references(def_id as u32, &temp_output.references)

diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs
@@ -10101,6 +10101,18 @@ All branches in an `if` must have the same type!
 
     Since these variables have the same name, it's easy to use the wrong
     one by accident. Give one of them a new name.
+
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This destructure assignment doesn't introduce any new variables:
+
+    5│  main = \n -> n + 2
+        ^^^^
+
+    If you don't need to use the value on the right-hand side of this
+    assignment, consider removing the assignment. Since effects are not
+    allowed at the top-level, assignments that don't introduce variables
+    cannot affect a program's behavior
     "###
     );
 
@@ -10806,7 +10818,55 @@ All branches in an `if` must have the same type!
             Foo = Foo
             "#
         ),
-        @""
+        @r###"
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This destructure assignment doesn't introduce any new variables:
+
+    3│  Pair _ _ = Pair 0 1
+        ^^^^^^^^
+
+    If you don't need to use the value on the right-hand side of this
+    assignment, consider removing the assignment. Since effects are not
+    allowed at the top-level, assignments that don't introduce variables
+    cannot affect a program's behavior
+
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This destructure assignment doesn't introduce any new variables:
+
+    5│  _ = Pair 0 1
+        ^
+
+    If you don't need to use the value on the right-hand side of this
+    assignment, consider removing the assignment. Since effects are not
+    allowed at the top-level, assignments that don't introduce variables
+    cannot affect a program's behavior
+
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This destructure assignment doesn't introduce any new variables:
+
+    7│  {} = {}
+        ^^
+
+    If you don't need to use the value on the right-hand side of this
+    assignment, consider removing the assignment. Since effects are not
+    allowed at the top-level, assignments that don't introduce variables
+    cannot affect a program's behavior
+
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This destructure assignment doesn't introduce any new variables:
+
+    9│  Foo = Foo
+        ^^^
+
+    If you don't need to use the value on the right-hand side of this
+    assignment, consider removing the assignment. Since effects are not
+    allowed at the top-level, assignments that don't introduce variables
+    cannot affect a program's behavior
+    "###
     );
 
     test_report!(

diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs
@@ -195,6 +195,7 @@ pub enum Problem {
         unbound_symbol: Symbol,
         region: Region,
     },
+    NoIdentifiersIntroduced(Region),
     OverloadedSpecialization {
         overload: Region,
         original_opaque: Symbol,
@@ -317,6 +318,7 @@ impl Problem {
             Problem::ImplementsNonRequired { .. } => Warning,
             Problem::DoesNotImplementAbility { .. } => RuntimeError,
             Problem::NotBoundInAllPatterns { .. } => RuntimeError,
+            Problem::NoIdentifiersIntroduced(_) => Warning,
             Problem::OverloadedSpecialization { .. } => Warning, // Ideally, will compile
             Problem::UnnecessaryOutputWildcard { .. } => Warning,
             // TODO: sometimes this can just be a warning, e.g. if you have [1, .., .., 2] but we
@@ -480,6 +482,7 @@ impl Problem {
             }
             | Problem::NotAnAbility(region)
             | Problem::ImplementsNonRequired { region, .. }
+            | Problem::NoIdentifiersIntroduced(region)
             | Problem::DoesNotImplementAbility { region, .. }
             | Problem::OverloadedSpecialization {
                 overload: region, ..

diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs
@@ -1189,6 +1189,14 @@ pub fn can_problem<'b>(
             ]);
             title = "NAME NOT BOUND IN ALL PATTERNS".to_string();
         }
+        Problem::NoIdentifiersIntroduced(region) => {
+            doc = alloc.stack([
+                alloc.reflow("This destructure assignment doesn't introduce any new variables:"),
+                alloc.region(lines.convert_region(region), severity),
+                alloc.reflow("If you don't need to use the value on the right-hand side of this assignment, consider removing the assignment. Since effects are not allowed at the top-level, assignments that don't introduce variables cannot affect a program's behavior"),
+            ]);
+            title = "UNNECESSARY DEFINITION".to_string();
+        }
         Problem::OverloadedSpecialization {
             ability_member,
             overload,