From d2163df6a0d6b9663a65438c1d960ea4ab5c2df0 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 14 Nov 2021 08:11:49 +0000 Subject: [PATCH] Fix offBy1 errors (#17606) * Fix offBy1 errors - Partially resolves #17596 - Resolve errors from go-critic `offBy1: Index() can return -1; maybe you wanted to do Index()+1`. * Match golang spec * Remove comments * Update migrations.go * Apply suggestions from code review Co-authored-by: delvh Co-authored-by: wxiaoguang Co-authored-by: delvh Co-authored-by: Lunny Xiao --- cmd/docs.go | 6 +++++- models/migrations/migrations.go | 9 ++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cmd/docs.go b/cmd/docs.go index 52233c7ac87..073c5749730 100644 --- a/cmd/docs.go +++ b/cmd/docs.go @@ -43,7 +43,11 @@ func runDocs(ctx *cli.Context) error { // Clean up markdown. The following bug was fixed in v2, but is present in v1. // It affects markdown output (even though the issue is referring to man pages) // https://github.com/urfave/cli/issues/1040 - docs = docs[strings.Index(docs, "#"):] + firstHashtagIndex := strings.Index(docs, "#") + + if firstHashtagIndex > 0 { + docs = docs[firstHashtagIndex:] + } } out := os.Stdout diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2686dfb3cff..c0d8f111d37 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -7,6 +7,7 @@ package migrations import ( "context" + "errors" "fmt" "os" "reflect" @@ -791,8 +792,14 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin } tableSQL := string(res[0]["sql"]) + // Get the string offset for column definitions: `CREATE TABLE ( column-definitions... )` + columnDefinitionsIndex := strings.Index(tableSQL, "(") + if columnDefinitionsIndex < 0 { + return errors.New("couldn't find column definitions") + } + // Separate out the column definitions - tableSQL = tableSQL[strings.Index(tableSQL, "("):] + tableSQL = tableSQL[columnDefinitionsIndex:] // Remove the required columnNames for _, name := range columnNames {