feat: Remove Go Typecheck Tools Implement GitHub Actions Based Typecheck for OpenIM (#2140)

* feat: remove go typecheck tools

* feat: add actions go  typecheck tools

* Update verify-typecheck.sh
pull/2136/head v3.6.1-beta.0
Xinwei Xiong 9 months ago committed by GitHub
parent f6ab243d2f
commit 71f62080f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -70,6 +70,9 @@ jobs:
version: '3.x' # If available, use the latest major version that's compatible
repo-token: ${{ secrets.GITHUB_TOKEN }}
- name: Code Typecheck Detector
uses: kubecub/typecheck@main
- name: Module Operations
run: |
sudo make tidy

@ -2,7 +2,6 @@ go 1.19
use (
.
./test/typecheck
./tools/changelog
./tools/component
./tools/formitychecker

@ -237,6 +237,17 @@ install.richgo:
install.rts:
@$(GO) install github.com/galeone/rts/cmd/rts@latest
# ================= kubecub openim tools =========================================
## install.typecheck: install kubecub typecheck check for go code
.PHONY: install.typecheck
install.typecheck:
@$(GO) install github.com/kubecub/typecheck@latest
## install.comment-lang-detector: install kubecub comment-lang-detector check for go code comment language
.PHONY: install.comment-lang-detector
install.comment-lang-detector:
@$(GO) install github.com/kubecub/comment-lang-detector/cmd/cld@latest
## tools.help: Display help information about the tools package
.PHONY: tools.help
tools.help: scripts/make-rules/tools.mk

@ -20,10 +20,6 @@
# the project.
# Usage: `scripts/run-in-gopath.sh <command>`.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"

@ -18,10 +18,6 @@
# immediately before exporting docs. We do not want to check these documents in
# by default.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"
@ -33,7 +29,7 @@ BINS=(
genman
genyaml
)
make -C "${OPENIM_ROOT}" WHAT="${BINS[*]}"
make -C "${OPENIM_ROOT}" BINS="${BINS[*]}"
openim::util::ensure-temp-dir

@ -13,11 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"

@ -17,10 +17,6 @@
# This script lints each shell script by `shellcheck`.
# Usage: `scripts/verify-shellcheck.sh`.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"

@ -17,10 +17,6 @@
# working directory by client9/misspell package.
# Usage: `scripts/verify-spelling.sh`.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
export OPENIM_ROOT
source "${OPENIM_ROOT}/scripts/lib/init.sh"

@ -16,26 +16,19 @@
# This script does a fast type check of script srnetes code for all platforms.
# Usage: `scripts/verify-typecheck.sh`.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"
openim::golang::verify_go_version
cd "${OPENIM_ROOT}"
# As of June, 2020 the typecheck tool is written in terms of go/packages, but
# that library doesn't work well with multiple modules. Until that is done,
# force this tooling to run in a fake GOPATH.
ret=0
TYPECHECK_SERIAL="${TYPECHECK_SERIAL:-false}"
scripts/run-in-gopath.sh \
go run test/typecheck/typecheck.go "$@" "--serial=$TYPECHECK_SERIAL" || ret=$?
make tools.verify.typecheck
${OPENIM_ROOT}/_output/tools/typecheck "$@" "--serial=$TYPECHECK_SERIAL" || ret=$?
if [[ $ret -ne 0 ]]; then
openim::log::error "Type Check has failed. This may cause cross platform build failures." >&2
openim::log::error "Please see https://github.com/openimsdk/open-im-server/tree/main/test/typecheck for more information." >&2
openim::log::error "Please see https://github.com/kubecub/typecheck for more information." >&2
exit 1
fi

@ -19,10 +19,6 @@
#
# Usage: `scripts/verify-yamlfmt.sh`.
OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${OPENIM_ROOT}/scripts/lib/init.sh"

@ -1,52 +0,0 @@
# OpenIM Typecheck: Cross-Platform Source Code Type Checking for Go
## Introduction
OpenIM Typecheck is a robust tool designed for cross-platform source code type checking across all Go build platforms. This utility leverages Gos built-in parsing and type-check libraries (`go/parser` and `go/types`) to deliver efficient and reliable code analysis.
## Advantages
- **Speed**: A complete compilation with OpenIM can take approximately 3 minutes. In contrast, OpenIM Typecheck achieves this in mere seconds, significantly enhancing productivity.
- **Resource Efficiency**: Unlike the typical requirement of over 40GB of RAM for standard processes, Typecheck operates effectively with less than 8GB of RAM. This reduction in resource consumption makes it highly suitable for a variety of systems, reducing overheads and facilitating smoother operations.
## Implementation
OpenIM Typecheck employs Go's native parsing and type-checking libraries (`go/parser` and `go/types`). However, it's important to note that these libraries aren't identical to those used by the Go compiler. While occasional mismatches may occur, these libraries generally provide close approximations to the compiler's functionality, offering a reliable basis for type checking.
## Error Handling
Typecheck's approach to error handling is pragmatic, focusing on practicality and build continuity.
**Errors reported by `go/types` but not by `go build`**:
- **Actual Errors** (as per the specification):
- These should ideally be rectified. If rectification is not feasible, such as in cases of ongoing work or external dependencies in the code, these errors can be overlooked.
- Example: Unused variables within a closure.
- **False Positives**:
- These errors should be ignored and, where appropriate, reported upstream for resolution.
- Example: Type mismatches between staging and generated types.
**Errors reported by `go build` but not by us**:
- CGo-related errors, including both syntax and linker issues, are outside our scope.
## Usage
### Locally
To run Typecheck locally, simply use the following command:
```bash
make verify
```
### Continuous Integration (CI)
In CI environments, Typecheck can be integrated into the workflow as follows:
```yaml
- name: Typecheck
run: make verify
```
This streamlined process facilitates efficient error detection and resolution, ensuring a robust and reliable build pipeline.
More to learn about typecheck [share blog](https://nsddd.top/posts/concurrent-type-checking-and-cross-platform-development-in-go/)

@ -1,10 +0,0 @@
module github.com/openimsdk/open-im-server/test/typecheck
go 1.19
require golang.org/x/tools v0.12.0
require (
golang.org/x/mod v0.12.0 // indirect
golang.org/x/sys v0.11.0 // indirect
)

@ -1,7 +0,0 @@
golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc=
golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/tools v0.12.0 h1:YW6HUoUmYBpwSgyaGaZq1fHjrBjX1rlpZ54T6mu2kss=
golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM=

@ -1,319 +0,0 @@
// Copyright © 2023 OpenIM. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// do a fast type check of openim code, for all platforms.
package main
import (
"flag"
"fmt"
"io"
"log"
"os"
"path/filepath"
"sort"
"strings"
"sync"
"time"
"golang.org/x/tools/go/packages"
)
var (
verbose = flag.Bool("verbose", false, "print more information")
cross = flag.Bool("cross", true, "build for all platforms")
platforms = flag.String("platform", "", "comma-separated list of platforms to typecheck")
timings = flag.Bool("time", false, "output times taken for each phase")
defuses = flag.Bool("defuse", false, "output defs/uses")
serial = flag.Bool("serial", false, "don't type check platforms in parallel (equivalent to --parallel=1)")
parallel = flag.Int("parallel", 2, "limits how many platforms can be checked in parallel. 0 means no limit.")
skipTest = flag.Bool("skip-test", false, "don't type check test code")
tags = flag.String("tags", "", "comma-separated list of build tags to apply in addition to go's defaults")
ignoreDirs = flag.String("ignore-dirs", "", "comma-separated list of directories to ignore in addition to the default hardcoded list including staging, vendor, and hidden dirs")
// When processed in order, windows and darwin are early to make
// interesting OS-based errors happen earlier.
crossPlatforms = []string{
"linux/amd64", "windows/386",
"darwin/amd64", "darwin/arm64",
"linux/386", "linux/arm",
"windows/amd64", "linux/arm64",
"linux/ppc64le", "linux/s390x",
"windows/arm64",
}
// directories we always ignore
standardIgnoreDirs = []string{
// Staging code is symlinked from vendor/k8s.io, and uses import
// paths as if it were inside of vendor/. It fails typechecking
// inside of staging/, but works when typechecked as part of vendor/.
"staging",
"components",
"logs",
// OS-specific vendor code tends to be imported by OS-specific
// packages. We recursively typecheck imported vendored packages for
// each OS, but don't typecheck everything for every OS.
"vendor",
"test",
"_output",
"*/mw/rpc_server_interceptor.go",
// Tools we use for maintaining the code base but not necessarily
// ship as part of the release
"sopenim::golang::setup_env:tools/yamlfmt/yamlfmt.go:tools",
}
)
func newConfig(platform string) *packages.Config {
platSplit := strings.Split(platform, "/")
goos, goarch := platSplit[0], platSplit[1]
mode := packages.NeedName | packages.NeedFiles | packages.NeedTypes | packages.NeedSyntax | packages.NeedDeps | packages.NeedImports | packages.NeedModule
if *defuses {
mode = mode | packages.NeedTypesInfo
}
env := append(os.Environ(),
"CGO_ENABLED=1",
fmt.Sprintf("GOOS=%s", goos),
fmt.Sprintf("GOARCH=%s", goarch))
tagstr := "selinux"
if *tags != "" {
tagstr = tagstr + "," + *tags
}
flags := []string{"-tags", tagstr}
return &packages.Config{
Mode: mode,
Env: env,
BuildFlags: flags,
Tests: !(*skipTest),
}
}
type collector struct {
dirs []string
ignoreDirs []string
}
func newCollector(ignoreDirs string) collector {
c := collector{
ignoreDirs: append([]string(nil), standardIgnoreDirs...),
}
if ignoreDirs != "" {
c.ignoreDirs = append(c.ignoreDirs, strings.Split(ignoreDirs, ",")...)
}
return c
}
func (c *collector) walk(roots []string) error {
for _, root := range roots {
err := filepath.Walk(root, c.handlePath)
if err != nil {
return err
}
}
sort.Strings(c.dirs)
return nil
}
// handlePath walks the filesystem recursively, collecting directories,
// ignoring some unneeded directories (hidden/vendored) that are handled
// specially later.
func (c *collector) handlePath(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
name := info.Name()
// Ignore hidden directories (.git, .cache, etc)
if (len(name) > 1 && (name[0] == '.' || name[0] == '_')) || name == "testdata" {
if *verbose {
fmt.Printf("DBG: skipping dir %s\n", path)
}
return filepath.SkipDir
}
for _, dir := range c.ignoreDirs {
if path == dir {
if *verbose {
fmt.Printf("DBG: ignoring dir %s\n", path)
}
return filepath.SkipDir
}
}
// Make dirs into relative pkg names.
// NOTE: can't use filepath.Join because it elides the leading "./"
pkg := path
if !strings.HasPrefix(pkg, "./") {
pkg = "./" + pkg
}
c.dirs = append(c.dirs, pkg)
if *verbose {
fmt.Printf("DBG: added dir %s\n", path)
}
}
return nil
}
func (c *collector) verify(plat string) ([]string, error) {
errors := []packages.Error{}
start := time.Now()
config := newConfig(plat)
rootPkgs, err := packages.Load(config, c.dirs...)
if err != nil {
return nil, err
}
// Recursively import all deps and flatten to one list.
allMap := map[string]*packages.Package{}
for _, pkg := range rootPkgs {
if *verbose {
serialFprintf(os.Stdout, "pkg %q has %d GoFiles\n", pkg.PkgPath, len(pkg.GoFiles))
}
allMap[pkg.PkgPath] = pkg
if len(pkg.Imports) > 0 {
for _, imp := range pkg.Imports {
if *verbose {
serialFprintf(os.Stdout, "pkg %q imports %q\n", pkg.PkgPath, imp.PkgPath)
}
allMap[imp.PkgPath] = imp
}
}
}
keys := make([]string, 0, len(allMap))
for k := range allMap {
keys = append(keys, k)
}
sort.Strings(keys)
allList := make([]*packages.Package, 0, len(keys))
for _, k := range keys {
allList = append(allList, allMap[k])
}
for _, pkg := range allList {
if len(pkg.GoFiles) > 0 {
if len(pkg.Errors) > 0 && (pkg.PkgPath == "main" || strings.Contains(pkg.PkgPath, ".")) {
errors = append(errors, pkg.Errors...)
}
}
if *defuses {
for id, obj := range pkg.TypesInfo.Defs {
serialFprintf(os.Stdout, "%s: %q defines %v\n",
pkg.Fset.Position(id.Pos()), id.Name, obj)
}
for id, obj := range pkg.TypesInfo.Uses {
serialFprintf(os.Stdout, "%s: %q uses %v\n",
pkg.Fset.Position(id.Pos()), id.Name, obj)
}
}
}
if *timings {
serialFprintf(os.Stdout, "%s took %.1fs\n", plat, time.Since(start).Seconds())
}
return dedup(errors), nil
}
func dedup(errors []packages.Error) []string {
ret := []string{}
m := map[string]bool{}
for _, e := range errors {
es := e.Error()
if !m[es] {
ret = append(ret, es)
m[es] = true
}
}
return ret
}
var outMu sync.Mutex
func serialFprintf(w io.Writer, format string, a ...any) (n int, err error) {
outMu.Lock()
defer outMu.Unlock()
return fmt.Fprintf(w, format, a...)
}
func main() {
flag.Parse()
args := flag.Args()
if *verbose {
*serial = true // to avoid confusing interleaved logs
}
if len(args) == 0 {
args = append(args, ".")
}
c := newCollector(*ignoreDirs)
if err := c.walk(args); err != nil {
log.Fatalf("Error walking: %v", err)
}
plats := crossPlatforms[:]
if *platforms != "" {
plats = strings.Split(*platforms, ",")
} else if !*cross {
plats = plats[:1]
}
var wg sync.WaitGroup
var failMu sync.Mutex
failed := false
if *serial {
*parallel = 1
} else if *parallel == 0 {
*parallel = len(plats)
}
throttle := make(chan int, *parallel)
for _, plat := range plats {
wg.Add(1)
go func(plat string) {
// block until there's room for this task
throttle <- 1
defer func() {
// indicate this task is done
<-throttle
}()
f := false
serialFprintf(os.Stdout, "type-checking %s\n", plat)
errors, err := c.verify(plat)
if err != nil {
serialFprintf(os.Stderr, "ERROR(%s): failed to verify: %v\n", plat, err)
f = true
} else if len(errors) > 0 {
for _, e := range errors {
// Special case CGo errors which may depend on headers we
// don't have.
if !strings.HasSuffix(e, "could not import C (no metadata for C)") {
f = true
serialFprintf(os.Stderr, "ERROR(%s): %s\n", plat, e)
}
}
}
failMu.Lock()
failed = failed || f
failMu.Unlock()
wg.Done()
}(plat)
}
wg.Wait()
if failed {
os.Exit(1)
}
}

@ -1,121 +0,0 @@
// Copyright © 2023 OpenIM. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package main
import (
"errors"
"flag"
"os"
"path/filepath"
"testing"
"golang.org/x/tools/go/packages"
)
// This exists because `go` is not always in the PATH when running CI.
var goBinary = flag.String("go", "", "path to a `go` binary")
func TestVerify(t *testing.T) {
// x/tools/packages is going to literally exec `go`, so it needs some
// setup.
setEnvVars(t)
tcs := []struct {
path string
expect int
}{
// {"./testdata/good", 0},
// {"./testdata/bad", 18},
}
for _, tc := range tcs {
c := newCollector("")
if err := c.walk([]string{tc.path}); err != nil {
t.Fatalf("error walking %s: %v", tc.path, err)
}
errs, err := c.verify("linux/amd64")
if err != nil {
t.Errorf("unexpected error: %v", err)
} else if len(errs) != tc.expect {
t.Errorf("Expected %d errors, got %d: %v", tc.expect, len(errs), errs)
}
}
}
func setEnvVars(t testing.TB) {
t.Helper()
if *goBinary != "" {
newPath := filepath.Dir(*goBinary)
curPath := os.Getenv("PATH")
if curPath != "" {
newPath = newPath + ":" + curPath
}
t.Setenv("PATH", newPath)
}
if os.Getenv("HOME") == "" {
t.Setenv("HOME", "/tmp")
}
}
func TestHandlePath(t *testing.T) {
c := collector{
ignoreDirs: standardIgnoreDirs,
}
e := errors.New("ex")
i, _ := os.Stat(".") // i.IsDir() == true
if c.handlePath("foo", nil, e) != e {
t.Error("handlePath not returning errors")
}
if c.handlePath("vendor", i, nil) != filepath.SkipDir {
t.Error("should skip vendor")
}
}
func TestDedup(t *testing.T) {
testcases := []struct {
input []packages.Error
expected int
}{{
input: nil,
expected: 0,
}, {
input: []packages.Error{
{Pos: "file:7", Msg: "message", Kind: packages.ParseError},
},
expected: 1,
}, {
input: []packages.Error{
{Pos: "file:7", Msg: "message1", Kind: packages.ParseError},
{Pos: "file:8", Msg: "message2", Kind: packages.ParseError},
},
expected: 2,
}, {
input: []packages.Error{
{Pos: "file:7", Msg: "message1", Kind: packages.ParseError},
{Pos: "file:8", Msg: "message2", Kind: packages.ParseError},
{Pos: "file:7", Msg: "message1", Kind: packages.ParseError},
},
expected: 2,
}}
for i, tc := range testcases {
out := dedup(tc.input)
if len(out) != tc.expected {
t.Errorf("[%d] dedup(%v) = '%v', expected %d",
i, tc.input, out, tc.expected)
}
}
}
Loading…
Cancel
Save