add a check for a 'replace' directive in a 'go.mod' for Go modules

[thepudds]

Summary

In short, while a replace directive in a go.mod might be a necessary evil or might be needed for a temporary workaround, I think it is fair to say that it is always cleaner to not have a replace, and hence very worthwhile to drop the "Go Report Card" score if there is a replace found in a Go module.

Background

Some background on replace is available in this FAQ from the Modules wiki.

Some examples of why a project might have a replace when they are publishing an open source module with a "Go Report Card" badge:

1. It might be colossal mistake where someone accidentally checked in a temporary replace.

   For example, the author of module foo might have accidentally checked in a go.mod with something like replace bar => /user/Gopher42/my/local/bar, which tells the go command to use a local copy of the bar module, but would mean everyone downloading and building foo via something like go build . would be broken (because that local bar would not be there, unless you happen to be user Gopher42 in this example).

   There is a general issue that two modules can't automatically find each other if you are working on more than one module at the same time. (e.g., skim cmd/go: support simultaneous edits of interdependent modules golang/go#27542 or cmd/go: allow go.mod.local to contain replace/exclude lines golang/go#26640 for 30 sec to get a sense of the issue, if interested). The development time solution is often to introduce a replace directive (e.g., as explained in the FAQ above), but then you need to remember to remove that development time replace when you are checking in or when you are publishing your module. It can be scripted (e.g., via git hooks), but in practice people make mistakes here.

2. It might be a significant mistake if a library module author incorrectly thinks library consumers will respect the replace

   There is a nuance in modules that a library module can have a replace, but that replace only applies if you are running go build or go test from inside the library itself, and that same replace is ignored if you are importing that library in a separate module (say, the top-level binary). Not everyone knows this, however, so some library authors in practice include a replace without knowing it will be ignored in the top-level binary.

   The "Build Control" section in the official proposal has a discussion about this, including:

Minimal version selection gives the top-level module in the build additional control, allowing it to exclude specific module versions or replace others with different code, but those exclusions and replacements only apply when found in the top-level module, not when the module is a dependency in a larger build.

A module author is therefore in complete control of that module's build when it is the main program being built, but not in complete control of other users' builds that depend on the module.

3. It might be a deliberate choice by a library author, but a confusing one

   A more sophisticated module library author might know that a replace is ignored in a consuming top-level module, and might in theory purposefully include a replace in their own library (e.g., maybe to help with local testing) under the knowledge that it won't impact a consumer, but then that would be confusing for a consumer to see that replace, and hence it could be reasonable to view that as not a great practice.

4. It might be an indication that a specific version of a dependency is being pinned for a binary

5. It might be an indication that a fork of a dependency is in use for a binary

   For 4. or 5. -- that might be "OK", but it is at least notable and not as "clean" as if there was no replace. More importantly,it helps the overall ecosystem if any temporary forks (e.g., to temporarily fix a bug) are eventually merged upstream, and similarly if any upstream bugs are fixed upstream so that the version of a dependency no longer needs to be pinned to work around that upstream bug. The go report card could help nudge people in that direction and/or remind them to nudge upstream, etc., and once the issue is resolved upstream, the replace can then be removed and return to a "cleaner" state of no replace directives.

Recommendation

I think if you put all all that together, it is at a minimum a fairly notable thing that a project uses a replace regardless of why, and at worst it is a major mistake. Even if it is purposeful, it is is cleaner for the overall ecosystem if any temporarily necessary replace is later removed.

I would recommend starting very simple and not trying to differentiate between different cases above, and the logic could just be something like -- if there is any replace found, drop the score. Given it is at least eyebrow-raising and potentially a big mistake, I would recommend moving the score enough to always visibly drop a partial grade (e.g., move an "A+" to an "A", or a "B" to a "B-", etc.).

Most of the same issues also apply to the exclude directive, but exclude is not used as frequently as replace, so could be very reasonable to start just with replace, or alternatively could have the same basic scoring logic for exclude as well.

[thepudds]

The implementation could be fairly straightfoward.

go mod edit -json prints the current module's go.mod file in JSON form, including showing whether or not there are any replace. Alternatively, doing a find . -name go.mod and then go mod edit -json <path/to/a/go.mod> does the same for a go.mod outside of the current module. (Documentation link)

The JSON output corresponds to these Go types:

type GoMod struct {
    Module  Module
    Go      string
    Require []Require
    Exclude []Module
    Replace []Replace
}

type Module struct {
    Path string
    Version string
}

type Require struct {
    Path string
    Version string
    Indirect bool
}

To check if there are any replace directives, one can verify the Replace []Replace JSON field does not have any elements (e.g., is zero length, null, or not present).

Here is an example JSON output (in this case, showing there are no replace directives as seen by "Replace": null in the output):

$ go mod edit -json ./tempmodule/go.mod
{
        "Module": {
                "Path": "tempmodule"
        },
        "Require": [
                {
                        "Path": "cloud.google.com/go",
                        "Version": "v0.37.0",
                        "Indirect": true
                }
        ],
        "Exclude": null,
        "Replace": null
}

Alternatively, github.com/rogpeppe/go-internal/modfile is an importable go package that can parse a go.mod file.

[github-actions]

This issue is stale because it has been open 60 days with no activity.

[marcelmiguel]

For us, it will be nice to have:

• "go build" able to ignore "replace" section
• Control at CI: stop merging if go.mod includes replace.