Go-tools: megacheck: false positive SA4011?

Created on 10 Jan 2018  路  3Comments  路  Source: dominikh/go-tools

Consider this code: https://play.golang.org/p/0qPDM7V03az

megacheck reports 11:4: ineffective break statement. Did you mean to break out of the outer loop? (SA4011)

even though the intention is to break the select case.

Megacheck stops whining if you create a new scope around select statement: https://play.golang.org/p/Ia4qEsrXjes

Am I missing something?

Most helpful comment

As a recommendation, if you'd really like to emphasize that the select case is empty, consider using a // Do nothing. comment. See https://play.golang.org/p/lB3Tos7DOhq. It makes it clear case is empty without making people suspect you might've meant to break out of the outer for loop.

All 3 comments

I don't think you need a break here, since Go is not C. If you just leave this case empty, it will be both shorter and, IMO, more understandable. break in this case is just confusing for both people and machines.

What Ainar said. The break statement there isn't wrong and doing what you intended, but it is also unnecessary. Worse, most people who use that sort of break use it incorrectly, thinking it breaks out of the outer for loop, which it does not.

As a recommendation, if you'd really like to emphasize that the select case is empty, consider using a // Do nothing. comment. See https://play.golang.org/p/lB3Tos7DOhq. It makes it clear case is empty without making people suspect you might've meant to break out of the outer for loop.

Was this page helpful?
0 / 5 - 0 ratings