x/mod/modfile: AddNewRequire doesn't put direct dependencies in the first block

[stevenh]

Go version

go version go1.23.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/steveh/.cache/go-build'
GOENV='/home/steveh/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/steveh/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/steveh/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/steveh/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/steveh/code/github.com/rocketsciencegg/congestion-control/tools/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build155249492=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Use AddNewRequire to add new requires.

package main

import (
	"testing"

	"github.com/stretchr/testify/require"
	"golang.org/x/mod/modfile"
	"golang.org/x/mod/module"
)

var testFile = `module test

go 1.23.0

require (
	github.com/test/test1 v0.1.0
	github.com/test/test2 v0.1.0
)

require (
	github.com/test/test3 v0.1.0 // indirect
)
`

var expected = `module test

go 1.23.0

require (
	github.com/foo/direct1 v0.1.1
	github.com/foo/direct2 v0.1.2
	github.com/test/test1 v0.1.0
	github.com/test/test2 v0.1.0
)

require (
	github.com/foo/indirect1 v0.2.1 // indirect
	github.com/foo/indirect2 v0.2.2 // indirect
	github.com/test/test3 v0.1.0 // indirect
)
`

func TestAddRequire(t *testing.T) {
	file, err := modfile.Parse("go.mod", []byte(testFile), nil)
	require.NoError(t, err)

	file.AddNewRequire("github.com/foo/indirect2", "v0.2.2", true)
	file.AddNewRequire("github.com/foo/direct2", "v0.1.2", false)
	file.AddNewRequire("github.com/foo/direct1", "v0.1.1", false)
	file.AddNewRequire("github.com/foo/indirect1", "v0.2.1", true)

	file.Cleanup()
	file.SortBlocks()

	data, err := file.Format()
	require.NoError(t, err)
	require.Equal(t, expected, string(data))
}

What did you see happen?

Direct requires should be added to first require block, indirect requires should be added to second block.

What did you expect to see?

When using AddNewRequire, requires are added to the last block. This is the documented behaviour, but other go tools such as go mod tidy maintain two blocks, so this use the same approach.

It seems possible to use SetRequireSeparateIndirect to replicate the desired behaviour but that was far from obvious.

If nothing else reference in AddNewRequire and Addequire to SetRequireSeparateIndirect would help users fine the right functionality, but ideally AddNewRequire and AddRequire should function as expected.

If a user isn't aware of the two block rule then using this will result in a file that go mod tidy handles badly, in some cases resulting in three blocks instead of two.

[gabyhelp]

Related Issues and Documentation

• cmd/go: `mod tidy -go=1.17` should move indirect dependencies to the second require part #47733 (closed)
• cmd/go: 'mod tidy' sometimes adds new, unnecessary require sections #67948
• cmd/go: separate direct and indirect dependencies in go.mod #27887 (closed)
• cmd/go: 'go mod tidy' may delete comments on require blocks #47563 (closed)
• cmd/go: mod tidy should join "require" sections if there are more than two #56471
• import/path: issue title more than require blocks in go.mod #68303 (closed)
• cmd/go: mod edit doesn't respect direct/indirect blocks in go.mod #51983 (closed)
• cmd/go: go directive is not only added during go mod init, but also under other conditions that are hard to deduce #30790 (closed)
• cmd/go: 'mod tidy' chooses points in requirement cycles that differ from other subcommands #34086 (closed)
• x/tools/gopls: missing direct requirements also listed in go.mod diagnostics #40470 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

See #68593, just parsing the modfile doesn't provide enough information for if a module should be direct or indirect, and the default addition is to mark it as indirect.

[stevenh]

See #68593, just parsing the modfile doesn't provide enough information for if a module should be direct or indirect, and the default addition is to mark it as indirect.

I'm not sure you correctly understood what I reported, AddNewRequire takes the indirect parameter to specify if the caller wants a direct or indirect, which it does correctly.

What it's not doing is adding it to the correct block.

[seankhliao]

Ah sorry.

[stevenh]

FYI I have some code which I believe fixes the behaviour, just need to get a PR created from it.

[gopherbot]

Change https://go.dev/cl/608056 mentions this issue: modfile: fix AddNewRequire block ordering

[matloob]

AddNewRequire is documented to add the require to the last block and I think we should be careful when making changes to behavior like this.

The go command only creates the two blocks for go modules declaring go 1.17 or later. We should make sure that we don't change that behavior.

Take a look at the code here: src/cmd/go/internal/modload/init.go#1846 The go command decides to call SetRequire or SetRequireSeparateIndirect based on the module's goVersion.

[stevenh]

Thanks for the link, to clarify are users of modfile expected to use SetRequireSeparateIndirect to add requires instead of AddRequire / AddNewRequire @matloob?