Rule Idea Board

[DonIsaac]

This is a mega-issue tracking the list of rules we want to implement. Rule requests are welcome.

1. no-undefined - ban undefined in assignments and initializations

   • Allow when a // SAFETY: comment is present
   • Allow self.* = undefined in deinit() functions

2. unsafe-optional-unwrap: bad optional unwrapping without a prior null check

3. use-after-deinit: ban references to foo after foo.deinit() calls (or other free-like calls)

4. no-const-reassign: do not reassign const-declared variables. Yes the compiler checks this, no they don't check it for tree-shaken code.

5. no-private-access: ban external references to _private container fields.

[DonIsaac]

I don't know what to call this, but @paperdave proposed it here

[DonIsaac]

Here's his full list (it needs triage)

[paperdave]

I don't know what to call this

"declaration can be accessed with a shorter path" / shorter_decl_path

[DonIsaac]

@paperdave decl-alias?

[DonIsaac]

unused-array-side-effects: Accidental mutation on a copied array

fn addEntry(self: *Foo, id: u32, item: u32) !void {
  // this creates a copy. It's dropped when the function returns
  var foos = self.foo[id]; // should be &self.foo[id];
  try foos.append(item);   // has no lasting side effects b/c foos is dropped
}

[MichaelBelousov]

unsafe-union-unwrap/check-union-payload-refs:

like unsafe-option-unwrap but enforces that a direct union variant access occurs in a scope where the union tag was checked and matches.

var a: union(enum) { int: i32, float: f32 } = .X;

if (a == .float) {
  std.debug.print("float: {}", .{a.float}); // legal
} else {
  std.debug.print("int: {}", .{a.float}); // illegal!
}

switch (a) {
  .int => std.debug.print("int: {}", .{a.float}), // illegal
}

[paperdave]

naming-convention: enforce standard library naming conventions declarations

[paperdave]

but enforces that a direct union variant access occurs in a scope where the union tag was checked and matches.

it's better to avoid this pattern entirely, if possible. these rules should encourage switch and captures:

var a: union(enum) { int: i32, float: f32 } = .X;

if (a == .float) {
  std.debug.print("float: {}", .{a.float}); // illegal
} else {
  std.debug.print("int: {}", .{a.float}); // illegal
}

// rewrite into
switch (a) {
  .float => |float| std.debug.print("float: {}", .{a.float}),
  .int => |int| std.debug.print("int: {}", .{a.int}),
}

[MichaelBelousov]

it's better to avoid this pattern entirely, if possible. these rules should encourage switch and captures

I agree in general, but it's the same reason to have unsafe-option-unwrap even when if (optional) |capture| exists.

There is code when you're checking multiple conditions and only one union variant where a switch may be cumbersome:

if (condition and a == .float) {
  std.debug.print("{}", .{a.float});
}

// versus

if (condition) switch (a) {
  .float => |v| {
    std.debug.print("{}", .{v});
  },
  else => {},
}

probably both unsafe-*-unwrap rules should mention the capture equivalents since those will also fix the rule (as opposed to making sure you access in a scope that checks the tag)

[DonIsaac]

but enforces that a direct union variant access occurs in a scope where the union tag was checked and matches.

it's better to avoid this pattern entirely, if possible. these rules should encourage switch and captures:

var a: union(enum) { int: i32, float: f32 } = .X;

if (a == .float) {

  std.debug.print("float: {}", .{a.float}); // illegal

} else {

  std.debug.print("int: {}", .{a.float}); // illegal

}

// rewrite into

switch (a) {

  .float => |float| std.debug.print("float: {}", .{a.float}),

  .int => |int| std.debug.print("int: {}", .{a.int}),

}

Should we add a rule banning it entirely?

[MichaelBelousov]

Should we add a rule banning it entirely?

Sure but I'd recommend allowing it with checks as the default.

It comes up often enough imo. A better example that I should have shown earlier is try checking that two separate variables have specific tags. You'd need a nested double switch if you don't allow tags checking in a single if statement

try rewriting this as a switch:

if (a == .float and b == .int) {
}

[paperdave]

i think we should split these into separate issues. this thread is getting very long. one issue per lint rule, or for related ones they can share one.

[nektro]

https://github.com/nektro/ziglint/issues has a bunch i havent implemented