Analyzer incorrectly detects comment directives on nested switch statements

[FastNav]

Problem

The analyzer seems to associate all comments within a switch statement with the switch statement. This seems to follow naturally from the definition of ast.CommentMap.Filter, which runs ast.Inspect on the specified node and grabs all comment groups from that recursive walk.

Reproduction

Apply the following patch to the ignore comment test case file.

diff --git testdata/src/ignore-comment/ignore_comment.go testdata/src/ignore-comment/ignore_comment.go
index a6c6e5c..6c2b65c 100644
--- testdata/src/ignore-comment/ignore_comment.go
+++ testdata/src/ignore-comment/ignore_comment.go
@@ -61,3 +61,22 @@ func _nested() {
                }
        }
 }
+
+func _reverse_nested() {
+       var d Direction
+
+       // this should report.
+       switch d { // want "^missing cases in switch of type Direction: E, directionInvalid$"
+       case N:
+       case S:
+       case W:
+       default:
+               // this should not report.
+               switch d { //exhaustive:ignore
+               case N:
+               case S:
+               case W:
+               default:
+               }
+       }
+}

Tests will now fail because the ignore directive from the inner switch statement propagates to the outer switch statement.

Resolution

I'm not able to find anything in the AST package that satisfies the use case exactly. Using a direct CommentMap[sw] drops inline comments placed right after the switch token, since that comment is associated with the first case clause according to the AST. The easiest solution seems to be to walk from the switchStmt to the first caseStmt and collect all comments associated with both of them before ending the walk.

A rare edge case here: the switch variable could be an anonymous function invocation that itself contains a comment directive. The walk should therefore ignore any nodes deeper than the case statements of the top level switch.

[nishanths]

Thanks for the issue.

Using a direct CommentMap[sw] drops inline comments placed right after the switch token, since that comment is associated with the first case clause according to the AST.

I think we should make the change to use CommentMap[sw] directly, and not use the recursive ast.CommentMap.Filter(sw). In the long-term, it is better for the exhaustive program to align with go/ast's definition of associated comments for a node. That is, the exhaustive program shouldn't associate comments with a switch statement node that go/ast in fact associates with the first case clause.

If I understand correctly, doing so would be a breaking change such that:

switch d { //exhaustive:ignore

this switch statement would no longer be ignored by the exhaustive program. We can adjust the tests accordingly, and mention this in the release notes. The suggested fix would be move the comment, like so:

//exhaustive:ignore
switch d { 

A rare edge case here: the switch variable could be an anonymous function invocation that itself contains a comment directive. The walk should therefore ignore any nodes deeper than the case statements of the top level switch.

I think making the above change to only considering CommentMap[sw] might address this naturally too.

Tests will now fail because the ignore directive from the inner switch statement propagates to the outer switch statement.

With the above change, this test case in the issue description should now pass too, I think.