chore: handle error types consistently (#9873)

Some error types are used inconsistently or wrong:

- `forgejo.org/modules/git.ErrNotExist` is meant to be a value error: <[modules/git/error.go#L23](https://codeberg.org/forgejo/forgejo/src/tag/v13.0.2/modules/git/error.go#L23)>

- `forgejo.org/models/repo.ErrRepoNotExist` is meant to be a value error: <[models/repo/repo.go#L750](https://codeberg.org/forgejo/forgejo/src/tag/v13.0.2/models/repo/repo.go#L750)>

- `errors.Is(logErr, &net.OpError{})` is always `false`: <[services/context/context_response.go#L188](https://codeberg.org/forgejo/forgejo/src/tag/v13.0.2/services/context/context_response.go#L188)>

- `forgejo.org/models/issues.ErrIssueContentHistoryNotExist` is used inconsistently: <[models/issues/content_history.go#L211](https://codeberg.org/forgejo/forgejo/src/tag/v13.0.2/models/issues/content_history.go#L211)>
Decided to use a value, since the structure is small and to be in line with the above errors.

These issued where found with the [errortype](https://codeberg.org/fillmore-labs/errortype) linter and add this to Makefile as part of the linter suite.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9873
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Oliver Eikemeier <eikemeier@fillmore-labs.com>
Co-committed-by: Oliver Eikemeier <eikemeier@fillmore-labs.com>

Makefile

@@ -46,6 +46,7 @@ XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest
 GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasource=go
 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go
 DEADCODE_PACKAGE ?= golang.org/x/tools/cmd/deadcode@v0.42.0 # renovate: datasource=go
+ERRORTYPE_PACKAGE ?= fillmore-labs.com/errortype@v0.0.10 # renovate: datasource=go
 GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.6.0 # renovate: datasource=go
 RENOVATE_NPM_PACKAGE ?= renovate@43.31.1 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate
 
@@ -486,6 +487,7 @@ RUN_DEADCODE = $(GO) run $(DEADCODE_PACKAGE) -generated=false -f='{{println .Pat
 .PHONY: lint-go
 lint-go:
 	$(GO) run $(GOLANGCI_LINT_PACKAGE) run $(GOLANGCI_LINT_ARGS)
+	$(GO) run $(ERRORTYPE_PACKAGE) ./...
 	$(RUN_DEADCODE) > .cur-deadcode-out
 	@$(DIFF) .deadcode-out .cur-deadcode-out \
 	|| (code=$$?; echo "Please run 'make lint-go-fix' and commit the result"; exit $${code})
@@ -955,6 +957,7 @@ deps-tools:
 	$(GO) install $(GO_LICENSES_PACKAGE)
 	$(GO) install $(GOVULNCHECK_PACKAGE)
 	$(GO) install $(GOMOCK_PACKAGE)
+	$(GO) install $(ERRORTYPE_PACKAGE)
 
 node_modules: package-lock.json
 	npm install --no-save

models/issues/content_history.go

@@ -222,7 +222,7 @@ func GetIssueContentHistoryAndPrev(dbCtx context.Context, issueID, id int64) (hi
 		return nil, nil, err
 	} else if !has {
 		log.Error("issue content history does not exist. id=%v. err=%v", id, err)
-		return nil, nil, &ErrIssueContentHistoryNotExist{id}
+		return nil, nil, ErrIssueContentHistoryNotExist{id}
 	}
 
 	prevHistory = &ContentHistory{}

modules/git/batch_reader.go

@@ -273,7 +273,7 @@ func ParseTreeLine(objectFormat ObjectFormat, rd *bufio.Reader, modeBuf, fnameBu
 	idx := bytes.IndexByte(readBytes, ' ')
 	if idx < 0 {
 		log.Debug("missing space in readBytes ParseTreeLine: %s", readBytes)
-		return mode, fname, sha, n, &ErrNotExist{}
+		return mode, fname, sha, n, ErrNotExist{}
 	}
 
 	n += idx + 1

modules/log/color_console_other.go

@@ -39,7 +39,7 @@ func fileStatDevIno(file *os.File) (uint64, uint64, bool) {
 
 	// Do a type conversion to uint64, because Dev isn't always uint64
 	// on every operating system + architecture combination.
-	return uint64(stat.Dev), stat.Ino, true //nolint:unconvert
+	return uint64(stat.Dev), stat.Ino, true //nolint:unconvert,nolintlint
 }
 
 func fileIsDevIno(file *os.File, dev, ino uint64) bool {

services/context/context_response.go

@@ -185,7 +185,8 @@ func (ctx *Context) ServerError(logMsg string, logErr error) {
 func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
 	if logErr != nil {
 		log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
-		if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) {
+		var opError *net.OpError
+		if errors.As(logErr, &opError) {
 			// This is an error within the underlying connection
 			// and further rendering will not work so just return
 			return

services/pull/temp_repo.go

@@ -53,7 +53,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 		return nil, nil, fmt.Errorf("%v LoadHeadRepo: %w", pr, err)
 	} else if pr.HeadRepo == nil {
 		log.Error("%-v HeadRepo %d does not exist", pr, pr.HeadRepoID)
-		return nil, nil, &repo_model.ErrRepoNotExist{
+		return nil, nil, repo_model.ErrRepoNotExist{
 			ID: pr.HeadRepoID,
 		}
 	} else if err := pr.LoadBaseRepo(ctx); err != nil {
@@ -61,7 +61,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 		return nil, nil, fmt.Errorf("%v LoadBaseRepo: %w", pr, err)
 	} else if pr.BaseRepo == nil {
 		log.Error("%-v BaseRepo %d does not exist", pr, pr.BaseRepoID)
-		return nil, nil, &repo_model.ErrRepoNotExist{
+		return nil, nil, repo_model.ErrRepoNotExist{
 			ID: pr.BaseRepoID,
 		}
 	} else if err := pr.HeadRepo.LoadOwner(ctx); err != nil {