Repository: mgechev/revive

Issue:
Add `macOs` runner for revive
As mentioned in https://github.com/mgechev/revive/issues/1452#issuecomment-3140540517, right now `revive` runs tests only on Windows and Linux.

Based on [github runners docs](https://docs.github.com/en/actions/reference/runners/github-hosted-runners), `macOs` runners are available for projects and `revive` could have it. In implementation, the `macos-latest` tag could be used


Discussion:
@ccoVeille: It's indeed a good idea

Solution patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 24fd788..fab004b 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -5,12 +5,16 @@ on:
     types: [opened, synchronize, reopened]
 
 jobs:
-  tests-on-unix:
-    name: Tests on Unix
-    runs-on: ubuntu-latest
+  tests:
+    name: Tests on ${{ matrix.os }} (Go ${{ matrix.go-version }})
+    runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
       matrix:
+        os:
+          - ubuntu-latest
+          - windows-latest
+          - macos-latest
         go-version:
           - oldstable
           - stable
@@ -23,16 +27,3 @@ jobs:
           go-version: ${{ matrix.go-version }}
       - name: Run tests
         run: go test -race -shuffle=on ./...
-
-  tests-on-windows:
-    name: Tests on Windows
-    runs-on: windows-latest
-    steps:
-      - name: Checkout code
-        uses: actions/checkout@v4
-      - name: Set up Go
-        uses: actions/setup-go@v5
-        with:
-          go-version: stable
-      - name: Run tests
-        run: go test -race -shuffle=on ./...
diff --git a/README.md b/README.md
index a6e6fb1..c12da86 100644
--- a/README.md
+++ b/README.md
@@ -599,6 +599,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`use-errors-new`](./RULES_DESCRIPTIONS.md#use-errors-new) | n/a   | Spots calls to `fmt.Errorf` that can be replaced by `errors.New` |   no    |  no   |
 | [`use-fmt-print`](./RULES_DESCRIPTIONS.md#use-fmt-print) | n/a   | Proposes to replace calls to built-in `print` and `println` with their equivalents from `fmt`. |   no    |  no   |
 | [`useless-break`](./RULES_DESCRIPTIONS.md#useless-break)          |  n/a   |  Warns on useless `break` statements in case clauses |    no    |  no   |
+| [`useless-fallthrough`](./RULES_DESCRIPTIONS.md#useless-fallthrough)  |  n/a   |  Warns on useless `fallthrough` statements in case clauses |    no    |  no   |
 | [`var-declaration`](./RULES_DESCRIPTIONS.md#var-declaration)     |  n/a   | Reduces redundancies around variable declaration.                |   yes    |  yes  |
 | [`var-naming`](./RULES_DESCRIPTIONS.md#var-naming)          |  allowlist & blocklist of initialisms   | Naming rules.                                                    |   yes    |  no   |
 | [`waitgroup-by-value`](./RULES_DESCRIPTIONS.md#waitgroup-by-value)  |  n/a   | Warns on functions taking sync.WaitGroup as a by-value parameter |    no    |  no   |
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 14c1be7..2463f86 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -93,6 +93,7 @@ List of all available rules.
   - [use-errors-new](#use-errors-new)
   - [use-fmt-print](#use-fmt-print)
   - [useless-break](#useless-break)
+  - [useless-fallthrough](#useless-fallthrough)
   - [var-declaration](#var-declaration)
   - [var-naming](#var-naming)
   - [waitgroup-by-value](#waitgroup-by-value)
@@ -1389,6 +1390,44 @@ The rule emits a specific warning for such cases.
 
 _Configuration_: N/A
 
+## useless-fallthrough
+
+_Description_: This rule warns on useless `fallthrough` statements in case clauses of switch statements.
+A `fallthrough` is considered _useless_ if it's the single statement of a case clause block.
+
+Go allows `switch` statements with clauses that group multiple cases.
+Thus, for example:
+
+```go
+switch category {
+  case "Lu":
+    fallthrough
+  case "Ll":
+    fallthrough    
+  case "Lt":
+    fallthrough
+  case "Lm": 
+    fallthrough
+  case "Lo":
+    return true
+  default:
+    return false
+}
+```
+
+can be written as
+
+```go
+switch category {
+  case "Lu", "Ll", "Lt", "Lm", "Lo":
+      return true
+  default:
+    return false
+}
+```
+
+_Configuration_: N/A
+
 ## var-declaration
 
 _Description_: This rule proposes simplifications of variable declarations.
diff --git a/config/config.go b/config/config.go
index 200753e..eb1585a 100644
--- a/config/config.go
+++ b/config/config.go
@@ -110,6 +110,7 @@ var allRules = append([]lint.Rule{
 	&rule.IdenticalIfElseIfConditionsRule{},
 	&rule.IdenticalIfElseIfBranchesRule{},
 	&rule.IdenticalSwitchBranchesRule{},
+	&rule.UselessFallthroughRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.
diff --git a/go.mod b/go.mod
index 575a162..3f900e0 100644
--- a/go.mod
+++ b/go.mod
@@ -10,9 +10,9 @@ require (
 	github.com/hashicorp/go-version v1.7.0
 	github.com/mgechev/dots v1.0.0
 	github.com/spf13/afero v1.14.0
-	golang.org/x/mod v0.26.0
+	golang.org/x/mod v0.27.0
 	golang.org/x/sync v0.16.0
-	golang.org/x/tools v0.35.0
+	golang.org/x/tools v0.36.0
 )
 
 require (
@@ -20,7 +20,7 @@ require (
 	github.com/mattn/go-colorable v0.1.14 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	golang.org/x/sys v0.34.0 // indirect
-	golang.org/x/text v0.27.0 // indirect
+	golang.org/x/sys v0.35.0 // indirect
+	golang.org/x/text v0.28.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
index d814f74..ee25768 100644
--- a/go.sum
+++ b/go.sum
@@ -22,17 +22,17 @@ github.com/spf13/afero v1.14.0 h1:9tH6MapGnn/j0eb0yIXiLjERO8RB6xIVZRDCX7PtqWA=
 github.com/spf13/afero v1.14.0/go.mod h1:acJQ8t0ohCGuMN3O+Pv0V0hgMxNYDlvdk+VTfyZmbYo=
 github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
 github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-golang.org/x/mod v0.26.0 h1:EGMPT//Ezu+ylkCijjPc+f4Aih7sZvaAr+O3EHBxvZg=
-golang.org/x/mod v0.26.0/go.mod h1:/j6NAhSk8iQ723BGAUyoAcn7SlD7s15Dp9Nd/SfeaFQ=
+golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ=
+golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc=
 golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw=
 golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA=
-golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
-golang.org/x/text v0.27.0 h1:4fGWRpyh641NLlecmyl4LOe6yDdfaYNrGb2zdfo4JV4=
-golang.org/x/text v0.27.0/go.mod h1:1D28KMCvyooCX9hBiosv5Tz/+YLxj0j7XhWjpSUF7CU=
-golang.org/x/tools v0.35.0 h1:mBffYraMEf7aa0sB+NuKnuCy8qI/9Bughn8dC2Gu5r0=
-golang.org/x/tools v0.35.0/go.mod h1:NKdj5HkL/73byiZSJjqJgKn3ep7KjFkBOkR/Hps3VPw=
+golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI=
+golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
+golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng=
+golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU=
+golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg=
+golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/rule/useless_fallthrough.go b/rule/useless_fallthrough.go
new file mode 100644
index 0000000..2238767
--- /dev/null
+++ b/rule/useless_fallthrough.go
@@ -0,0 +1,91 @@
+package rule
+
+import (
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// UselessFallthroughRule warns on useless fallthroughs in switch case clauses.
+type UselessFallthroughRule struct{}
+
+// Apply applies the rule to given file.
+func (*UselessFallthroughRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	commentsMap := file.CommentMap()
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintUselessFallthrough{onFailure: onFailure, commentsMap: commentsMap}
+	for _, decl := range file.AST.Decls {
+		fn, ok := decl.(*ast.FuncDecl)
+		if !ok || fn.Body == nil {
+			continue
+		}
+
+		ast.Walk(w, fn.Body)
+	}
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*UselessFallthroughRule) Name() string {
+	return "useless-fallthrough"
+}
+
+type lintUselessFallthrough struct {
+	onFailure   func(lint.Failure)
+	commentsMap ast.CommentMap
+}
+
+func (w *lintUselessFallthrough) Visit(node ast.Node) ast.Visitor {
+	switchStmt, ok := node.(*ast.SwitchStmt)
+	if !ok { // not a switch statement, keep walking the AST
+		return w
+	}
+
+	if switchStmt.Tag == nil {
+		return w // Not interested in un-tagged switches
+	}
+
+	casesCount := len(switchStmt.Body.List)
+	for i := range casesCount - 1 {
+		caseClause := switchStmt.Body.List[i].(*ast.CaseClause)
+		caseBody := caseClause.Body
+
+		if len(caseBody) != 1 {
+			continue // skip if body is not exactly one statement
+		}
+
+		branchStmt, ok := caseBody[0].(*ast.BranchStmt)
+		if !ok || branchStmt.Tok != token.FALLTHROUGH {
+			continue // not a fallthrough
+		}
+
+		confidence := 1.0
+		if nextCaseClause := switchStmt.Body.List[i+1].(*ast.CaseClause); nextCaseClause.List == nil {
+			// the next case clause is the default clause, report with lower confidence.
+			confidence = 0.8
+		}
+		if _, ok := w.commentsMap[branchStmt]; ok {
+			// The fallthrough has a comment, report with lower confidence.
+			confidence = 0.5
+		}
+
+		w.onFailure(lint.Failure{
+			Confidence: confidence,
+			Node:       branchStmt,
+			Category:   lint.FailureCategoryCodeStyle,
+			Failure:    `this "fallthrough" can be removed by consolidating this case clause with the next one`,
+		})
+
+		ast.Walk(w, caseClause)
+	}
+
+	return nil
+}



Test patch:
diff --git a/test/useless_fallthrough_test.go b/test/useless_fallthrough_test.go
new file mode 100644
index 0000000..0496c81
--- /dev/null
+++ b/test/useless_fallthrough_test.go
@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUselessFallTrhough(t *testing.T) {
+	testRule(t, "useless_fallthrough", &rule.UselessFallthroughRule{})
+}
diff --git a/testdata/useless_fallthrough.go b/testdata/useless_fallthrough.go
new file mode 100644
index 0000000..7f2b9cd
--- /dev/null
+++ b/testdata/useless_fallthrough.go
@@ -0,0 +1,79 @@
+package fixtures
+
+func uselessFallthrough() {
+
+	switch a {
+	case 0:
+		println()
+		fallthrough
+	default:
+	}
+
+	switch a {
+	case 0:
+		fallthrough // MATCH /this "fallthrough" can be removed by consolidating this case clause with the next one/
+	case 1:
+		println()
+	default:
+	}
+
+	switch a {
+	case 0:
+		fallthrough // MATCH /this "fallthrough" can be removed by consolidating this case clause with the next one/
+	case 1:
+		fallthrough // MATCH /this "fallthrough" can be removed by consolidating this case clause with the next one/
+	case 2:
+		println()
+	default:
+	}
+
+	switch a {
+	case 0:
+		fallthrough // json:{"MATCH": "this \"fallthrough\" can be removed by consolidating this case clause with the next one","Confidence": 0.8}
+	default:
+		println()
+	}
+
+	switch a {
+	case 0:
+		fallthrough // json:{"MATCH": "this \"fallthrough\" can be removed by consolidating this case clause with the next one","Confidence": 0.8}
+	default:
+		println()
+	case 1:
+		fallthrough // MATCH /this "fallthrough" can be removed by consolidating this case clause with the next one/
+	case 2:
+		println()
+	}
+
+	switch a {
+	case 0:
+		// This a comment on the case, the confidence must be 0.5
+		fallthrough // json:{"MATCH": "this \"fallthrough\" can be removed by consolidating this case clause with the next one","Confidence": 0.5}
+	default:
+		println()
+	}
+
+	switch goos {
+	case "linux":
+		// TODO(bradfitz): be fancy and use linkat with AT_EMPTY_PATH to avoid
+		// copying? I couldn't get it to work, though.
+		// For now, just do the same thing as every other Unix and copy
+		// the binary.
+		fallthrough // json:{"MATCH": "this \"fallthrough\" can be removed by consolidating this case clause with the next one","Confidence": 0.5}
+	case "darwin", "freebsd", "openbsd", "netbsd":
+		return
+	case "windows":
+		return
+	default:
+		return
+	}
+
+	switch a {
+	case 0:
+		//foo:bar
+		fallthrough // json:{"MATCH": "this \"fallthrough\" can be removed by consolidating this case clause with the next one","Confidence": 0.5}
+	default:
+		println()
+	}
+
+}