noGuardInTests rule removes array bounds checks

[nicklockwood]

@calda I was testing out the new noGuardInTests rule in one of my projects. It works great for cases like this:

guard let intersection = plane1.intersection(with: plane2) else {
    XCTFail()
    return
}

XCTAssertEqual(intersection, Line(origin: [-1, 0, 1], direction: -.unitY))

However I had to disable it because it was also removing the guard in cases like this:

guard polygons.count > 1 else {
    return
}

let a = Set(polygons[0].vertices.map(\.position))

I wonder if there's a way to make it configurable to only apply in the former and not the latter cases?

It's probably too complex to try to examine the condition itself for bounds checks, but perhaps we could have an option to only remove guard let constructs and not other types of guard?

[calda]

Yeah, this makes sense -- to be 100% safe the rule would need to exclude boolean guard cases. An option for this is a good idea.

In our codebase I only hit this issue two or three times, so decided to leave it in.

In your test case, is polygons.count dynamic (sometimes empty, sometimes not empty), or static? If polygons.count is static / hardcoded to be zero for the test case then the code after the guard would be dead code and could just be removed. I'm guessing that it's probably dynamic, but I did see some examples of dead code like that in our test suite (a guard statement that was guaranteed to always fail).

[nicklockwood]

Yeah, it's dynamic. Of course in theory a sufficiently advanced rule could eliminate dead code, but that kind of static analysis seems way out of scope