enforce explanation for necessary nolints and fix bugs

.golangci.yml

@@ -46,7 +46,8 @@ linters:
             - pkg: gitea.com/go-chi/cache
               desc: do not use the go-chi cache package, use gitea's cache system
     nolintlint:
-      # require-explanation: true
+      allow-unused: false
+      require-explanation: true
       require-specific: true
     gocritic:
       disabled-checks:

cmd/embedded.go

@@ -295,16 +295,14 @@ func collectAssetFilesByPattern(c *cli.Command, globs []glob.Glob, path string,
 	}
 }
 
-func compileCollectPatterns(args []string) ([]glob.Glob, error) {
+func compileCollectPatterns(args []string) (_ []glob.Glob, err error) {
 	if len(args) == 0 {
 		args = []string{"**"}
 	}
 	pat := make([]glob.Glob, len(args))
 	for i := range args {
-		if g, err := glob.Compile(args[i], '/'); err != nil {
-			return nil, fmt.Errorf("'%s': Invalid glob pattern: %w", args[i], err)
-		} else { //nolint:revive
-			pat[i] = g
+		if pat[i], err = glob.Compile(args[i], '/'); err != nil {
+			return nil, fmt.Errorf("invalid glob patterh %q: %w", args[i], err)
 		}
 	}
 	return pat, nil

contrib/backport/backport.go

@@ -1,7 +1,7 @@
 // Copyright 2023 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-//nolint:forbidigo
+//nolint:forbidigo // use of print functions is allowed in cli
 package main
 
 import (

models/actions/task.go

@@ -278,14 +278,13 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask
 		return nil, false, err
 	}
 
-	var workflowJob *jobparser.Job
-	if gots, err := jobparser.Parse(job.WorkflowPayload); err != nil {
+	parsedWorkflows, err := jobparser.Parse(job.WorkflowPayload)
+	if err != nil {
 		return nil, false, fmt.Errorf("parse workflow of job %d: %w", job.ID, err)
-	} else if len(gots) != 1 {
+	} else if len(parsedWorkflows) != 1 {
 		return nil, false, fmt.Errorf("workflow of job %d: not single workflow", job.ID)
-	} else { //nolint:revive
-		_, workflowJob = gots[0].Job()
 	}
+	_, workflowJob := parsedWorkflows[0].Job()
 
 	if _, err := e.Insert(task); err != nil {
 		return nil, false, err

models/auth/auth_token.go

@@ -15,7 +15,7 @@ import (
 
 var ErrAuthTokenNotExist = util.NewNotExistErrorf("auth token does not exist")
 
-type AuthToken struct { //nolint:revive
+type AuthToken struct { //nolint:revive // export stutter
 	ID          string `xorm:"pk"`
 	TokenHash   string
 	UserID      int64              `xorm:"INDEX"`

models/db/sql_postgres_with_schema.go

@@ -39,7 +39,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) {
 
 	// golangci lint is incorrect here - there is no benefit to using driver.ExecerContext here
 	// and in any case pq does not implement it
-	if execer, ok := conn.(driver.Execer); ok { //nolint:staticcheck
+	if execer, ok := conn.(driver.Execer); ok { //nolint:staticcheck // see above
 		_, err := execer.Exec(`SELECT set_config(
 			'search_path',
 			$1 || ',' || current_setting('search_path'),
@@ -64,7 +64,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) {
 	// driver.String.ConvertValue will never return err for string
 
 	// golangci lint is incorrect here - there is no benefit to using stmt.ExecWithContext here
-	_, err = stmt.Exec([]driver.Value{schemaValue}) //nolint:staticcheck
+	_, err = stmt.Exec([]driver.Value{schemaValue}) //nolint:staticcheck // see above
 	if err != nil {
 		_ = conn.Close()
 		return nil, err

models/migrations/base/tests.go

@@ -1,7 +1,6 @@
 // Copyright 2022 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-//nolint:forbidigo
 package base
 
 import (
@@ -106,7 +105,7 @@ func MainTest(m *testing.M) {
 	giteaConf := os.Getenv("GITEA_CONF")
 	if giteaConf == "" {
 		giteaConf = filepath.Join(filepath.Dir(setting.AppPath), "tests/sqlite.ini")
-		fmt.Printf("Environment variable $GITEA_CONF not set - defaulting to %s\n", giteaConf)
+		_, _ = fmt.Fprintf(os.Stderr, "Environment variable $GITEA_CONF not set - defaulting to %s\n", giteaConf)
 	}
 
 	if !filepath.IsAbs(giteaConf) {
@@ -134,7 +133,7 @@ func MainTest(m *testing.M) {
 	exitStatus := m.Run()
 
 	if err := removeAllWithRetry(setting.RepoRootPath); err != nil {
-		fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err)
+		_, _ = fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err)
 	}
 	os.Exit(exitStatus)
 }

models/migrations/v1_11/v112.go

@@ -4,9 +4,9 @@
 package v1_11
 
 import (
-	"fmt"
 	"path/filepath"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
@@ -31,7 +31,7 @@ func RemoveAttachmentMissedRepo(x *xorm.Engine) error {
 		for i := 0; i < len(attachments); i++ {
 			uuid := attachments[i].UUID
 			if err = util.RemoveAll(filepath.Join(setting.Attachment.Storage.Path, uuid[0:1], uuid[1:2], uuid)); err != nil {
-				fmt.Printf("Error: %v", err) //nolint:forbidigo
+				log.Warn("Unable to remove attachment file by UUID %s: %v", uuid, err)
 			}
 		}
 

models/migrations/v1_13/v140.go

@@ -21,12 +21,7 @@ func FixLanguageStatsToSaveSize(x *xorm.Engine) error {
 	// RepoIndexerType specifies the repository indexer type
 	type RepoIndexerType int
 
-	const (
-		// RepoIndexerTypeCode code indexer - 0
-		RepoIndexerTypeCode RepoIndexerType = iota //nolint:unused
-		// RepoIndexerTypeStats repository stats indexer - 1
-		RepoIndexerTypeStats
-	)
+	const RepoIndexerTypeStats RepoIndexerType = 1
 
 	// RepoIndexerStatus see models/repo_indexer.go
 	type RepoIndexerStatus struct {

models/migrations/v1_14/v157.go

@@ -8,17 +8,6 @@ import (
 )
 
 func FixRepoTopics(x *xorm.Engine) error {
-	type Topic struct { //nolint:unused
-		ID        int64  `xorm:"pk autoincr"`
-		Name      string `xorm:"UNIQUE VARCHAR(25)"`
-		RepoCount int
-	}
-
-	type RepoTopic struct { //nolint:unused
-		RepoID  int64 `xorm:"pk"`
-		TopicID int64 `xorm:"pk"`
-	}
-
 	type Repository struct {
 		ID     int64    `xorm:"pk autoincr"`
 		Topics []string `xorm:"TEXT JSON"`

models/migrations/v1_14/v165.go

@@ -16,10 +16,7 @@ func ConvertHookTaskTypeToVarcharAndTrim(x *xorm.Engine) error {
 		return nil
 	}
 
-	type HookTask struct { //nolint:unused
-		Typ string `xorm:"VARCHAR(16) index"`
-	}
-
+	// HookTask: Typ string `xorm:"VARCHAR(16) index"`
 	if err := base.ModifyColumn(x, "hook_task", &schemas.Column{
 		Name: "typ",
 		SQLType: schemas.SQLType{
@@ -42,10 +39,7 @@ func ConvertHookTaskTypeToVarcharAndTrim(x *xorm.Engine) error {
 		return err
 	}
 
-	type Webhook struct { //nolint:unused
-		Type string `xorm:"VARCHAR(16) index"`
-	}
-
+	// Webhook: Type string `xorm:"VARCHAR(16) index"`
 	if err := base.ModifyColumn(x, "webhook", &schemas.Column{
 		Name: "type",
 		SQLType: schemas.SQLType{

models/user/badge.go

@@ -19,7 +19,7 @@ type Badge struct {
 }
 
 // UserBadge represents a user badge
-type UserBadge struct { //nolint:revive
+type UserBadge struct { //nolint:revive // export stutter
 	ID      int64 `xorm:"pk autoincr"`
 	BadgeID int64
 	UserID  int64 `xorm:"INDEX"`

modules/auth/password/hash/common.go

@@ -18,7 +18,7 @@ func parseIntParam(value, param, algorithmName, config string, previousErr error
 	return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
 }
 
-func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) { //nolint:unparam
+func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) { //nolint:unparam // algorithmName is always argon2
 	parsed, err := strconv.ParseUint(value, 10, 64)
 	if err != nil {
 		log.Error("invalid integer for %s representation in %s hash spec %s", param, algorithmName, config)

modules/cache/cache_redis.go

@@ -11,7 +11,7 @@ import (
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/nosql"
 
-	"gitea.com/go-chi/cache" //nolint:depguard
+	"gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here
 	"github.com/redis/go-redis/v9"
 )
 

modules/cache/cache_twoqueue.go

@@ -10,7 +10,7 @@ import (
 
 	"code.gitea.io/gitea/modules/json"
 
-	mc "gitea.com/go-chi/cache" //nolint:depguard
+	mc "gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here
 	lru "github.com/hashicorp/golang-lru/v2"
 )
 

modules/cache/string_cache.go

@@ -11,7 +11,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
-	chi_cache "gitea.com/go-chi/cache" //nolint:depguard
+	chi_cache "gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here
 )
 
 type GetJSONError struct {

modules/graceful/manager_windows.go

@@ -41,8 +41,7 @@ func (g *Manager) start() {
 	// Make SVC process
 	run := svc.Run
 
-	//lint:ignore SA1019 We use IsAnInteractiveSession because IsWindowsService has a different permissions profile
-	isAnInteractiveSession, err := svc.IsAnInteractiveSession() //nolint:staticcheck
+	isAnInteractiveSession, err := svc.IsAnInteractiveSession() //nolint:staticcheck // must use IsAnInteractiveSession because IsWindowsService has a different permissions profile
 	if err != nil {
 		log.Error("Unable to ascertain if running as an Windows Service: %v", err)
 		return

modules/json/json.go

@@ -3,11 +3,10 @@
 
 package json
 
-// Allow "encoding/json" import.
 import (
 	"bytes"
 	"encoding/binary"
-	"encoding/json" //nolint:depguard
+	"encoding/json" //nolint:depguard // this package wraps it
 	"io"
 
 	jsoniter "github.com/json-iterator/go"

modules/log/logger.go

@@ -45,6 +45,6 @@ type Logger interface {
 	LevelLogger
 }
 
-type LogStringer interface { //nolint:revive
+type LogStringer interface { //nolint:revive // export stutter
 	LogString() string
 }

modules/markup/markdown/transform_blockquote.go

@@ -46,7 +46,7 @@ func (g *ASTTransformer) extractBlockquoteAttentionEmphasis(firstParagraph ast.N
 	if !ok {
 		return "", nil
 	}
-	val1 := string(node1.Text(reader.Source())) //nolint:staticcheck
+	val1 := string(node1.Text(reader.Source())) //nolint:staticcheck // Text is deprecated
 	attentionType := strings.ToLower(val1)
 	if g.attentionTypes.Contains(attentionType) {
 		return attentionType, []ast.Node{node1}

modules/markup/markdown/transform_codespan.go

@@ -68,7 +68,7 @@ func cssColorHandler(value string) bool {
 }
 
 func (g *ASTTransformer) transformCodeSpan(_ *markup.RenderContext, v *ast.CodeSpan, reader text.Reader) {
-	colorContent := v.Text(reader.Source()) //nolint:staticcheck
+	colorContent := v.Text(reader.Source()) //nolint:staticcheck // Text is deprecated
 	if cssColorHandler(string(colorContent)) {
 		v.AppendChild(v, NewColorPreview(colorContent))
 	}

modules/markup/markdown/transform_heading.go

@@ -19,7 +19,7 @@ func (g *ASTTransformer) transformHeading(_ *markup.RenderContext, v *ast.Headin
 			v.SetAttribute(attr.Name, fmt.Appendf(nil, "%v", attr.Value))
 		}
 	}
-	txt := v.Text(reader.Source()) //nolint:staticcheck
+	txt := v.Text(reader.Source()) //nolint:staticcheck // Text is deprecated
 	header := Header{
 		Text:  util.UnsafeBytesToString(txt),
 		Level: v.Level,

modules/markup/mdstripper/mdstripper.go

@@ -46,7 +46,7 @@ func (r *stripRenderer) Render(w io.Writer, source []byte, doc ast.Node) error {
 				coalesce := prevSibIsText
 				r.processString(
 					w,
-					v.Text(source), //nolint:staticcheck
+					v.Text(source), //nolint:staticcheck // Text is deprecated
 					coalesce)
 				if v.SoftLineBreak() {
 					r.doubleSpace(w)

modules/optional/serialization_test.go

@@ -4,7 +4,7 @@
 package optional_test
 
 import (
-	std_json "encoding/json" //nolint:depguard
+	std_json "encoding/json" //nolint:depguard // for testing purpose
 	"testing"
 
 	"code.gitea.io/gitea/modules/json"

modules/setting/config_env.go

@@ -97,7 +97,7 @@ func decodeEnvSectionKey(encoded string) (ok bool, section, key string) {
 
 // decodeEnvironmentKey decode the environment key to section and key
 // The environment key is in the form of GITEA__SECTION__KEY or GITEA__SECTION__KEY__FILE
-func decodeEnvironmentKey(prefixGitea, suffixFile, envKey string) (ok bool, section, key string, useFileValue bool) { //nolint:unparam
+func decodeEnvironmentKey(prefixGitea, suffixFile, envKey string) (ok bool, section, key string, useFileValue bool) {
 	if !strings.HasPrefix(envKey, prefixGitea) {
 		return false, "", "", false
 	}

modules/setting/config_env_test.go

@@ -73,6 +73,9 @@ func TestDecodeEnvironmentKey(t *testing.T) {
 	assert.Equal(t, "sec", section)
 	assert.Equal(t, "KEY", key)
 	assert.True(t, file)
+
+	ok, _, _, _ = decodeEnvironmentKey("PREFIX__", "", "PREFIX__SEC__KEY")
+	assert.True(t, ok)
 }
 
 func TestEnvironmentToConfig(t *testing.T) {

modules/setting/config_provider.go

@@ -15,7 +15,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/util"
 
-	"gopkg.in/ini.v1" //nolint:depguard
+	"gopkg.in/ini.v1" //nolint:depguard // wrapper for this package
 )
 
 type ConfigKey interface {

modules/setting/security.go

@@ -111,7 +111,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) {
 	if SecretKey == "" {
 		// FIXME: https://github.com/go-gitea/gitea/issues/16832
 		// Until it supports rotating an existing secret key, we shouldn't move users off of the widely used default value
-		SecretKey = "!#@FDEWREWR&*(" //nolint:gosec
+		SecretKey = "!#@FDEWREWR&*("
 	}
 
 	CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("gitea_incredible")

modules/setting/storage.go

@@ -158,7 +158,7 @@ const (
 	targetSecIsSec                                  // target section is from the name seciont [name]
 )
 
-func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) { //nolint:unparam
+func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) { //nolint:unparam // FIXME: targetSecType is always 0, wrong design?
 	targetSec, err := rootCfg.GetSection(storageSectionName + "." + typ)
 	if err != nil {
 		if !IsValidStorageType(StorageType(typ)) {
@@ -283,7 +283,7 @@ func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType,
 	return &storage, nil
 }
 
-func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl
+func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl // duplicates azure setup
 	var storage Storage
 	storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String())
 	if err := targetSec.MapTo(&storage.MinioConfig); err != nil {
@@ -312,7 +312,7 @@ func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType,
 	return &storage, nil
 }
 
-func getStorageForAzureBlob(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl
+func getStorageForAzureBlob(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl // duplicates minio setup
 	var storage Storage
 	storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String())
 	if err := targetSec.MapTo(&storage.AzureBlobConfig); err != nil {

modules/templates/htmlrenderer.go

@@ -42,7 +42,7 @@ var (
 
 var ErrTemplateNotInitialized = errors.New("template system is not initialized, check your log for errors")
 
-func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ctx context.Context) error { //nolint:revive
+func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ctx context.Context) error { //nolint:revive // we don't use ctx, only pass it to the template executor
 	name := string(tplName)
 	if respWriter, ok := w.(http.ResponseWriter); ok {
 		if respWriter.Header().Get("Content-Type") == "" {
@@ -57,7 +57,7 @@ func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ct
 	return t.Execute(w, data)
 }
 
-func (h *HTMLRender) TemplateLookup(name string, ctx context.Context) (TemplateExecutor, error) { //nolint:revive
+func (h *HTMLRender) TemplateLookup(name string, ctx context.Context) (TemplateExecutor, error) { //nolint:revive // we don't use ctx, only pass it to the template executor
 	tmpls := h.templates.Load()
 	if tmpls == nil {
 		return nil, ErrTemplateNotInitialized