Add cpp check

.github/workflows/go.yml

@@ -24,8 +24,8 @@ jobs:
     - name: Run go vet
       run: go vet ./...
     - name: Run go fmt
-      run: go fmt ./...   
+      run: go fmt ./...
     - name: Run go test
-      run: go test -v ./...          
+      run: go test -v ./...
     - name: Run go build
       run: go build ./...

Dockerfile

@@ -24,6 +24,9 @@ RUN apt-get update && apt-get install -y luarocks \
     && luarocks install luacheck \
     && rm -rf /var/lib/apt/lists/*
 
+# install cppcheck lint tools
+RUN apt-get update && apt-get install -y cppcheck \
+    && rm -rf /var/lib/apt/lists/*
 
 # 设置golang环境
 ENV GOLANG_DOWNLOAD_URL https://go.dev/dl/go1.21.5.linux-amd64.tar.gz

config/config.yaml

@@ -1,4 +1,4 @@
-# Reviewbot follows a zero-configuration approach in its design, where the default behavior implemented in the code should be suitable for the majority of scenarios. 
+# Reviewbot follows a zero-configuration approach in its design, where the default behavior implemented in the code should be suitable for the majority of scenarios.
 # Configuration of the file is only necessary in special cases, such as:
 # 1. Using specific commands for a particular repository.
 # 2. Modifying the default global configuration.
@@ -11,43 +11,43 @@ customConfig: # custom config for specific orgs or repos
   qbox: # github organization name
     golangci-lint:
       enable: true
-      args: ["run", "-D", "staticcheck"] # disable staticcheck globally since we have a separate linter for it 
+      args: ["run", "-D", "staticcheck"] # disable staticcheck globally since we have a separate linter for it
 
-  qbox/net-cache: 
-    luacheck: 
+  qbox/net-cache:
+    luacheck:
       enable: true
       workDir: "nginx" # only run in the nginx directory since there are .luacheckrc files in this directory
 
-  qbox/kodo: 
-    staticcheck: 
+  qbox/kodo:
+    staticcheck:
       enable: true
       workDir: "src/qiniu.com/kodo" # only run in the src/qiniu.com/kodo directory since this is a monorepo
 
-  # NOTE: The following is primarily for the gray test github_check_run feature and will be removed in the future. 
-  qbox/qhr: 
-    golangci-lint: 
+  # NOTE: The following is primarily for the gray test github_check_run feature and will be removed in the future.
+  qbox/qhr:
+    golangci-lint:
       enable: true
       githubReportType: "github_check_run"
-      args: ["run", "-D", "staticcheck"] 
-    staticcheck: 
+      args: ["run", "-D", "staticcheck"]
+    staticcheck:
       enable: true
       githubReportType: "github_check_run"
       args: ["-debug.no-compile-errors=true", "./..."]
-  qbox/net-gslb: 
-    golangci-lint: 
+  qbox/net-gslb:
+    golangci-lint:
       enable: true
       githubReportType: "github_check_run"
-      args: ["run", "-D", "staticcheck"] 
-    staticcheck: 
+      args: ["run", "-D", "staticcheck"]
+    staticcheck:
       enable: true
       githubReportType: "github_check_run"
-      args: ["-debug.no-compile-errors=true", "./..."]     
-  qbox/qtest: 
-    golangci-lint: 
+      args: ["-debug.no-compile-errors=true", "./..."]
+  qbox/qtest:
+    golangci-lint:
       enable: true
       githubReportType: "github_check_run"
-      args: ["run", "-D", "staticcheck"] 
-    staticcheck: 
+      args: ["run", "-D", "staticcheck"]
+    staticcheck:
       enable: true
       githubReportType: "github_check_run"
-      args: ["-debug.no-compile-errors=true", "./..."]          
+      args: ["-debug.no-compile-errors=true", "./..."]

internal/linters/c/cppcheck/README.md

@@ -0,0 +1,11 @@
+# Cppcheck - A tool for static C/C++ code analysis
+# cppcheck linter
+Syntax:<br />
+    cppcheck [OPTIONS] [files or paths]<br />
+
+in this repo:<br />
+Recursively check the current folder, format the error messages as file name:line number:column number: warning message and don't print progress:<br />
+cppcheck --quiet --template='{file}:{line}:{column}:  {message}' .<br />
+
+For more information:<br />
+    https://files.cppchecksolutions.com/manual.pdf<br />

internal/linters/c/cppcheck/cppcheck.go

@@ -0,0 +1,40 @@
+package cppcheck
+
+import (
+	"strings"
+
+	"github.com/qiniu/reviewbot/internal/linters"
+	"github.com/qiniu/x/xlog"
+)
+
+// refer to https://cppcheck.sourceforge.io/
+var linterName = "cppcheck"
+
+func init() {
+	linters.RegisterPullRequestHandler(linterName, cppcheckHandler)
+}
+
+func cppcheckHandler(log *xlog.Logger, a linters.Agent) error {
+
+	if linters.IsEmpty(a.LinterConfig.Args...) {
+		a.LinterConfig.Args = append([]string{}, "--quiet", "--template='{file}:{line}:{column}: {message}'", ".")
+	}
+
+	return linters.GeneralHandler(log, a, func(l *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) {
+		return linters.Parse(log, output, cppcheckParser)
+	})
+}
+
+func cppcheckParser(line string) (*linters.LinterOutput, error) {
+	lineResult, err := linters.GeneralLineParser(line)
+	if err != nil {
+		return nil, err
+
+	}
+	return &linters.LinterOutput{
+		File:    strings.TrimLeft(lineResult.File, "'"),
+		Line:    lineResult.Line,
+		Column:  lineResult.Column,
+		Message: strings.TrimRight(lineResult.Message, "'"),
+	}, nil
+}

internal/linters/c/cppcheck/cppcheck_test.go

@@ -0,0 +1,52 @@
+package cppcheck
+
+import (
+	"testing"
+
+	"github.com/qiniu/reviewbot/internal/linters"
+)
+
+func TestFormatCppCheckLine(t *testing.T) {
+	tc := []struct {
+		input    string
+		expected *linters.LinterOutput
+	}{
+		{"'cppcheck_test.c:6:7: Array 'a[10]' accessed at index 10, which is out of bounds.'", &linters.LinterOutput{
+			File:    "cppcheck_test.c",
+			Line:    6,
+			Column:  7,
+			Message: "Array 'a[10]' accessed at index 10, which is out of bounds.",
+		}},
+		{"'fdk-aac-0.1.4-libMpegTPDec-src/tpdec_asc.cpp:1198:23: Variable 'bitsAvailable' is reassigned a value before the old one has been used.'", &linters.LinterOutput{
+			File:    "fdk-aac-0.1.4-libMpegTPDec-src/tpdec_asc.cpp",
+			Line:    1198,
+			Column:  23,
+			Message: "Variable 'bitsAvailable' is reassigned a value before the old one has been used.",
+		}},
+		{"Checking test/tpdec_adts.c", nil},
+	}
+
+	for _, c := range tc {
+
+		output, err := cppcheckParser(c.input)
+
+		if output == nil {
+			if c.expected != nil {
+				t.Errorf("expected: %v, got: %v", c.expected, output)
+			}
+			continue
+		}
+
+		if c.expected == nil && output != nil {
+			t.Errorf("expected error, got: %v", output)
+		}
+
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+
+		if output.File != c.expected.File || output.Line != c.expected.Line || output.Column != c.expected.Column || output.Message != c.expected.Message {
+			t.Errorf("expected: %v, got: %v", c.expected, output)
+		}
+	}
+}

internal/linters/linters.go

@@ -139,7 +139,7 @@ func ExecRun(workDir, command string, args ...string) ([]byte, error) {
 	c := exec.Command(g, args...)
 	c.Dir = workDir
 
-	return c.Output()
+	return c.CombinedOutput()
 }
 
 // GeneralParse parses the output of a linter command.
@@ -276,6 +276,7 @@ func Parse(log *xlog.Logger, output []byte, lineParser LineParser) (map[string][
 
 // common format LinterLine
 func GeneralLineParser(line string) (*LinterOutput, error) {
+	log.Infof("line is %v", line)
 	pattern := `^(.*):(\d+):(\d+): (.*)$`
 	regex, err := regexp.Compile(pattern)
 	if err != nil {

internal/linters/linters_test.go

@@ -48,23 +48,20 @@ func TestFormatStaticcheckLine(t *testing.T) {
 
 	for _, c := range tc {
 		output, err := GeneralLineParser(c.input)
-		if c.expected == nil && output != nil {
-			t.Errorf("expected error, got: %v", output)
-		} else {
-			continue
-		}
-
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-
 		if output == nil {
 			if c.expected != nil {
 				t.Errorf("expected: %v, got: %v", c.expected, output)
 			}
 			continue
 		}
 
+		if c.expected == nil && output != nil {
+			t.Errorf("expected error, got: %v", output)
+		}
+
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
 		if output.File != c.expected.File || output.Line != c.expected.Line || output.Column != c.expected.Column || output.Message != c.expected.Message {
 			t.Errorf("expected: %v, got: %v", c.expected, output)
 		}

internal/linters/lua/luacheck/luacheck_test.go

@@ -28,25 +28,25 @@ func TestFormatLuaCheckLine(t *testing.T) {
 		expected *linters.LinterOutput
 	}{
 		{" video/mp4/libs/mp4lib.lua:184:11: value assigned to variable mem_data is overwritten on line 202 before use", &linters.LinterOutput{
-			File:    " video/mp4/libs/mp4lib.lua:184:11",
+			File:    "video/mp4/libs/mp4lib.lua",
 			Line:    184,
 			Column:  11,
 			Message: "value assigned to variable mem_data is overwritten on line 202 before use",
 		}},
 		{" utils/jsonschema.lua:723:121: line is too long (142 > 120)", &linters.LinterOutput{
-			File:    " utils/jsonschema.lua",
+			File:    "utils/jsonschema.lua",
 			Line:    723,
 			Column:  121,
 			Message: "line is too long (142 > 120)",
 		}},
 		{" utils/httpc/http_simple.lua:24:1: setting read-only global variable _VERSION", &linters.LinterOutput{
-			File:    " utils/httpc/http_simple.lua",
+			File:    "utils/httpc/http_simple.lua",
 			Line:    24,
 			Column:  1,
 			Message: "setting read-only global variable _VERSION",
 		}},
 		{" test/qtest_access.lua:1220:1: inconsistent indentation (SPACE followed by TAB)", &linters.LinterOutput{
-			File:    " test/qtest_access.lua",
+			File:    "test/qtest_access.lua",
 			Line:    1220,
 			Column:  1,
 			Message: "inconsistent indentation (SPACE followed by TAB)",
@@ -56,23 +56,21 @@ func TestFormatLuaCheckLine(t *testing.T) {
 
 	for _, c := range tc {
 		output, err := luacheckParser(c.input)
+		if output == nil {
+			if c.expected != nil {
+				t.Errorf("expected: %v, got: %v", c.expected, output)
+			}
+			continue
+		}
+
 		if c.expected == nil && output != nil {
 			t.Errorf("expected error, got: %v", output)
-		} else {
-			continue
 		}
 
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
 
-		if output == nil {
-			if c.expected != nil {
-				t.Errorf("expected: %v, got: %v", c.expected, output)
-			}
-			continue
-		}
-
 		if output.File != c.expected.File || output.Line != c.expected.Line || output.Column != c.expected.Column || output.Message != c.expected.Message {
 			t.Errorf("expected: %v, got: %v", c.expected, output)
 		}

main.go

@@ -30,6 +30,7 @@ import (
 	gitv2 "k8s.io/test-infra/prow/git/v2"
 
 	// linters import
+	_ "github.com/qiniu/reviewbot/internal/linters/c/cppcheck"
 	_ "github.com/qiniu/reviewbot/internal/linters/git-flow/commit-check"
 	_ "github.com/qiniu/reviewbot/internal/linters/go/gofmt"
 	_ "github.com/qiniu/reviewbot/internal/linters/go/golangci_lint"