Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 101 additions & 2 deletions src/canonicalize/Mod.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6228,6 +6228,13 @@ fn checkUsedUnderscoreVariable(
}

fn checkScopeForUnusedVariables(self: *Self, scope: *const Scope) std.mem.Allocator.Error!void {
// Define the type for unused variables
const UnusedVar = struct { ident: base.Ident.Idx, region: Region };

// Collect all unused variables first so we can sort them
var unused_vars = std.ArrayList(UnusedVar).init(self.env.gpa);
defer unused_vars.deinit();

// Iterate through all identifiers in this scope
var iterator = scope.idents.iterator();
while (iterator.next()) |entry| {
Expand All @@ -6247,10 +6254,26 @@ fn checkScopeForUnusedVariables(self: *Self, scope: *const Scope) std.mem.Alloca
// Get the region for this pattern to provide good error location
const region = self.env.store.getPatternRegion(pattern_idx);

// Report unused variable
try self.env.pushDiagnostic(Diagnostic{ .unused_variable = .{
// Collect unused variable for sorting
try unused_vars.append(.{
.ident = ident_idx,
.region = region,
});
}

// Sort unused variables by region (earlier in file first)
std.mem.sort(UnusedVar, unused_vars.items, {}, struct {
fn lessThan(_: void, a: UnusedVar, b: UnusedVar) bool {
// Compare by start offset (position in file)
return a.region.start.offset < b.region.start.offset;
}
}.lessThan);

// Report unused variables in sorted order
for (unused_vars.items) |unused| {
try self.env.pushDiagnostic(Diagnostic{ .unused_variable = .{
.ident = unused.ident,
.region = unused.region,
} });
}
}
Expand Down Expand Up @@ -8679,3 +8702,79 @@ test "hex literal parsing logic integration" {
try std.testing.expectEqual(tc.expected_value, u128_val);
}
}

test "unused variables are sorted by region" {
const gpa = std.testing.allocator;

// Create a test program with unused variables in non-alphabetical order
const source =
\\app [main!] { pf: platform "../basic-cli/main.roc" }
\\
\\func = |_| {
\\ zebra = 5 # Line 3 - should be reported first
\\ apple = 10 # Line 4 - should be reported second
\\ monkey = 15 # Line 5 - should be reported third
\\ used = 20 # Line 6 - this one is used
\\ used
\\}
\\
\\main! = |_| func({})
;

var ctx = try ScopeTestContext.init(gpa);
defer ctx.deinit();

// Parse the source
const ast = try parse.AST.parseFromStr(gpa, source, "test.roc", &ctx.module_env.string_interner);
defer ast.deinit();

// Canonicalize the AST
const parsed_module = ast.parsed_module;
var self = try Self.initFromAST(parsed_module, &ctx.module_env, source);
try self.canonicalizeModule();
defer self.deinit();

// Check that we have unused variable diagnostics
var unused_var_diagnostics = std.ArrayList(struct {
ident: base.Ident.Idx,
region: Region,
}).init(gpa);
defer unused_var_diagnostics.deinit();

// Collect all unused variable diagnostics
for (ctx.module_env.diagnostics.items) |diagnostic| {
switch (diagnostic) {
.unused_variable => |data| {
try unused_var_diagnostics.append(.{
.ident = data.ident,
.region = data.region,
});
},
else => continue,
}
}

// We should have exactly 3 unused variables (zebra, apple, monkey)
try std.testing.expectEqual(@as(usize, 3), unused_var_diagnostics.items.len);

// Check that they are sorted by region (line number)
// The source positions should be in increasing order
var prev_offset: u32 = 0;
for (unused_var_diagnostics.items) |diagnostic| {
const current_offset = diagnostic.region.start.offset;

// Each unused variable should appear after the previous one in the source
try std.testing.expect(current_offset > prev_offset);
prev_offset = current_offset;

// Also verify the names are in the expected order (zebra, apple, monkey)
const ident_text = ctx.module_env.idents.getText(diagnostic.ident);
if (unused_var_diagnostics.items[0].ident.idx == diagnostic.ident.idx) {
try std.testing.expectEqualStrings("zebra", ident_text);
} else if (unused_var_diagnostics.items[1].ident.idx == diagnostic.ident.idx) {
try std.testing.expectEqualStrings("apple", ident_text);
} else if (unused_var_diagnostics.items[2].ident.idx == diagnostic.ident.idx) {
try std.testing.expectEqualStrings("monkey", ident_text);
}
}
}
26 changes: 13 additions & 13 deletions src/snapshots/crash_and_ellipsis_test.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 23 additions & 23 deletions src/snapshots/expr/tuple_comprehensive.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ type=expr
~~~
# EXPECTED
EMPTY TUPLE NOT ALLOWED - tuple_comprehensive.md:9:10:9:12
UNUSED VARIABLE - tuple_comprehensive.md:10:2:10:8
UNUSED VARIABLE - tuple_comprehensive.md:11:2:11:6
UNUSED VARIABLE - tuple_comprehensive.md:12:2:12:8
UNUSED VARIABLE - tuple_comprehensive.md:10:2:10:8
UNUSED VARIABLE - tuple_comprehensive.md:13:2:13:8
UNUSED VARIABLE - tuple_comprehensive.md:14:2:14:7
UNUSED VARIABLE - tuple_comprehensive.md:15:2:15:11
UNUSED VARIABLE - tuple_comprehensive.md:16:2:16:13
UNUSED VARIABLE - tuple_comprehensive.md:14:2:14:7
# PROBLEMS
**EMPTY TUPLE NOT ALLOWED**
I am part way through parsing this tuple, but it is empty:
Expand All @@ -45,6 +45,18 @@ I am part way through parsing this tuple, but it is empty:

If you want to represent nothing, try using an empty record: `{}`.

**UNUSED VARIABLE**
Variable `single` is not used anywhere in your code.

If you don't need this variable, prefix it with an underscore like `_single` to suppress this warning.
The unused variable is declared here:
**tuple_comprehensive.md:10:2:10:8:**
```roc
single = (42)
```
^^^^^^


**UNUSED VARIABLE**
Variable `pair` is not used anywhere in your code.

Expand All @@ -70,27 +82,27 @@ The unused variable is declared here:


**UNUSED VARIABLE**
Variable `single` is not used anywhere in your code.
Variable `nested` is not used anywhere in your code.

If you don't need this variable, prefix it with an underscore like `_single` to suppress this warning.
If you don't need this variable, prefix it with an underscore like `_nested` to suppress this warning.
The unused variable is declared here:
**tuple_comprehensive.md:10:2:10:8:**
**tuple_comprehensive.md:13:2:13:8:**
```roc
single = (42)
nested = ((1, 2), (3, 4))
```
^^^^^^


**UNUSED VARIABLE**
Variable `nested` is not used anywhere in your code.
Variable `mixed` is not used anywhere in your code.

If you don't need this variable, prefix it with an underscore like `_nested` to suppress this warning.
If you don't need this variable, prefix it with an underscore like `_mixed` to suppress this warning.
The unused variable is declared here:
**tuple_comprehensive.md:13:2:13:8:**
**tuple_comprehensive.md:14:2:14:7:**
```roc
nested = ((1, 2), (3, 4))
mixed = (add_one(5), "world", [1, 2, 3])
```
^^^^^^
^^^^^


**UNUSED VARIABLE**
Expand All @@ -117,18 +129,6 @@ The unused variable is declared here:
^^^^^^^^^^^


**UNUSED VARIABLE**
Variable `mixed` is not used anywhere in your code.

If you don't need this variable, prefix it with an underscore like `_mixed` to suppress this warning.
The unused variable is declared here:
**tuple_comprehensive.md:14:2:14:7:**
```roc
mixed = (add_one(5), "world", [1, 2, 3])
```
^^^^^


# TOKENS
~~~zig
OpenCurly(1:1-1:2),
Expand Down
Loading
Loading