add golangci-lint to CI?

[0pcom]

Describe the feature

golangci-lint is a good test suite, and I think it helps identify common issues with code and helps ensure high quality of code.

Here's the config for golangci-lint which I've used (i.e. .golangci.yaml)

# options for analysis running
run:
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  deadline: 3m

  # exit code when at least one issue was found, default is 1
  issues-exit-code: 1

  # include test files or not, default is true
  tests: true

  # list of build tags, all linters use it. Default is empty list.
  build-tags:

  # which dirs to skip: they won't be analyzed;
  # can use regexp here: generated.*, regexp is applied on full path;
  # default value is empty list, but next dirs are always skipped independently
  # from this option's value:
  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  skip-dirs:

  # which files to skip: they will be analyzed, but issues from them
  # won't be reported. Default value is empty list, but there is
  # no need to include all autogenerated files, we confidently recognize
  # autogenerated files. If it's not please let us know.
  skip-files:

  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
  # If invoked with -mod=readonly, the go command is disallowed from the implicit
  # automatic updating of go.mod described above. Instead, it fails when any changes
  # to go.mod are needed. This setting is most useful to check that go.mod does
  # not need updates, such as in a continuous integration and testing system.
  # If invoked with -mod=vendor, the go command assumes that the vendor
  # directory holds the correct copies of dependencies and ignores
  # the dependency descriptions in go.mod.
  modules-download-mode: vendor


# output configuration options
output:
  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
  formats: colored-line-number

  # print lines of code with issue, default is true
  print-issued-lines: true

  # print linter name in the end of issue text, default is true
  print-linter-name: true


# all available settings of specific linters
linters-settings:
  errcheck:
    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
    # default is false: such cases aren't reported by default.
    check-type-assertions: false

    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
    # default is false: such cases aren't reported by default.
    check-blank: true
  govet:
    # report about shadowed variables
    shadow: true
    # Obtain type information from installed (to $GOPATH/pkg) package files:
    # golangci-lint will execute `go install -i` and `go test -i` for analyzed packages
    # before analyzing them.
    # By default this option is disabled and govet gets type information by loader from source code.
    # Loading from source code is slow, but it's done only once for all linters.
    # Go-installing of packages first time is much slower than loading them from source code,
    # therefore this option is disabled by default.
    # But repeated installation is fast in go >= 1.10 because of build caching.
    # Enable this option only if all conditions are met:
    #  1. you use only "fast" linters (--fast e.g.): no program loading occurs
    #  2. you use go >= 1.10
    #  3. you do repeated runs (false for CI) or cache $GOPATH/pkg or `go env GOCACHE` dir in CI.
    use-installed-packages: false
  golint:
    # minimal confidence for issues, default is 0.8
    min-confidence: 0.8
  gofmt:
    # simplify code: gofmt with `-s` option, true by default
    simplify: true
  gocyclo:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 10
  maligned:
    # print struct with more effective memory layout or not, false by default
    suggest-new: true
  dupl:
    # tokens count to trigger issue, 150 by default
    threshold: 100
  goconst:
    # minimal length of string constant, 3 by default
    min-len: 3
    # minimal occurrences count to trigger, 3 by default
    min-occurrences: 3
  depguard:
    list-type: blacklist
    include-go-root: false
    packages:
      - github.com/pkg/errors
  misspell:
    # Correct spellings using locale preferences for US or UK.
    # Default is to use a neutral variety of English.
    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
    locale: US
  lll:
    # max line length, lines longer will be reported. Default is 120.
    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
    line-length: 120
    # tab width in spaces. Default to 1.
    tab-width: 1
  unused:
    # treat code as a program (not a library) and report unused exported identifiers; default is false.
    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  unparam:
    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
    # and rta for programs with main packages. Default is cha.
    algo: cha

    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  prealloc:
    # XXX: we don't recommend using this linter before doing performance profiling.
    # For most programs usage of prealloc will be a premature optimization.

    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
    # True by default.
    simple: true
    range-loops: true # Report preallocation suggestions on range loops, true by default
    for-loops: false # Report preallocation suggestions on for loops, false by default
  goimports:
    local-prefixes: github.com/cogentcore/core


linters:
  enable:
    - revive
    - goimports
    - unparam
    - errcheck
    - gosimple
    - staticcheck
    - ineffassign
    - typecheck
    - gosec
    - gosimple
    - staticcheck
    - unused
    - misspell
    - nakedret
  enable-all: false
  disable-all: true
  presets:
  fast: false


issues:
  # List of regexps of issue texts to exclude, empty list by default.
  # But independently from this option we use default exclude patterns,
  # it can be disabled by `exclude-use-default: false`. To list all
  # excluded by default patterns execute `golangci-lint run --help`
  exclude:

  # Independently from option `exclude` we use default exclude patterns,
  # it can be disabled by this option. To list all
  # excluded by default patterns execute `golangci-lint run --help`.
  # Default value for this option is true.
  exclude-use-default: false

  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-per-linter: 0

  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same: 0

  # Show only new issues: if there are unstaged changes or untracked files,
  # only those changes are analyzed, else only changes in HEAD~ are analyzed.
  # It's a super-useful option for integration of golangci-lint into existing
  # large codebase. It's not practical to fix all existing issues at the moment
  # of integration: much better don't allow issues in new code.
  # Default is false.
  new: false

I think it's helpful for contributors to the codebase to run this ; because not everyone who might contribute uses the same IDE, and some may not use one at all. So this is just something that could be run separately to ensure at least that no further code quality issues or related errors are introduced. It could be run as part of the CI for this repo.

However, I will note that getting all the initial errors solved is likely to be a huge undertaking with the state of the code as it is currently. The tool is configurable to do any kind of checks you want, or even to not check certain things.

Posted below are some of the results of running golangci-lint for the cmd folder in this repo - non exhaustive output

Relevant code

$ golangci-lint --version
golangci-lint has version 1.62.2 built with go1.23.4 from 89476e7 on 2024-12-12T18:00:50Z

$ golangci-lint run -c .golangci.yml ./cmd/...
cmd/core/config/config.go:244:14: Error return value of `errors.Log` is not checked (errcheck)
			errors.Log(err)
			          ^
cmd/core/config/web.go:11:8: `containts` is a misspelling of `contains` (misspell)
// Web containts the configuration information for building for web and creating
       ^
cmd/core/config/config.go:78:6: exported: exported type Build should have comment or be unexported (revive)
type Build struct { //types:add
     ^
cmd/core/config/config.go:120:6: exported: exported type Pack should have comment or be unexported (revive)
type Pack struct { //types:add
     ^
cmd/core/config/config.go:127:6: exported: exported type Log should have comment or be unexported (revive)
type Log struct { //types:add
     ^
cmd/core/config/config.go:139:6: exported: exported type Generate should have comment or be unexported (revive)
type Generate struct { //types:add
     ^
cmd/core/config/config.go:155:1: exported: exported method Config.OnConfig should have comment or be unexported (revive)
func (c *Config) OnConfig(cmd string) error {
^
cmd/core/mobile/sdkpath/sdkpath.go:60:20: Error return value of `sdkDir.Close` is not checked (errcheck)
	defer sdkDir.Close()
	                  ^
cmd/core/mobile/sdkpath/sdkpath.go:56:17: G304: Potential file inclusion via variable (gosec)
	sdkDir, err := os.Open(filepath.Join(sdk, "platforms"))
	               ^
cmd/core/web/serve.go:26:18: Error return value of `logx.PrintlnWarn` is not checked (errcheck)
	logx.PrintlnWarn("Serving at http://localhost:" + c.Web.Port)
	                ^
cmd/core/web/build.go:9:2: G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
	"crypto/sha1"
	^
cmd/core/web/build.go:65:33: G401: Use of weak cryptographic primitive (gosec)
		c.Version = fmt.Sprintf(`%x`, sha1.Sum([]byte(t)))
		                              ^
cmd/core/web/build.go:73:9: G306: Expect WriteFile permissions to be 0600 or less (gosec)
	err := os.WriteFile(filepath.Join(odir, "wasm_exec.js"), wej, 0666)
	       ^
cmd/core/web/build.go:82:8: G306: Expect WriteFile permissions to be 0600 or less (gosec)
	err = os.WriteFile(filepath.Join(odir, "app.js"), ajs, 0666)
	      ^
cmd/core/web/build.go:91:8: G306: Expect WriteFile permissions to be 0600 or less (gosec)
	err = os.WriteFile(filepath.Join(odir, "app-worker.js"), awjs, 0666)
	      ^
cmd/core/web/build.go:150:8: G301: Expect directory permissions to be 0750 or less (gosec)
	err = os.MkdirAll(filepath.Join(odir, "icons"), 0777)
	      ^
cmd/core/web/build.go:200:9: G301: Expect directory permissions to be 0750 or less (gosec)
		err = os.MkdirAll(opath, 0777)
		      ^
cmd/core/web/serve.go:27:9: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
	return http.ListenAndServe(":"+c.Web.Port, nil)
	       ^
cmd/core/cmd/changed.go:26:12: Error return value of `fs.WalkDir` is not checked (errcheck)
	fs.WalkDir(os.DirFS("."), ".", func(path string, d fs.DirEntry, err error) error {
	          ^
cmd/core/cmd/pack.go:115:18: Error return value of `fapp.Close` is not checked (errcheck)
	defer fapp.Close()
	                ^
cmd/core/cmd/pack.go:134:19: Error return value of `fctrl.Close` is not checked (errcheck)
	defer fctrl.Close()
	                 ^
cmd/core/cmd/pack.go:223:18: Error return value of `fdsi.Close` is not checked (errcheck)
	defer fdsi.Close()
	                ^
cmd/core/cmd/pack.go:238:20: Error return value of `fplist.Close` is not checked (errcheck)
	defer fplist.Close()
	                  ^
cmd/core/cmd/pack.go:268:18: Error return value of `fdmg.Close` is not checked (errcheck)
	defer fdmg.Close()
	                ^
cmd/core/cmd/pack.go:363:12: Error return value of `fman.Close` is not checked (errcheck)
	fman.Close()
	          ^
cmd/core/cmd/pull.go:23:12: Error return value of `fs.WalkDir` is not checked (errcheck)
	fs.WalkDir(os.DirFS("."), ".", func(path string, d fs.DirEntry, err error) error {
	          ^
cmd/core/cmd/run.go:37:19: Error return value of `logx.PrintlnWarn` is not checked (errcheck)
		logx.PrintlnWarn("warning: only installing, not running, because there is no effective way to just launch an app on iOS from the terminal without debugging; pass the -d flag to run and debug")
		                ^
cmd/core/cmd/setup.go:30:20: Error return value of `logx.PrintlnWarn` is not checked (errcheck)
			logx.PrintlnWarn("xcode tools already installed")
			                ^
cmd/core/cmd/build.go:44:11: G301: Expect directory permissions to be 0750 or less (gosec)
			err := os.MkdirAll(c.Build.Output, 0777)
			       ^
cmd/core/cmd/pack.go:111:15: G304: Potential file inclusion via variable (gosec)
	fapp, err := os.Create(filepath.Join(usapath, anm+".desktop"))
	             ^
cmd/core/cmd/pack.go:130:16: G304: Potential file inclusion via variable (gosec)
	fctrl, err := os.Create(filepath.Join(dpath, "control"))
	              ^
cmd/core/cmd/log.go:27:20: `continiously` is a misspelling of `continuously` (misspell)
	// we are logging continiously so we can't buffer, and we must be verbose
	                  ^
cmd/core/cmd/registry_notwindows.go:5:29: unused-parameter: parameter 'path' seems to be unused, consider removing or renaming it as _ (revive)
func windowsRegistryAddPath(path string) error {
                            ^
cmd/core/cmd/changed.go:23:14: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
func Changed(c *config.Config) error { //types:add
             ^
cmd/core/cmd/setup.go:19:12: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
func Setup(c *config.Config) error { //types:add
           ^
cmd/core/cmd/pull.go:20:11: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
func Pull(c *config.Config) error { //types:add
          ^
cmd/core/generate/icons.go:50:12: Error return value of `fs.WalkDir` is not checked (errcheck)
	fs.WalkDir(os.DirFS(c.Generate.Icons), ".", func(path string, d fs.DirEntry, err error) error {
	          ^
cmd/core/generate/pages.go:52:16: Error return value of `f.Close` is not checked (errcheck)
		defer f.Close()
		             ^
cmd/core/rendericon/rendericon.go:5:1: package-comments: should have a package comment (revive)
package rendericon
^
cmd/core/core.go:20:9: Error return value of `cli.Run` is not checked (errcheck)
	cli.Run(opts, &config.Config{}, cmd.Setup, cmd.Build, cmd.Run, cmd.Pack, cmd.Install, generate.Generate, cmd.Changed, cmd.Pull, cmd.Log, cmd.Release, cmd.NextRelease)
	       ^
cmd/core/mobile/build.go:136:8: Error return value of `strconv.ParseFloat` is not checked (errcheck)
				v, _ := strconv.ParseFloat(c.Build.IOSVersion, 64)
				   ^
cmd/core/mobile/build.go:173:9: Error return value of `w.Close` is not checked (errcheck)
	w.Close()
	       ^
cmd/core/mobile/build_androidapp.go:65:19: Error return value of `logx.PrintfDebug` is not checked (errcheck)
		logx.PrintfDebug("generated AndroidManifest.xml:\n%s\n", manifestData)
		                ^
cmd/core/mobile/build_androidapp.go:134:18: Error return value of `logx.PrintfInfo` is not checked (errcheck)
		logx.PrintfInfo("apk: %s\n", name)
		               ^
cmd/core/mobile/build_androidapp.go:146:16: Error return value of `f.Close` is not checked (errcheck)
		defer f.Close()
		             ^
cmd/core/mobile/build_apple.go:129:18: Error return value of `fdsi.Close` is not checked (errcheck)
	defer fdsi.Close()
	                ^
cmd/core/mobile/build_test.go:89:29: Error return value of `reflectx.SetFromDefaultTags` is not checked (errcheck)
	reflectx.SetFromDefaultTags(c)
	                           ^
cmd/core/mobile/build_test.go:90:12: Error return value of `c.OnConfig` is not checked (errcheck)
	c.OnConfig("build")
	          ^
cmd/core/mobile/build_test.go:97:12: Error return value of `os.Setenv` is not checked (errcheck)
		os.Setenv("HOMEDRIVE", "C:")
		         ^
cmd/core/mobile/build_test.go:239:20: Error return value of `os.RemoveAll` is not checked (errcheck)
	defer os.RemoveAll(dir)
	                  ^
cmd/core/mobile/build_test.go:275:33: Error return value of `reflectx.SetFromDefaultTags` is not checked (errcheck)
					reflectx.SetFromDefaultTags(c)
					                           ^
cmd/core/mobile/build_test.go:276:16: Error return value of `c.OnConfig` is not checked (errcheck)
					c.OnConfig("build")
					          ^
cmd/core/mobile/cert_test.go:39:17: Error return value of `os.Remove` is not checked (errcheck)
	defer os.Remove(sigPath)
	               ^
cmd/core/mobile/env.go:110:18: Error return value of `logx.PrintlnInfo` is not checked (errcheck)
	logx.PrintlnInfo("GOMOBILE=" + goMobilePath)
	                ^
cmd/core/mobile/env.go:119:17: Error return value of `exec.RemoveAll` is not checked (errcheck)
		exec.RemoveAll(tmpDir)
		              ^
cmd/core/mobile/env.go:125:18: Error return value of `logx.PrintlnInfo` is not checked (errcheck)
	logx.PrintlnInfo("WORK=" + tmpDir)
	                ^
cmd/core/mobile/env.go:274:27: Error return value of `platformsJson.Close` is not checked (errcheck)
	defer platformsJson.Close()
	                         ^
cmd/core/mobile/env.go:291:22: Error return value of `abisJson.Close` is not checked (errcheck)
	defer abisJson.Close()
	                    ^
cmd/core/mobile/env.go:336:24: Error return value of `properties.Close` is not checked (errcheck)
	defer properties.Close()
	                      ^
cmd/core/mobile/env_test.go:24:13: Error return value of `os.Unsetenv` is not checked (errcheck)
	os.Unsetenv("ANDROID_HOME")
	           ^
cmd/core/mobile/env_test.go:26:13: Error return value of `os.Unsetenv` is not checked (errcheck)
	os.Unsetenv("ANDROID_NDK_HOME")
	           ^
cmd/core/mobile/env_test.go:29:12: Error return value of `os.Setenv` is not checked (errcheck)
		os.Setenv("ANDROID_HOME", homeorig)
		         ^
cmd/core/mobile/env_test.go:30:12: Error return value of `os.Setenv` is not checked (errcheck)
		os.Setenv("ANDROID_NDK_HOME", ndkhomeorig)
		         ^
cmd/core/mobile/env_test.go:31:15: Error return value of `os.RemoveAll` is not checked (errcheck)
		os.RemoveAll(home)
		            ^
cmd/core/mobile/env_test.go:35:21: Error return value of `cli.SetFromDefaults` is not checked (errcheck)
	cli.SetFromDefaults(c)
	                   ^
cmd/core/mobile/env_test.go:66:20: Error return value of `os.Unsetenv` is not checked (errcheck)
		defer os.Unsetenv("ANDROID_HOME")
		                 ^
cmd/core/mobile/env_test.go:83:21: Error return value of `os.RemoveAll` is not checked (errcheck)
		defer os.RemoveAll(sdkNDK)
		                  ^
cmd/core/mobile/writer.go:167:14: Error return value of `fmt.Fprintf` is not checked (errcheck)
		fmt.Fprintf(cHash, "Name: %s\r\nSHA1-Digest: %s\r\n\r\n", n, h)
		           ^
cmd/core/mobile/writer_test.go:35:9: Error return value of `f.Close` is not checked (errcheck)
	f.Close()
	       ^
cmd/core/mobile/writer_test.go:36:17: Error return value of `os.Remove` is not checked (errcheck)
	defer os.Remove(f.Name())
	               ^
cmd/core/mobile/writer_test.go:43:17: Error return value of `os.Remove` is not checked (errcheck)
	defer os.Remove(apkPath)
	               ^
cmd/core/mobile/build_androidapp.go:289:7: G101: Potential hardcoded credentials: RSA private key (gosec)
const debugCert = `
-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAy6ItnWZJ8DpX9R5FdWbS9Kr1U8Z7mKgqNByGU7No99JUnmyu
NQ6Uy6Nj0Gz3o3c0BXESECblOC13WdzjsH1Pi7/L9QV8jXOXX8cvkG5SJAyj6hcO
LOapjDiN89NXjXtyv206JWYvRtpexyVrmHJgRAw3fiFI+m4g4Qop1CxcIF/EgYh7
rYrqh4wbCM1OGaCleQWaOCXxZGm+J5YNKQcWpjZRrDrb35IZmlT0bK46CXUKvCqK
x7YXHgfhC8ZsXCtsScKJVHs7gEsNxz7A0XoibFw6DoxtjKzUCktnT0w3wxdY7OTj
9AR8mobFlM9W3yirX8TtwekWhDNTYEu8dwwykwIDAQABAoIBAA2hjpIhvcNR9H9Z
BmdEecydAQ0ZlT5zy1dvrWI++UDVmIp+Ve8BSd6T0mOqV61elmHi3sWsBN4M1Rdz
3N38lW2SajG9q0fAvBpSOBHgAKmfGv3Ziz5gNmtHgeEXfZ3f7J95zVGhlHqWtY95
JsmuplkHxFMyITN6WcMWrhQg4A3enKLhJLlaGLJf9PeBrvVxHR1/txrfENd2iJBH
FmxVGILL09fIIktJvoScbzVOneeWXj5vJGzWVhB17DHBbANGvVPdD5f+k/s5aooh
hWAy/yLKocr294C4J+gkO5h2zjjjSGcmVHfrhlXQoEPX+iW1TGoF8BMtl4Llc+jw
lKWKfpECgYEA9C428Z6CvAn+KJ2yhbAtuRo41kkOVoiQPtlPeRYs91Pq4+NBlfKO
2nWLkyavVrLx4YQeCeaEU2Xoieo9msfLZGTVxgRlztylOUR+zz2FzDBYGicuUD3s
EqC0Wv7tiX6dumpWyOcVVLmR9aKlOUzA9xemzIsWUwL3PpyONhKSq7kCgYEA1X2F
f2jKjoOVzglhtuX4/SP9GxS4gRf9rOQ1Q8DzZhyH2LZ6Dnb1uEQvGhiqJTU8CXxb
7odI0fgyNXq425Nlxc1Tu0G38TtJhwrx7HWHuFcbI/QpRtDYLWil8Zr7Q3BT9rdh
moo4m937hLMvqOG9pyIbyjOEPK2WBCtKW5yabqsCgYEAu9DkUBr1Qf+Jr+IEU9I8
iRkDSMeusJ6gHMd32pJVCfRRQvIlG1oTyTMKpafmzBAd/rFpjYHynFdRcutqcShm
aJUq3QG68U9EAvWNeIhA5tr0mUEz3WKTt4xGzYsyWES8u4tZr3QXMzD9dOuinJ1N
+4EEumXtSPKKDG3M8Qh+KnkCgYBUEVSTYmF5EynXc2xOCGsuy5AsrNEmzJqxDUBI
SN/P0uZPmTOhJIkIIZlmrlW5xye4GIde+1jajeC/nG7U0EsgRAV31J4pWQ5QJigz
0+g419wxIUFryGuIHhBSfpP472+w1G+T2mAGSLh1fdYDq7jx6oWE7xpghn5vb9id
EKLjdwKBgBtz9mzbzutIfAW0Y8F23T60nKvQ0gibE92rnUbjPnw8HjL3AZLU05N+
cSL5bhq0N5XHK77sscxW9vXjG0LJMXmFZPp9F6aV6ejkMIXyJ/Yz/EqeaJFwilTq
Mc6xR47qkdzu0dQ1aPm4XD7AWDtIvPo/GG2DKOucLBbQc2cOWtKS
-----END RSA PRIVATE KEY-----
`
cmd/core/mobile/cert.go:10:2: G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
	"crypto/sha1"
	^
cmd/core/mobile/cert.go:44:7: G401: Use of weak cryptographic primitive (gosec)
	h := sha1.New()
	     ^
cmd/core/mobile/writer.go:74:2: G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
	"crypto/sha1"
	^
cmd/core/mobile/writer.go:132:9: G401: Use of weak cryptographic primitive (gosec)
		sha1: sha1.New(),
		      ^
cmd/core/mobile/cert_test.go:72:7: G101: Potential hardcoded credentials: RSA private key (gosec)
const testKey = `
-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAy6ItnWZJ8DpX9R5FdWbS9Kr1U8Z7mKgqNByGU7No99JUnmyu
NQ6Uy6Nj0Gz3o3c0BXESECblOC13WdzjsH1Pi7/L9QV8jXOXX8cvkG5SJAyj6hcO
LOapjDiN89NXjXtyv206JWYvRtpexyVrmHJgRAw3fiFI+m4g4Qop1CxcIF/EgYh7
rYrqh4wbCM1OGaCleQWaOCXxZGm+J5YNKQcWpjZRrDrb35IZmlT0bK46CXUKvCqK
x7YXHgfhC8ZsXCtsScKJVHs7gEsNxz7A0XoibFw6DoxtjKzUCktnT0w3wxdY7OTj
9AR8mobFlM9W3yirX8TtwekWhDNTYEu8dwwykwIDAQABAoIBAA2hjpIhvcNR9H9Z
BmdEecydAQ0ZlT5zy1dvrWI++UDVmIp+Ve8BSd6T0mOqV61elmHi3sWsBN4M1Rdz
3N38lW2SajG9q0fAvBpSOBHgAKmfGv3Ziz5gNmtHgeEXfZ3f7J95zVGhlHqWtY95
JsmuplkHxFMyITN6WcMWrhQg4A3enKLhJLlaGLJf9PeBrvVxHR1/txrfENd2iJBH
FmxVGILL09fIIktJvoScbzVOneeWXj5vJGzWVhB17DHBbANGvVPdD5f+k/s5aooh
hWAy/yLKocr294C4J+gkO5h2zjjjSGcmVHfrhlXQoEPX+iW1TGoF8BMtl4Llc+jw
lKWKfpECgYEA9C428Z6CvAn+KJ2yhbAtuRo41kkOVoiQPtlPeRYs91Pq4+NBlfKO
2nWLkyavVrLx4YQeCeaEU2Xoieo9msfLZGTVxgRlztylOUR+zz2FzDBYGicuUD3s
EqC0Wv7tiX6dumpWyOcVVLmR9aKlOUzA9xemzIsWUwL3PpyONhKSq7kCgYEA1X2F
f2jKjoOVzglhtuX4/SP9GxS4gRf9rOQ1Q8DzZhyH2LZ6Dnb1uEQvGhiqJTU8CXxb
7odI0fgyNXq425Nlxc1Tu0G38TtJhwrx7HWHuFcbI/QpRtDYLWil8Zr7Q3BT9rdh
moo4m937hLMvqOG9pyIbyjOEPK2WBCtKW5yabqsCgYEAu9DkUBr1Qf+Jr+IEU9I8
iRkDSMeusJ6gHMd32pJVCfRRQvIlG1oTyTMKpafmzBAd/rFpjYHynFdRcutqcShm
aJUq3QG68U9EAvWNeIhA5tr0mUEz3WKTt4xGzYsyWES8u4tZr3QXMzD9dOuinJ1N
+4EEumXtSPKKDG3M8Qh+KnkCgYBUEVSTYmF5EynXc2xOCGsuy5AsrNEmzJqxDUBI
SN/P0uZPmTOhJIkIIZlmrlW5xye4GIde+1jajeC/nG7U0EsgRAV31J4pWQ5QJigz
0+g419wxIUFryGuIHhBSfpP472+w1G+T2mAGSLh1fdYDq7jx6oWE7xpghn5vb9id
EKLjdwKBgBtz9mzbzutIfAW0Y8F23T60nKvQ0gibE92rnUbjPnw8HjL3AZLU05N+
cSL5bhq0N5XHK77sscxW9vXjG0LJMXmFZPp9F6aV6ejkMIXyJ/Yz/EqeaJFwilTq
Mc6xR47qkdzu0dQ1aPm4XD7AWDtIvPo/GG2DKOucLBbQc2cOWtKS
-----END RSA PRIVATE KEY-----
`
cmd/core/mobile/writer_test.go:71:14: G204: Subprocess launched with variable (gosec)
	out, err := exec.Command("aapt", "list", "-a", apkPath).CombinedOutput()
	            ^
cmd/core/mobile/writer_test.go:169:15: G204: Subprocess launched with variable (gosec)
	data, err := exec.Command("diff", "-u", wantPath, gotPath).CombinedOutput()
	             ^
cmd/core/mobile/env.go:430:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
	} else {
		var arch string
		switch runtime.GOARCH {
		case "386":
			arch = "x86"
		case "amd64":
			arch = "x86_64"
		case "arm64":
			// Android NDK does not contain arm64 toolchains (until and
			// including NDK 23), use use x86_64 instead. See:
			// https://github.com/android/ndk/issues/1299
			if runtime.GOOS == "darwin" {
				arch = "x86_64"
				break
			}
			if runtime.GOOS == "android" { // termux
				return "linux-aarch64"
			}
			fallthrough
		default:
			panic("unsupported GOARCH: " + runtime.GOARCH)
		}
		return runtime.GOOS + "-" + arch
	}
cmd/core/mobile/writer.go:225:29: (*writer).clearCur - result 0 (error) is always nil (unparam)
func (w *writer) clearCur() error {
                            ^
cmd/core/mobile/build_test.go:246:3: ineffectual assignment to path (ineffassign)
		path += string(filepath.ListSeparator) + p
		^
cmd/core/mobile/cert_test.go:11:2: SA1019: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os], and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"
	^
cmd/core/mobile/writer_test.go:11:2: SA1019: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os], and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"
	^
cmd/core/mobile/binres/node.go:168:34: Error return value of `(*cogentcore.org/core/cmd/core/mobile/binres.NodeHeader).UnmarshalBinary` is not checked (errcheck)
	(&el.NodeHeader).UnmarshalBinary(bin)
	                                ^
cmd/core/mobile/binres/pool.go:270:34: Error return value of `(*cogentcore.org/core/cmd/core/mobile/binres.chunkHeader).UnmarshalBinary` is not checked (errcheck)
	(&m.chunkHeader).UnmarshalBinary(bin)
	                                ^
cmd/core/mobile/binres/sdk.go:30:16: Error return value of `zr.Close` is not checked (errcheck)
	defer zr.Close()
	              ^
cmd/core/mobile/binres/sdk.go:39:13: G110: Potential DoS vulnerability via decompression bomb (gosec)
			_, err = io.Copy(buf, rc)
			         ^
cmd/core/mobile/binres/table.go:116:15: G110: Potential DoS vulnerability via decompression bomb (gosec)
	if _, err := io.Copy(&buf, zr); err != nil {
	             ^
cmd/core/mobile/binres/binres.go:336:24: G115: integer overflow conversion int -> uint32 (gosec)
					LineNumber: uint32(line),
					                  ^
cmd/core/mobile/binres/binres.go:364:26: G115: integer overflow conversion int -> uint32 (gosec)
							LineNumber: uint32(line),
							                  ^
cmd/core/mobile/binres/binres.go:390:38: G115: integer overflow conversion int -> uint32 (gosec)
						nattr.TypedValue.Value = uint32(i)
						                               ^
cmd/core/mobile/binres/binres.go:415:20: G115: integer overflow conversion uint32 -> uint8 (gosec)
						t := DataType(val.data.Value)
						             ^
cmd/core/mobile/binres/node.go:126:28: G115: integer overflow conversion int -> uint16 (gosec)
	el.AttributeCount = uint16(len(el.attrs))
	                          ^
cmd/core/mobile/binres/pool.go:189:30: G115: integer overflow conversion int -> uint16 (gosec)
			strs = append(strs, uint16(len(p))) // string length (implicitly includes zero terminator to follow)
			                          ^
cmd/core/mobile/binres/table.go:34:23: G115: integer overflow conversion uint32 -> uint8 (gosec)
	pkg := tbl.pkgs[uint8(ref>>24)-1]
	                     ^
cmd/core/mobile/binres/table.go:35:25: G115: integer overflow conversion uint32 -> uint8 (gosec)
	spec := pkg.specs[uint8(ref>>16)-1]
	                       ^
cmd/core/mobile/binres/table.go:36:15: G115: integer overflow conversion uint32 -> uint16 (gosec)
	idx := uint16(ref)
	             ^
cmd/core/mobile/binres/table.go:39:18: G115: integer overflow conversion int -> uint16 (gosec)
		if idx < uint16(len(typ.entries)) {
		               ^
cmd/core/mobile/binres/binres.go:11:23: `marshalling` is a misspelling of `marshaling` (misspell)
// Implementations of marshalling for each struct must produce the exact input
                      ^
cmd/core/mobile/binres/pool.go:27:37: `marshalled` is a misspelling of `marshaled` (misspell)
// Pool has the following structure marshalled:
                                    ^
cmd/core/mobile/binres/binres.go:64:6: exported: exported type ResType should have comment or be unexported (revive)
type ResType uint16
     ^
cmd/core/mobile/binres/binres.go:66:1: exported: exported method ResType.IsSupported should have comment or be unexported (revive)
func (t ResType) IsSupported() bool {
^
cmd/core/mobile/binres/binres.go:153:6: exported: exported type XML should have comment or be unexported (revive)
type XML struct {
     ^
cmd/core/mobile/binres/node.go:15:1: exported: exported method NodeHeader.UnmarshalBinary should have comment or be unexported (revive)
func (hdr *NodeHeader) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/pool.go:13:2: exported: exported const SortedFlag should have comment (or a comment on this block) or be unexported (revive)
	SortedFlag uint32 = 1 << 0
	^
cmd/core/mobile/binres/table.go:17:7: exported: exported const NoEntry should have comment or be unexported (revive)
const NoEntry = 0xFFFFFFFF
      ^
cmd/core/mobile/binres/node.go:24:1: exported: exported method NodeHeader.MarshalBinary should have comment or be unexported (revive)
func (hdr *NodeHeader) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/binres.go:791:1: exported: exported method XML.MarshalBinary should have comment or be unexported (revive)
func (bx *XML) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:176:1: exported: exported method Table.UnmarshalBinary should have comment or be unexported (revive)
func (tbl *Table) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/pool.go:81:1: exported: exported method Pool.IsSorted should have comment or be unexported (revive)
func (pl *Pool) IsSorted() bool { return pl.flags&SortedFlag == SortedFlag }
^
cmd/core/mobile/binres/node.go:36:6: exported: exported type Namespace should have comment or be unexported (revive)
type Namespace struct {
     ^
cmd/core/mobile/binres/pool.go:82:1: exported: exported method Pool.IsUTF8 should have comment or be unexported (revive)
func (pl *Pool) IsUTF8() bool   { return pl.flags&UTF8Flag == UTF8Flag }
^
cmd/core/mobile/binres/table.go:206:1: exported: exported method Table.MarshalBinary should have comment or be unexported (revive)
func (tbl *Table) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/pool.go:84:1: exported: exported method Pool.UnmarshalBinary should have comment or be unexported (revive)
func (pl *Pool) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:44:1: exported: exported method Namespace.UnmarshalBinary should have comment or be unexported (revive)
func (ns *Namespace) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/pool.go:168:1: exported: exported method Pool.MarshalBinary should have comment or be unexported (revive)
func (pl *Pool) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:255:1: exported: exported method Package.UnmarshalBinary should have comment or be unexported (revive)
func (pkg *Package) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/pool.go:244:6: exported: exported type Span should have comment or be unexported (revive)
type Span struct {
     ^
cmd/core/mobile/binres/node.go:54:1: exported: exported method Namespace.MarshalBinary should have comment or be unexported (revive)
func (ns *Namespace) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/pool.go:250:1: exported: exported method Span.UnmarshalBinary should have comment or be unexported (revive)
func (spn *Span) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/table.go:336:1: exported: exported method Package.MarshalBinary should have comment or be unexported (revive)
func (pkg *Package) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/pool.go:269:1: exported: exported method Map.UnmarshalBinary should have comment or be unexported (revive)
func (m *Map) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:74:6: exported: exported type Element should have comment or be unexported (revive)
type Element struct {
     ^
cmd/core/mobile/binres/pool.go:279:1: exported: exported method Map.MarshalBinary should have comment or be unexported (revive)
func (m *Map) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:414:1: exported: exported method TypeSpec.UnmarshalBinary should have comment or be unexported (revive)
func (spec *TypeSpec) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:92:1: exported: exported method Element.UnmarshalBinary should have comment or be unexported (revive)
func (el *Element) UnmarshalBinary(buf []byte) error {
^
cmd/core/mobile/binres/node.go:121:1: exported: exported method Element.MarshalBinary should have comment or be unexported (revive)
func (el *Element) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:434:1: exported: exported method TypeSpec.MarshalBinary should have comment or be unexported (revive)
func (spec *TypeSpec) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:513:1: exported: exported method Type.UnmarshalBinary should have comment or be unexported (revive)
func (typ *Type) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:167:1: exported: exported method ElementEnd.UnmarshalBinary should have comment or be unexported (revive)
func (el *ElementEnd) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:175:1: exported: exported method ElementEnd.MarshalBinary should have comment or be unexported (revive)
func (el *ElementEnd) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:581:1: exported: exported method Type.MarshalBinary should have comment or be unexported (revive)
func (typ *Type) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:636:6: exported: exported type StagedAliasEntry should have comment or be unexported (revive)
type StagedAliasEntry struct {
     ^
cmd/core/mobile/binres/node.go:191:6: exported: exported type Attribute should have comment or be unexported (revive)
type Attribute struct {
     ^
cmd/core/mobile/binres/node.go:198:1: exported: exported method Attribute.UnmarshalBinary should have comment or be unexported (revive)
func (attr *Attribute) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/table.go:641:1: exported: exported method StagedAliasEntry.MarshalBinary should have comment or be unexported (revive)
func (ae *StagedAliasEntry) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/table.go:648:1: exported: exported method StagedAliasEntry.UnmarshalBinary should have comment or be unexported (revive)
func (ae *StagedAliasEntry) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/node.go:205:1: exported: exported method Attribute.MarshalBinary should have comment or be unexported (revive)
func (attr *Attribute) MarshalBinary() ([]byte, error) {
^
cmd/core/mobile/binres/node.go:225:1: exported: exported method CharData.UnmarshalBinary should have comment or be unexported (revive)
func (cdt *CharData) UnmarshalBinary(bin []byte) error {
^
cmd/core/mobile/binres/binres_test.go:61:6: func `compareBytes` is unused (unused)
func compareBytes(a, b []byte) error {
     ^
cmd/core/mobile/binres/binres_test.go:105:9: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
	return fmt.Errorf(buf.String())
	       ^
cmd/core/mobile/binres/binres_test.go:332:10: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
		return fmt.Errorf(buf.String())
		       ^

[kkoreilly]

Thank you for this suggestion. It probably makes sense to add golangci-lint to the CI as you suggested at some point, but I am not sure that we would want to enable that many linters, as I see many false positives in your output above (particularly with some of our helper functions like errors.Log and logx.PrintlnInfo), in addition to many less important things like documentation for various rarely used symbols. As such, fixing all of those issues would likely be an inefficient use of time compared to fixing more pressing bugs and implementing key features. However, things like staticcheck are pretty good, so I would be fine adding that to the CI through golangci-lint as we are already largely compliant with that. Regardless, this is not a super high priority, although I will certainly do it at some point.